From 138d10d5d62a25c2e00bd9051c835b9e78a36de4 Mon Sep 17 00:00:00 2001 From: Ryan Dwyer Date: Fri, 28 Sep 2018 21:58:23 +1000 Subject: [PATCH 1/2] Rename workspace_outputs to workspace_configs and fix memory leak When we eventually implement `workspace gaps inner|outer `, we'll need to store the gaps settings for workspaces before they're created. Rather than create a workspace_gaps struct, the approach I'm taking is to rename workspace_outputs to workspace_configs and then add gaps settings to that. I've added a lookup function workspace_find_config. Note that we have a similar thing for outputs (output_config struct and output_find_config). Lastly, when freeing config it would create a memory leak by freeing the list items but not the workspace or output names inside them. This has been rectified using a free_workspace_config function. --- include/sway/config.h | 13 +++++----- include/sway/tree/workspace.h | 2 ++ sway/commands/workspace.c | 38 ++++++++++++++++++--------- sway/config.c | 11 +++++--- sway/tree/workspace.c | 49 ++++++++++++++++------------------- 5 files changed, 66 insertions(+), 47 deletions(-) diff --git a/include/sway/config.h b/include/sway/config.h index 35f0e708f..af5c7a188 100644 --- a/include/sway/config.h +++ b/include/sway/config.h @@ -167,13 +167,12 @@ struct output_config { }; /** - * Maps a workspace name to an output name. - * - * Set via `workspace output ` + * Stores configuration for a workspace, regardless of whether the workspace + * exists. */ -struct workspace_output { - char *output; +struct workspace_config { char *workspace; + char *output; }; struct bar_config { @@ -327,7 +326,7 @@ struct sway_config { list_t *modes; list_t *bars; list_t *cmd_queue; - list_t *workspace_outputs; + list_t *workspace_configs; list_t *output_configs; list_t *input_configs; list_t *seat_configs; @@ -518,6 +517,8 @@ struct bar_config *default_bar_config(void); void free_bar_config(struct bar_config *bar); +void free_workspace_config(struct workspace_config *wsc); + /** * Updates the value of config->font_height based on the max title height * reported by each container. If recalculate is true, the containers will diff --git a/include/sway/tree/workspace.h b/include/sway/tree/workspace.h index e4b616d13..c8220b395 100644 --- a/include/sway/tree/workspace.h +++ b/include/sway/tree/workspace.h @@ -48,6 +48,8 @@ struct sway_workspace { extern char *prev_workspace_name; +struct workspace_config *workspace_find_config(const char *ws_name); + struct sway_output *workspace_get_initial_output(const char *name); struct sway_workspace *workspace_create(struct sway_output *output, diff --git a/sway/commands/workspace.c b/sway/commands/workspace.c index f026a39d4..2ce7872d6 100644 --- a/sway/commands/workspace.c +++ b/sway/commands/workspace.c @@ -10,6 +10,26 @@ #include "log.h" #include "stringop.h" +static struct workspace_config *workspace_config_find_or_create(char *ws_name) { + struct workspace_config *wsc = workspace_find_config(ws_name); + if (wsc) { + return wsc; + } + wsc = calloc(1, sizeof(struct workspace_config)); + if (!wsc) { + return NULL; + } + wsc->workspace = strdup(ws_name); + list_add(config->workspace_configs, wsc); + return wsc; +} + +void free_workspace_config(struct workspace_config *wsc) { + free(wsc->workspace); + free(wsc->output); + free(wsc); +} + struct cmd_results *cmd_workspace(int argc, char **argv) { struct cmd_results *error = NULL; if ((error = checkarg(argc, "workspace", EXPECTED_AT_LEAST, 1))) { @@ -28,21 +48,15 @@ struct cmd_results *cmd_workspace(int argc, char **argv) { if ((error = checkarg(argc, "workspace", EXPECTED_EQUAL_TO, output_location + 2))) { return error; } - struct workspace_output *wso = calloc(1, sizeof(struct workspace_output)); - if (!wso) { + char *ws_name = join_args(argv, argc - 2); + struct workspace_config *wsc = workspace_config_find_or_create(ws_name); + free(ws_name); + if (!wsc) { return cmd_results_new(CMD_FAILURE, "workspace output", "Unable to allocate workspace output"); } - wso->workspace = join_args(argv, argc - 2); - wso->output = strdup(argv[output_location + 1]); - int i = -1; - if ((i = list_seq_find(config->workspace_outputs, workspace_output_cmp_workspace, wso)) != -1) { - struct workspace_output *old = config->workspace_outputs->items[i]; - free(old); // workspaces can only be assigned to a single output - list_del(config->workspace_outputs, i); - } - wlr_log(WLR_DEBUG, "Assigning workspace %s to output %s", wso->workspace, wso->output); - list_add(config->workspace_outputs, wso); + free(wsc->output); + wsc->output = strdup(argv[output_location + 1]); } else { if (config->reading || !config->active) { return cmd_results_new(CMD_DEFER, "workspace", NULL); diff --git a/sway/config.c b/sway/config.c index 830fb65f6..1e08559d4 100644 --- a/sway/config.c +++ b/sway/config.c @@ -95,7 +95,12 @@ void free_config(struct sway_config *config) { list_free(config->bars); } list_free(config->cmd_queue); - list_free(config->workspace_outputs); + if (config->workspace_configs) { + for (int i = 0; i < config->workspace_configs->length; i++) { + free_workspace_config(config->workspace_configs->items[i]); + } + list_free(config->workspace_configs); + } if (config->output_configs) { for (int i = 0; i < config->output_configs->length; i++) { free_output_config(config->output_configs->items[i]); @@ -175,7 +180,7 @@ static void config_defaults(struct sway_config *config) { if (!(config->symbols = create_list())) goto cleanup; if (!(config->modes = create_list())) goto cleanup; if (!(config->bars = create_list())) goto cleanup; - if (!(config->workspace_outputs = create_list())) goto cleanup; + if (!(config->workspace_configs = create_list())) goto cleanup; if (!(config->criteria = create_list())) goto cleanup; if (!(config->no_focus = create_list())) goto cleanup; if (!(config->input_configs = create_list())) goto cleanup; @@ -804,7 +809,7 @@ char *do_var_replacement(char *str) { // would compare two structs in full, while this method only compares the // workspace. int workspace_output_cmp_workspace(const void *a, const void *b) { - const struct workspace_output *wsa = a, *wsb = b; + const struct workspace_config *wsa = a, *wsb = b; return lenient_strcmp(wsa->workspace, wsb->workspace); } diff --git a/sway/tree/workspace.c b/sway/tree/workspace.c index 16031e873..e592302ce 100644 --- a/sway/tree/workspace.c +++ b/sway/tree/workspace.c @@ -20,17 +20,23 @@ #include "log.h" #include "util.h" +struct workspace_config *workspace_find_config(const char *ws_name) { + for (int i = 0; i < config->workspace_configs->length; ++i) { + struct workspace_config *wsc = config->workspace_configs->items[i]; + if (strcmp(wsc->workspace, ws_name) == 0) { + return wsc; + } + } + return NULL; +} + struct sway_output *workspace_get_initial_output(const char *name) { - // Search for workspace<->output pair - for (int i = 0; i < config->workspace_outputs->length; ++i) { - struct workspace_output *wso = config->workspace_outputs->items[i]; - if (strcasecmp(wso->workspace, name) == 0) { - // Find output to use if it exists - struct sway_output *output = output_by_name(wso->output); - if (output) { - return output; - } - break; + // Check workspace configs for a workspace<->output pair + struct workspace_config *wsc = workspace_find_config(name); + if (wsc && wsc->output) { + struct sway_output *output = output_by_name(wsc->output); + if (output) { + return output; } } // Otherwise put it on the focused output @@ -121,17 +127,8 @@ void next_name_map(struct sway_container *ws, void *data) { static bool workspace_valid_on_output(const char *output_name, const char *ws_name) { - int i; - for (i = 0; i < config->workspace_outputs->length; ++i) { - struct workspace_output *wso = config->workspace_outputs->items[i]; - if (strcasecmp(wso->workspace, ws_name) == 0) { - if (strcasecmp(wso->output, output_name) != 0) { - return false; - } - } - } - - return true; + struct workspace_config *wsc = workspace_find_config(ws_name); + return !wsc || strcmp(wsc->output, output_name) == 0; } static void workspace_name_from_binding(const struct sway_binding * binding, @@ -231,13 +228,13 @@ char *workspace_next_name(const char *output_name) { workspace_name_from_binding(mode->keycode_bindings->items[i], output_name, &order, &target); } - for (int i = 0; i < config->workspace_outputs->length; ++i) { + for (int i = 0; i < config->workspace_configs->length; ++i) { // Unlike with bindings, this does not guarantee order - const struct workspace_output *wso = config->workspace_outputs->items[i]; - if (strcmp(wso->output, output_name) == 0 - && workspace_by_name(wso->workspace) == NULL) { + const struct workspace_config *wsc = config->workspace_configs->items[i]; + if (wsc->output && strcmp(wsc->output, output_name) == 0 + && workspace_by_name(wsc->workspace) == NULL) { free(target); - target = strdup(wso->workspace); + target = strdup(wsc->workspace); break; } } From 56e9f31b2f5ad057c3f6809fafc73132647ae686 Mon Sep 17 00:00:00 2001 From: Ryan Dwyer Date: Fri, 28 Sep 2018 22:34:51 +1000 Subject: [PATCH 2/2] Check for NULL output in workspace_valid_on_output --- sway/tree/workspace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/sway/tree/workspace.c b/sway/tree/workspace.c index e592302ce..d65a3e4ca 100644 --- a/sway/tree/workspace.c +++ b/sway/tree/workspace.c @@ -128,7 +128,7 @@ void next_name_map(struct sway_container *ws, void *data) { static bool workspace_valid_on_output(const char *output_name, const char *ws_name) { struct workspace_config *wsc = workspace_find_config(ws_name); - return !wsc || strcmp(wsc->output, output_name) == 0; + return !wsc || !wsc->output || strcmp(wsc->output, output_name) == 0; } static void workspace_name_from_binding(const struct sway_binding * binding,