From f8b9c796f45359e60a4aba3a0c1af88158e9a8ec Mon Sep 17 00:00:00 2001 From: mstoeckl Date: Thu, 23 Aug 2018 17:25:48 -0400 Subject: [PATCH 1/2] Split parsing and execution of commands Due to variable substitution and string unescaping, parsing commands necessitates allocation. Because allocation can fail, commands (invoked, via bindings or criteria) may in turn fail when memory is restricted, which is most often the case in anomalous situations (i.e., a memory leak in sway, overcommit disabled, bad config/ipc, etc.). This commit defines a `struct stored_command` in which a parsed command can be stored, and then executed from. Command execution in execute_command now generates a series of struct stored_command, which are promptly invoked by execute_stored_command. --- include/sway/commands.h | 15 +++++- sway/commands.c | 102 ++++++++++++++++++++++------------------ 2 files changed, 71 insertions(+), 46 deletions(-) diff --git a/include/sway/commands.h b/include/sway/commands.h index 8e91c158f..81ae75def 100644 --- a/include/sway/commands.h +++ b/include/sway/commands.h @@ -51,7 +51,20 @@ struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, /** * Parse and executes a command. */ -struct cmd_results *execute_command(char *command, struct sway_seat *seat); +struct cmd_results *execute_command(const char *command, struct sway_seat *seat); + +struct stored_command { + sway_cmd *handle; + int argc; + char** argv; + struct criteria* criteria; +}; + +/** + * Executes a pre-parsed command. (Returns NULL if command was not unsuccessful: TODO, fix) + */ +struct cmd_results *execute_stored_command(const struct stored_command *command, + struct sway_seat *seat); /** * Parse and handles a command during config file loading. * diff --git a/sway/commands.c b/sway/commands.c index d9c54adc4..38cc9fac6 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -209,7 +209,7 @@ struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, return res; } -struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { +struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { // Even though this function will process multiple commands we will only // return the last error, if any (for now). (Since we have access to an // error string we could e.g. concatenate all errors there.) @@ -218,7 +218,6 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { char *head = exec; char *cmdlist; char *cmd; - list_t *views = NULL; if (seat == NULL) { // passing a NULL seat means we just pick the default seat @@ -228,25 +227,20 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { } } - config->handler_context.seat = seat; - head = exec; do { // Extract criteria (valid for this command list only). - config->handler_context.using_criteria = false; + struct criteria *criteria = NULL; if (*head == '[') { char *error = NULL; - struct criteria *criteria = criteria_parse(head, &error); + criteria = criteria_parse(head, &error); if (!criteria) { results = cmd_results_new(CMD_INVALID, head, "%s", error); free(error); goto cleanup; } - views = criteria_get_views(criteria); head += strlen(criteria->raw); - criteria_destroy(criteria); - config->handler_context.using_criteria = true; // Skip leading whitespace head += strspn(head, whitespace); } @@ -280,6 +274,9 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { } results = cmd_results_new(CMD_INVALID, cmd, "Unknown/invalid command"); free_argv(argc, argv); + if (criteria) { + criteria_destroy(criteria); + } goto cleanup; } @@ -289,53 +286,68 @@ struct cmd_results *execute_command(char *_exec, struct sway_seat *seat) { unescape_string(argv[i]); } - if (!config->handler_context.using_criteria) { - // without criteria, the command acts upon the focused - // container - config->handler_context.current_container = - seat_get_focus_inactive(seat, &root_container); - if (!sway_assert(config->handler_context.current_container, - "could not get focus-inactive for root container")) { - return NULL; - } - struct cmd_results *res = handler->handle(argc-1, argv+1); - if (res->status != CMD_SUCCESS) { - free_argv(argc, argv); - if (results) { - free_cmd_results(results); - } - results = res; - goto cleanup; - } - free_cmd_results(res); - } else { - for (int i = 0; i < views->length; ++i) { - struct sway_view *view = views->items[i]; - config->handler_context.current_container = view->swayc; - struct cmd_results *res = handler->handle(argc-1, argv+1); - if (res->status != CMD_SUCCESS) { - free_argv(argc, argv); - if (results) { - free_cmd_results(results); - } - results = res; - goto cleanup; - } - free_cmd_results(res); - } - } + struct stored_command command; + command.handle = handler->handle; + command.argv = argv; + command.argc = argc; + command.criteria = criteria; + struct cmd_results *res = execute_stored_command(&command, seat); free_argv(argc, argv); + + if (res->status != CMD_SUCCESS) { + if (results) { + free_cmd_results(results); + } + results = res; + if (criteria) { + criteria_destroy(criteria); + } + goto cleanup; + } + free_cmd_results(res); + } while(cmdlist); } while(head); cleanup: free(exec); - list_free(views); if (!results) { results = cmd_results_new(CMD_SUCCESS, NULL, NULL); } return results; } +struct cmd_results *execute_stored_command(const struct stored_command* command, + struct sway_seat* seat) { + config->handler_context.seat = seat; + + if (!command->criteria) { + // without criteria, the command acts upon the focused container + config->handler_context.using_criteria = true; + config->handler_context.current_container = + seat_get_focus_inactive(seat, &root_container); + if (!sway_assert(config->handler_context.current_container, + "could not get focus-inactive for root container")) { + return NULL; + } + return command->handle(command->argc-1, command->argv+1); + } else { + config->handler_context.using_criteria = false; + list_t *views = criteria_get_views(command->criteria); + for (int i = 0; i < views->length; ++i) { + struct sway_view *view = views->items[i]; + config->handler_context.current_container = view->swayc; + struct cmd_results *res = command->handle(command->argc-1, command->argv+1); + if (res->status != CMD_SUCCESS) { + return res; + } + free_cmd_results(res); + } + + list_free(views); + } + return NULL; +} + // this is like execute_command above, except: // 1) it ignores empty commands (empty lines) // 2) it does variable substitution From 6a805aace0b843e34083b005dd0b93ba8e7bb5fc Mon Sep 17 00:00:00 2001 From: mstoeckl Date: Thu, 23 Aug 2018 20:04:44 -0400 Subject: [PATCH 2/2] Use stored commands for bindings The preceding commit includes motivation; specifically, invoking keyboard and mouse bindings should never fail unless they perform an action which can fail. For instance, a `bindcode Ctrl+9 exit` should always exit; of course, `exec` may fail for external reasons. The core changes are to bind.c and commands.c. As a single command may actually, on being parsed, generate multiple commands to be executed (consider `workspace 1; splith`), the parsing code in execute_command is moved to a separate function, parse_command, which returns a list_t of stored commands; leaving execute_command to parse the command, execute each resulting list element; and then cleanup. An additional field for the list of parsed stored commands, is added to struct sway_binding, usurping the name "command". This is initialized via parse_command, and then seat_execute_command is adjusted accordingly. Side effects of this change include: * Reference counting `struct criteria`, because a criterion parsed once may apply to multiple stored commands, and the alternative of duplicating criteria is even more complicated. * Adding reading and active arguments to config_handler, because when bindsyms parse commands, they require handler lookups to be performed as though it were runtime. * Reference counting `struct sway_binding`, because the alternative of updating sway_binding_dup is even more complicated when allocation failures are be taken into account. --- common/list.c | 1 + include/sway/commands.h | 13 +++- include/sway/config.h | 16 ++-- include/sway/criteria.h | 3 + sway/commands.c | 165 +++++++++++++++++++++++++++------------- sway/commands/bar.c | 9 ++- sway/commands/bind.c | 116 ++++++++++++++-------------- sway/commands/input.c | 3 +- sway/commands/output.c | 3 +- sway/config.c | 18 ++++- sway/criteria.c | 1 + sway/ipc-server.c | 2 +- sway/tree/workspace.c | 2 +- 13 files changed, 217 insertions(+), 135 deletions(-) diff --git a/common/list.c b/common/list.c index ee268c9af..96190b12c 100644 --- a/common/list.c +++ b/common/list.c @@ -18,6 +18,7 @@ list_t *create_list(void) { static void list_resize(list_t *list) { if (list->length == list->capacity) { list->capacity += 10; + // TODO: safely handle realloc failure list->items = realloc(list->items, sizeof(void*) * list->capacity); } } diff --git a/include/sway/commands.h b/include/sway/commands.h index 81ae75def..8ed574d28 100644 --- a/include/sway/commands.h +++ b/include/sway/commands.h @@ -47,7 +47,7 @@ struct cmd_results *checkarg(int argc, const char *name, enum expected_args type, int val); struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, - int handlers_size); + int handlers_size, bool reading, bool active); /** * Parse and executes a command. */ @@ -56,10 +56,17 @@ struct cmd_results *execute_command(const char *command, struct sway_seat *seat struct stored_command { sway_cmd *handle; int argc; - char** argv; - struct criteria* criteria; + char **argv; + struct criteria *criteria; }; +void free_stored_command(struct stored_command *command); +/** + * Parses a command, and returns a list of stored_commands. If `for_runtime` + * is true, then command handler lookup is performed as though it were runtime; + * otherwise, as appropriate for the current `config` state. + */ +list_t *parse_command(const char *command, bool for_runtime); /** * Executes a pre-parsed command. (Returns NULL if command was not unsuccessful: TODO, fix) */ diff --git a/include/sway/config.h b/include/sway/config.h index c2eaea1bc..879111c5f 100644 --- a/include/sway/config.h +++ b/include/sway/config.h @@ -38,7 +38,9 @@ enum binding_flags { }; /** - * A key binding and an associated command. + * A key binding and an associated command. In order to extend the lifetime + * of a binding slightly while executing the reload command, bindings are + * reference counted, and should only be freed if the reference count is <= 0. */ struct sway_binding { enum binding_input_type type; @@ -46,15 +48,9 @@ struct sway_binding { uint32_t flags; list_t *keys; // sorted in ascending order uint32_t modifiers; - char *command; -}; - -/** - * A mouse binding and an associated command. - */ -struct sway_mouse_binding { - uint32_t button; - char *command; + char *command_str; // original command string for IPC compatibility + list_t *command; // a preparsed list of struct stored_command + int refcount; }; /** diff --git a/include/sway/criteria.h b/include/sway/criteria.h index 7a1e547b4..bb936a294 100644 --- a/include/sway/criteria.h +++ b/include/sway/criteria.h @@ -36,6 +36,9 @@ struct criteria { bool tiling; char urgent; // 'l' for latest or 'o' for oldest char *workspace; + // optional refcount for users (like struct stored_command) for which + // a criterion may be referenced multiple times. + int refcount; }; bool criteria_is_empty(struct criteria *criteria); diff --git a/sway/commands.c b/sway/commands.c index 38cc9fac6..a2c427b45 100644 --- a/sway/commands.c +++ b/sway/commands.c @@ -170,12 +170,12 @@ static int handler_compare(const void *_a, const void *_b) { } struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, - int handlers_size) { + int handlers_size, bool reading, bool active) { struct cmd_handler d = { .command=line }; struct cmd_handler *res = NULL; wlr_log(WLR_DEBUG, "find_handler(%s)", line); - bool config_loading = config->reading || !config->active; + bool config_loading = reading || !active; if (!config_loading) { res = bsearch(&d, command_handlers, @@ -210,14 +210,11 @@ struct cmd_handler *find_handler(char *line, struct cmd_handler *cmd_handlers, } struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { - // Even though this function will process multiple commands we will only - // return the last error, if any (for now). (Since we have access to an - // error string we could e.g. concatenate all errors there.) - struct cmd_results *results = NULL; - char *exec = strdup(_exec); - char *head = exec; - char *cmdlist; - char *cmd; + list_t *subcommands = parse_command(_exec, false); + if (!subcommands) { + return cmd_results_new(CMD_INVALID, _exec, + "Failed to allocate list of commands"); + } if (seat == NULL) { // passing a NULL seat means we just pick the default seat @@ -227,7 +224,53 @@ struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { } } - head = exec; + // Even though this function will process multiple commands we will only + // return the last error, if any (for now). (Since we have access to an + // error string we could e.g. concatenate all errors there.) + struct cmd_results *results = NULL; + for (int i = 0; i < subcommands->length; i++) { + struct stored_command *command = subcommands->items[i]; + struct cmd_results *res = execute_stored_command(command, seat); + if (res->status != CMD_SUCCESS) { + if (results) { + free_cmd_results(results); + } + results = res; + goto cleanup; + } + } +cleanup: + for (int i = 0; i < subcommands->length; i++) { + free_stored_command(subcommands->items[i]); + } + list_free(subcommands); + if (!results) { + results = cmd_results_new(CMD_SUCCESS, NULL, NULL); + } + return results; +} + +void free_stored_command(struct stored_command *command) { + free_argv(command->argc, command->argv); + if (command->criteria) { + command->criteria->refcount--; + if (command->criteria->refcount <= 0) { + criteria_destroy(command->criteria); + } + } + free(command); +} + +list_t *parse_command(const char *_exec, bool for_runtime) { + char *exec = strdup(_exec); + if (!exec) { + return NULL; + } + list_t *stored_list = create_list(); + + bool abort = false; + + char *head = exec; do { // Extract criteria (valid for this command list only). struct criteria *criteria = NULL; @@ -235,28 +278,28 @@ struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { char *error = NULL; criteria = criteria_parse(head, &error); if (!criteria) { - results = cmd_results_new(CMD_INVALID, head, - "%s", error); + wlr_log(WLR_ERROR, "%s", error); free(error); goto cleanup; } + criteria->refcount++; head += strlen(criteria->raw); // Skip leading whitespace head += strspn(head, whitespace); } // Split command list - cmdlist = argsep(&head, ";"); + char *cmdlist = argsep(&head, ";"); cmdlist += strspn(cmdlist, whitespace); do { // Split commands - cmd = argsep(&cmdlist, ","); + char *cmd = argsep(&cmdlist, ","); cmd += strspn(cmd, whitespace); if (strcmp(cmd, "") == 0) { wlr_log(WLR_INFO, "Ignoring empty command."); continue; } wlr_log(WLR_INFO, "Handling command '%s'", cmd); - //TODO better handling of argv + // TODO better handling of argv int argc; char **argv = split_args(cmd, &argc); if (strcmp(argv[0], "exec") != 0) { @@ -267,17 +310,20 @@ struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { } } } - struct cmd_handler *handler = find_handler(argv[0], NULL, 0); + // Identify the command + struct cmd_handler *handler; + if (for_runtime) { + handler = find_handler(argv[0], NULL, 0, false, true); + } else { + handler = find_handler(argv[0], NULL, 0, + config->reading, config->active); + } + if (!handler) { - if (results) { - free_cmd_results(results); - } - results = cmd_results_new(CMD_INVALID, cmd, "Unknown/invalid command"); + wlr_log(WLR_ERROR, "Unknown/invalid command"); free_argv(argc, argv); - if (criteria) { - criteria_destroy(criteria); - } - goto cleanup; + abort = true; + break; } // Var replacement, for all but first argument of set @@ -286,38 +332,51 @@ struct cmd_results *execute_command(const char *_exec, struct sway_seat *seat) { unescape_string(argv[i]); } - struct stored_command command; - command.handle = handler->handle; - command.argv = argv; - command.argc = argc; - command.criteria = criteria; - struct cmd_results *res = execute_stored_command(&command, seat); - free_argv(argc, argv); - - if (res->status != CMD_SUCCESS) { - if (results) { - free_cmd_results(results); - } - results = res; - if (criteria) { - criteria_destroy(criteria); - } - goto cleanup; + struct stored_command *command = (struct stored_command *) + calloc(1, sizeof(struct stored_command)); + if (!command) { + wlr_log(WLR_ERROR, "Unknown/invalid command"); + abort = true; + break; } - free_cmd_results(res); + // TODO: handle realloc failure case for list_add + list_add(stored_list, command); + + command->handle = handler->handle; + command->argv = argv; + command->argc = argc; + command->criteria = criteria; + if (criteria) { + command->criteria->refcount++; + } } while(cmdlist); + + if (criteria) { + criteria->refcount--; + if (criteria->refcount <= 0) { + criteria_destroy(criteria); + } + } + if (abort) { + break; + } } while(head); + cleanup: free(exec); - if (!results) { - results = cmd_results_new(CMD_SUCCESS, NULL, NULL); - } - return results; + return stored_list; } -struct cmd_results *execute_stored_command(const struct stored_command* command, - struct sway_seat* seat) { +struct cmd_results *execute_stored_command(const struct stored_command *command, + struct sway_seat *seat) { + if (seat == NULL) { + // passing a NULL seat means we just pick the default seat + seat = input_manager_get_default_seat(input_manager); + if (!sway_assert(seat, "could not find a seat to run the command on")) { + return NULL; + } + } config->handler_context.seat = seat; if (!command->criteria) { @@ -378,7 +437,8 @@ struct cmd_results *config_command(char *exec) { goto cleanup; } wlr_log(WLR_INFO, "handling config command '%s'", exec); - struct cmd_handler *handler = find_handler(argv[0], NULL, 0); + struct cmd_handler *handler = find_handler(argv[0], NULL, 0, + config->reading, config->active); if (!handler) { char *input = argv[0] ? argv[0] : "(empty)"; results = cmd_results_new(CMD_INVALID, input, "Unknown/invalid command"); @@ -414,7 +474,7 @@ struct cmd_results *config_subcommand(char **argv, int argc, free(command); struct cmd_handler *handler = find_handler(argv[0], handlers, - handlers_size); + handlers_size, config->reading, config->active); if (!handler) { char *input = argv[0] ? argv[0] : "(empty)"; return cmd_results_new(CMD_INVALID, input, "Unknown/invalid command"); @@ -448,7 +508,8 @@ struct cmd_results *config_commands_command(char *exec) { goto cleanup; } - struct cmd_handler *handler = find_handler(cmd, NULL, 0); + struct cmd_handler *handler = find_handler(cmd, NULL, 0, + config->reading, config->active); if (!handler && strcmp(cmd, "*") != 0) { results = cmd_results_new(CMD_INVALID, cmd, "Unknown/invalid command"); goto cleanup; diff --git a/sway/commands/bar.c b/sway/commands/bar.c index f6a70c175..a6b72eb44 100644 --- a/sway/commands/bar.c +++ b/sway/commands/bar.c @@ -48,7 +48,8 @@ struct cmd_results *cmd_bar(int argc, char **argv) { if (!config->reading) { if (!find_handler(argv[0], bar_config_handlers, - sizeof(bar_config_handlers))) { + sizeof(bar_config_handlers), config->reading, + config->active)) { return cmd_results_new(CMD_FAILURE, "bar", "Can only be used in config file."); } @@ -58,8 +59,10 @@ struct cmd_results *cmd_bar(int argc, char **argv) { if (argc > 1) { struct bar_config *bar = NULL; - if (!find_handler(argv[0], bar_handlers, sizeof(bar_handlers)) - && find_handler(argv[1], bar_handlers, sizeof(bar_handlers))) { + if (!find_handler(argv[0], bar_handlers, sizeof(bar_handlers), + config->reading, config->active) + && find_handler(argv[1], bar_handlers, sizeof(bar_handlers), + config->reading, config->active)) { for (int i = 0; i < config->bars->length; ++i) { struct bar_config *item = config->bars->items[i]; if (strcmp(item->id, argv[0]) == 0) { diff --git a/sway/commands/bind.c b/sway/commands/bind.c index 8270b958d..7b5577472 100644 --- a/sway/commands/bind.c +++ b/sway/commands/bind.c @@ -20,43 +20,21 @@ int binding_order = 0; void free_sway_binding(struct sway_binding *binding) { if (!binding) { + wlr_log(WLR_DEBUG, "Unexpected: tried to free a null binding"); return; } if (binding->keys) { free_flat_list(binding->keys); } - free(binding->command); + for (int i = 0; i < binding->command->length; i++) { + free_stored_command(binding->command->items[i]); + } + list_free(binding->command); + free(binding->command_str); free(binding); } -static struct sway_binding *sway_binding_dup(struct sway_binding *sb) { - struct sway_binding *new_sb = calloc(1, sizeof(struct sway_binding)); - if (!new_sb) { - return NULL; - } - - new_sb->type = sb->type; - new_sb->order = sb->order; - new_sb->flags = sb->flags; - new_sb->modifiers = sb->modifiers; - new_sb->command = strdup(sb->command); - - new_sb->keys = create_list(); - int i; - for (i = 0; i < sb->keys->length; ++i) { - xkb_keysym_t *key = malloc(sizeof(xkb_keysym_t)); - if (!key) { - free_sway_binding(new_sb); - return NULL; - } - *key = *(xkb_keysym_t *)sb->keys->items[i]; - list_add(new_sb->keys, key); - } - - return new_sb; -} - /** * Returns true if the bindings have the same key and modifier combinations. * Note that keyboard layout is not considered, so the bindings might actually @@ -110,8 +88,8 @@ static int key_qsort_cmp(const void *keyp_a, const void *keyp_b) { * the value of *type if the initial type guess was incorrect and if this * was the first identified key. */ -static struct cmd_results *identify_key(const char* name, bool first_key, - uint32_t* key_val, enum binding_input_type* type) { +static struct cmd_results *identify_key(const char *name, bool first_key, + uint32_t *key_val, enum binding_input_type *type) { if (*type == BINDING_KEYCODE) { // check for keycode xkb_keycode_t keycode = strtol(name, NULL, 10); @@ -180,6 +158,7 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv, binding->modifiers = 0; binding->flags = 0; binding->type = bindcode ? BINDING_KEYCODE : BINDING_KEYSYM; + binding->refcount = 1; bool exclude_titlebar = false; @@ -213,7 +192,18 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv, "(expected at least 2 non-option arguments, got %d)", bindtype, argc); } - binding->command = join_args(argv + 1, argc - 1); + binding->command_str = join_args(argv + 1, argc - 1); + if (!binding->command_str) { + free_sway_binding(binding); + return cmd_results_new(CMD_FAILURE, bindtype, "Unable to allocate command string"); + } + binding->command = parse_command(binding->command_str, true); + if (!binding->command) { + free_sway_binding(binding); + return cmd_results_new(CMD_FAILURE, bindtype, + "Unable to allocate list of commands"); + } + list_t *split = split_string(argv[0], "+"); for (int i = 0; i < split->length; ++i) { @@ -272,8 +262,9 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv, for (int i = 0; i < mode_bindings->length; ++i) { struct sway_binding *config_binding = mode_bindings->items[i]; if (binding_key_compare(binding, config_binding)) { - wlr_log(WLR_DEBUG, "overwriting old binding with command '%s'", - config_binding->command); + wlr_log(WLR_DEBUG, "overwriting old binding, #%d with command %s", + config_binding->order, + config_binding->command_str); free_sway_binding(config_binding); mode_bindings->items[i] = binding; overwritten = true; @@ -284,8 +275,8 @@ static struct cmd_results *cmd_bindsym_or_bindcode(int argc, char **argv, list_add(mode_bindings, binding); } - wlr_log(WLR_DEBUG, "%s - Bound %s to command %s", - bindtype, argv[0], binding->command); + wlr_log(WLR_DEBUG, "%s - Binding #%d bound %s with command '%s'", + bindtype, binding->order, argv[0], binding->command_str); return cmd_results_new(CMD_SUCCESS, NULL, NULL); } @@ -303,33 +294,38 @@ struct cmd_results *cmd_bindcode(int argc, char **argv) { * Execute the command associated to a binding */ void seat_execute_command(struct sway_seat *seat, struct sway_binding *binding) { - wlr_log(WLR_DEBUG, "running command for binding: %s", - binding->command); + for (int i = 0; i < binding->command->length; i++) { + struct stored_command *command = binding->command->items[i]; - struct sway_binding *binding_copy = binding; - bool reload = false; - // if this is a reload command we need to make a duplicate of the - // binding since it will be gone after the reload has completed. - if (strcasecmp(binding->command, "reload") == 0) { - reload = true; - binding_copy = sway_binding_dup(binding); - if (!binding_copy) { - wlr_log(WLR_ERROR, "Failed to duplicate binding during reload"); + wlr_log(WLR_DEBUG, "running command for binding, '%s' (with %d arguments)", + command->argv[0], command->argc - 1); + + bool reload = false; + // if this is a reload command, we increment the refcount to + // keep the binding alive up to the end of this function + if (command->handle == cmd_reload) { + reload = true; + binding->refcount++; + } + + config->handler_context.seat = seat; + struct cmd_results *results = execute_stored_command(command, NULL); + if (results->status == CMD_SUCCESS) { + ipc_event_binding(binding); + } else { + wlr_log(WLR_DEBUG, "could not run command for binding #%d: %s", + binding->order, results->error); + } + free_cmd_results(results); + + if (reload) { + // free the binding if/once it's no longer referenced, + // and return, since this binding may have been freed + binding->refcount--; + if (binding->refcount <= 0) { + free_sway_binding(binding); + } return; } } - - config->handler_context.seat = seat; - struct cmd_results *results = execute_command(binding->command, NULL); - if (results->status == CMD_SUCCESS) { - ipc_event_binding(binding_copy); - } else { - wlr_log(WLR_DEBUG, "could not run command for binding: %s (%s)", - binding->command, results->error); - } - - if (reload) { // free the binding if we made a copy - free_sway_binding(binding_copy); - } - free_cmd_results(results); } diff --git a/sway/commands/input.c b/sway/commands/input.c index 84888fbb7..9e8f56236 100644 --- a/sway/commands/input.c +++ b/sway/commands/input.c @@ -53,7 +53,8 @@ struct cmd_results *cmd_input(int argc, char **argv) { struct cmd_results *res; if (find_handler(argv[1], input_config_handlers, - sizeof(input_config_handlers))) { + sizeof(input_config_handlers), config->reading, + config->active)) { if (config->reading) { res = config_subcommand(argv + 1, argc - 1, input_config_handlers, sizeof(input_config_handlers)); diff --git a/sway/commands/output.c b/sway/commands/output.c index ef1b7a692..e708c620a 100644 --- a/sway/commands/output.c +++ b/sway/commands/output.c @@ -40,7 +40,8 @@ struct cmd_results *cmd_output(int argc, char **argv) { config->handler_context.leftovers.argc = 0; config->handler_context.leftovers.argv = NULL; - if (find_handler(*argv, output_handlers, sizeof(output_handlers))) { + if (find_handler(*argv, output_handlers, sizeof(output_handlers), + config->reading, config->active)) { error = config_subcommand(argv, argc, output_handlers, sizeof(output_handlers)); } else { diff --git a/sway/config.c b/sway/config.c index 642abbac8..5d5020201 100644 --- a/sway/config.c +++ b/sway/config.c @@ -47,19 +47,31 @@ static void free_mode(struct sway_mode *mode) { free(mode->name); if (mode->keysym_bindings) { for (i = 0; i < mode->keysym_bindings->length; i++) { - free_sway_binding(mode->keysym_bindings->items[i]); + struct sway_binding* binding = mode->keysym_bindings->items[i]; + binding->refcount--; + if (binding->refcount <= 0) { + free_sway_binding(binding); + } } list_free(mode->keysym_bindings); } if (mode->keycode_bindings) { for (i = 0; i < mode->keycode_bindings->length; i++) { - free_sway_binding(mode->keycode_bindings->items[i]); + struct sway_binding* binding = mode->keycode_bindings->items[i]; + binding->refcount--; + if (binding->refcount <= 0) { + free_sway_binding(binding); + } } list_free(mode->keycode_bindings); } if (mode->mouse_bindings) { for (i = 0; i < mode->mouse_bindings->length; i++) { - free_sway_binding(mode->mouse_bindings->items[i]); + struct sway_binding* binding = mode->mouse_bindings->items[i]; + binding->refcount--; + if (binding->refcount <= 0) { + free_sway_binding(binding); + } } list_free(mode->mouse_bindings); } diff --git a/sway/criteria.c b/sway/criteria.c index 81c2325aa..5eb78d40d 100644 --- a/sway/criteria.c +++ b/sway/criteria.c @@ -525,6 +525,7 @@ struct criteria *criteria_parse(char *raw, char **error_arg) { } ++head; + // zero allocation also ensures the optional refcount defaults to zero struct criteria *criteria = calloc(sizeof(struct criteria), 1); char *name = NULL, *value = NULL; bool in_quotes = false; diff --git a/sway/ipc-server.c b/sway/ipc-server.c index 34e940ad8..a2e0e6a67 100644 --- a/sway/ipc-server.c +++ b/sway/ipc-server.c @@ -379,7 +379,7 @@ void ipc_event_binding(struct sway_binding *binding) { wlr_log(WLR_DEBUG, "Sending binding event"); json_object *json_binding = json_object_new_object(); - json_object_object_add(json_binding, "command", json_object_new_string(binding->command)); + json_object_object_add(json_binding, "command", json_object_new_string(binding->command_str)); const char *names[10]; int len = get_modifier_names(names, binding->modifiers); diff --git a/sway/tree/workspace.c b/sway/tree/workspace.c index 93c4b3d31..f308293e6 100644 --- a/sway/tree/workspace.c +++ b/sway/tree/workspace.c @@ -162,7 +162,7 @@ static bool workspace_valid_on_output(const char *output_name, static void workspace_name_from_binding(const struct sway_binding * binding, const char* output_name, int *min_order, char **earliest_name) { - char *cmdlist = strdup(binding->command); + char *cmdlist = strdup(binding->command_str); char *dup = cmdlist; char *name = NULL;