From ce17788533600bedf06cdbfbac1f2bd373484a24 Mon Sep 17 00:00:00 2001 From: Dominique Martinet Date: Sun, 1 Jul 2018 00:00:15 +0900 Subject: exec_always: fix leaks - child would leak in the workspace_record_pid path - removing malloc lets us get rid of That Comment nobody seems to remember what it was about - we would leak pipe fds on first fork failling - we didn't return an error if second fork failed - the final executed process still had both pipe fds (would show up in /proc/23560/fd in launched programs) - we would write twice to the pipe if execl failed for some reason (e.g. if /bin/sh doesn't exist?!) --- sway/commands/exec_always.c | 27 +++++++++++++-------------- 1 file changed, 13 insertions(+), 14 deletions(-) (limited to 'sway/commands/exec_always.c') diff --git a/sway/commands/exec_always.c b/sway/commands/exec_always.c index 682d195e..1c99de97 100644 --- a/sway/commands/exec_always.c +++ b/sway/commands/exec_always.c @@ -42,43 +42,42 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { wlr_log(L_ERROR, "Unable to create pipe for fork"); } - pid_t pid; - pid_t *child = malloc(sizeof(pid_t)); // malloc'd so that Linux can avoid copying the process space - if (!child) { - return cmd_results_new(CMD_FAILURE, "exec_always", "Unable to allocate child pid"); - } + pid_t pid, child; // Fork process if ((pid = fork()) == 0) { // Fork child process again setsid(); - if ((*child = fork()) == 0) { + close(fd[0]); + if ((child = fork()) == 0) { + close(fd[1]); execl("/bin/sh", "/bin/sh", "-c", cmd, (void *)NULL); - // Not reached + _exit(0); } - close(fd[0]); ssize_t s = 0; while ((size_t)s < sizeof(pid_t)) { - s += write(fd[1], ((uint8_t *)child) + s, sizeof(pid_t) - s); + s += write(fd[1], ((uint8_t *)&child) + s, sizeof(pid_t) - s); } close(fd[1]); _exit(0); // Close child process } else if (pid < 0) { - free(child); + close(fd[0]); + close(fd[1]); return cmd_results_new(CMD_FAILURE, "exec_always", "fork() failed"); } close(fd[1]); // close write ssize_t s = 0; while ((size_t)s < sizeof(pid_t)) { - s += read(fd[0], ((uint8_t *)child) + s, sizeof(pid_t) - s); + s += read(fd[0], ((uint8_t *)&child) + s, sizeof(pid_t) - s); } close(fd[0]); // cleanup child process waitpid(pid, NULL, 0); - if (*child > 0) { - wlr_log(L_DEBUG, "Child process created with pid %d", *child); + if (child > 0) { + wlr_log(L_DEBUG, "Child process created with pid %d", child); // TODO: add PID to active workspace } else { - free(child); + return cmd_results_new(CMD_FAILURE, "exec_always", + "Second fork() failed"); } return cmd_results_new(CMD_SUCCESS, NULL, NULL); -- cgit v1.2.3 From 63b4bf500020cf35cebfdce2d73f8e359ff495c2 Mon Sep 17 00:00:00 2001 From: emersion Date: Mon, 9 Jul 2018 22:54:30 +0100 Subject: Update for swaywm/wlroots#1126 --- sway/commands/exec_always.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'sway/commands/exec_always.c') diff --git a/sway/commands/exec_always.c b/sway/commands/exec_always.c index 1c99de97..c7727857 100644 --- a/sway/commands/exec_always.c +++ b/sway/commands/exec_always.c @@ -20,7 +20,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { char *tmp = NULL; if (strcmp((char*)*argv, "--no-startup-id") == 0) { - wlr_log(L_INFO, "exec switch '--no-startup-id' not supported, ignored."); + wlr_log(WLR_INFO, "exec switch '--no-startup-id' not supported, ignored."); if ((error = checkarg(argc - 1, "exec_always", EXPECTED_MORE_THAN, 0))) { return error; } @@ -35,11 +35,11 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { strncpy(cmd, tmp, sizeof(cmd) - 1); cmd[sizeof(cmd) - 1] = 0; free(tmp); - wlr_log(L_DEBUG, "Executing %s", cmd); + wlr_log(WLR_DEBUG, "Executing %s", cmd); int fd[2]; if (pipe(fd) != 0) { - wlr_log(L_ERROR, "Unable to create pipe for fork"); + wlr_log(WLR_ERROR, "Unable to create pipe for fork"); } pid_t pid, child; @@ -73,7 +73,7 @@ struct cmd_results *cmd_exec_always(int argc, char **argv) { // cleanup child process waitpid(pid, NULL, 0); if (child > 0) { - wlr_log(L_DEBUG, "Child process created with pid %d", child); + wlr_log(WLR_DEBUG, "Child process created with pid %d", child); // TODO: add PID to active workspace } else { return cmd_results_new(CMD_FAILURE, "exec_always", -- cgit v1.2.3