Merge pull request #2722 from RyanDwyer/workspace-configs

Rename workspace_outputs to workspace_configs and fix memory leak

include/sway/config.h

@@ -167,13 +167,12 @@ struct output_config {
 };
 
 /**
- * Maps a workspace name to an output name.
+ * Stores configuration for a workspace, regardless of whether the workspace
- *
+ * exists.
- * Set via `workspace <x> output <y>`
  */
-struct workspace_output {
+struct workspace_config {
-	char *output;
 	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

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,

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));
+		char *ws_name = join_args(argv, argc - 2);
-		if (!wso) {
+		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);
+		free(wsc->output);
-		wso->output = strdup(argv[output_location + 1]);
+		wsc->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);
 	} else {
 		if (config->reading || !config->active) {
 			return cmd_results_new(CMD_DEFER, "workspace", NULL);

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);
 }
 

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
+	// Check workspace configs for a workspace<->output pair
-	for (int i = 0; i < config->workspace_outputs->length; ++i) {
+	struct workspace_config *wsc = workspace_find_config(name);
-		struct workspace_output *wso = config->workspace_outputs->items[i];
+	if (wsc && wsc->output) {
-		if (strcasecmp(wso->workspace, name) == 0) {
+		struct sway_output *output = output_by_name(wsc->output);
-			// Find output to use if it exists
+		if (output) {
-			struct sway_output *output = output_by_name(wso->output);
+			return output;
-			if (output) {
-				return output;
-			}
-			break;
 		}
 	}
 	// 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;
+	struct workspace_config *wsc = workspace_find_config(ws_name);
-	for (i = 0; i < config->workspace_outputs->length; ++i) {
+	return !wsc || !wsc->output || strcmp(wsc->output, output_name) == 0;
-		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;
 }
 
 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];
+		const struct workspace_config *wsc = config->workspace_configs->items[i];
-		if (strcmp(wso->output, output_name) == 0
+		if (wsc->output && strcmp(wsc->output, output_name) == 0
-				&& workspace_by_name(wso->workspace) == NULL) {
+				&& workspace_by_name(wsc->workspace) == NULL) {
 			free(target);
-			target = strdup(wso->workspace);
+			target = strdup(wsc->workspace);
 			break;
 		}
 	}