From 93f3ea7c667de8bf4379795d6433b88327db8b80 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 29 Apr 2025 12:58:26 +0200 Subject: [PATCH] fix(exec): prevent leak when calling exit() in a subprocess This was because at the point at which exit is called, we can only free a t_simple_cmd, but not the whole t_cmd tree. This commit introduces a convention of returning a t_subprocess enum from each function in exec. If the current thread is a subprocess, SUBPROCESS is returned, else PARENTPROCESS. We also no longer call exit in subproceses. This way, all processes bubble up to main, where if SUBPROCESS is returned, all memory is freed and the program exits. This also removes the previous should_exit convention. Still TODO: handling of the exit builtin, which should also bubble up in the same way. --- src/executing/cmd/cmd_execute.c | 13 +++--- src/executing/cmd/cmd_execute.h | 4 +- src/executing/connec_cmd/connec_cmd_execute.c | 46 +++++++++++++------ src/executing/connec_cmd/connec_cmd_execute.h | 5 +- .../connec_cmd/connec_pipe_cmd_execute.c | 17 +++---- .../connec_cmd/connec_pipe_cmd_execute.h | 4 +- src/executing/group_cmd/group_cmd_execute.c | 19 ++++---- src/executing/group_cmd/group_cmd_execute.h | 4 +- src/executing/simple_cmd/simple_cmd_execute.c | 23 +++++----- src/executing/simple_cmd/simple_cmd_execute.h | 5 +- src/executing/simple_cmd/subprocess.c | 14 ++---- src/executing/simple_cmd/subprocess.h | 6 +-- src/get_command.c | 6 ++- src/minishell.c | 23 ++++++---- src/minishell.h | 8 +++- src/sig/sig_handlers.c | 3 +- 16 files changed, 111 insertions(+), 89 deletions(-) diff --git a/src/executing/cmd/cmd_execute.c b/src/executing/cmd/cmd_execute.c index 13ce16f..f56fc72 100644 --- a/src/executing/cmd/cmd_execute.c +++ b/src/executing/cmd/cmd_execute.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/04 19:26:37 by khais #+# #+# */ -/* Updated: 2025/04/16 16:44:24 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:10:47 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,14 +15,15 @@ #include "../connec_cmd/connec_cmd_execute.h" #include "../group_cmd/group_cmd_execute.h" -void cmd_execute(t_cmd *cmd, t_minishell *app, bool should_exit) +t_subprocess cmd_execute(t_cmd *cmd, t_minishell *app) { if (cmd == NULL) - return ; + return (PARENTPROCESS); if (cmd->type == FT_SIMPLE) - simple_cmd_execute(cmd->value.simple, app, should_exit); + return (simple_cmd_execute(cmd->value.simple, app)); if (cmd->type == FT_GROUP) - group_cmd_execute(cmd->value.group, app, should_exit); + return (group_cmd_execute(cmd->value.group, app)); if (cmd->type == FT_CONNECTION) - connec_cmd_execute(cmd->value.connection, app, should_exit); + return (connec_cmd_execute(cmd->value.connection, app)); + return (PARENTPROCESS); } diff --git a/src/executing/cmd/cmd_execute.h b/src/executing/cmd/cmd_execute.h index d413bb6..65708ee 100644 --- a/src/executing/cmd/cmd_execute.h +++ b/src/executing/cmd/cmd_execute.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/04 19:25:39 by khais #+# #+# */ -/* Updated: 2025/04/16 16:44:37 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:10:53 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,6 +15,6 @@ # include "../../minishell.h" -void cmd_execute(t_cmd *cmd, t_minishell *app, bool should_exit); +t_subprocess cmd_execute(t_cmd *cmd, t_minishell *app); #endif // CMD_EXECUTE_H diff --git a/src/executing/connec_cmd/connec_cmd_execute.c b/src/executing/connec_cmd/connec_cmd_execute.c index 6c5b6be..a22b5c3 100644 --- a/src/executing/connec_cmd/connec_cmd_execute.c +++ b/src/executing/connec_cmd/connec_cmd_execute.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/07 10:38:55 by khais #+# #+# */ -/* Updated: 2025/04/28 15:04:45 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:11:32 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -16,31 +16,47 @@ #include #include -static void connec_and_cmd_execute(t_connec_cmd *cmd, t_minishell *app) +static t_subprocess connec_and_cmd_execute(t_connec_cmd *cmd, t_minishell *app) { - cmd_execute(cmd->first, app, false); + int retvalue; + + retvalue = cmd_execute(cmd->first, app); + if (retvalue == SUBPROCESS) + return (SUBPROCESS); if (app->last_return_value == 0) - cmd_execute(cmd->second, app, false); + { + retvalue = cmd_execute(cmd->second, app); + if (retvalue == SUBPROCESS) + return (retvalue); + } + return (PARENTPROCESS); } -static void connec_or_cmd_execute(t_connec_cmd *cmd, t_minishell *app) +static t_subprocess connec_or_cmd_execute(t_connec_cmd *cmd, t_minishell *app) { - cmd_execute(cmd->first, app, false); + int retvalue; + + retvalue = cmd_execute(cmd->first, app); + if (retvalue == SUBPROCESS) + return (SUBPROCESS); if (app->last_return_value != 0) - cmd_execute(cmd->second, app, false); + { + retvalue = cmd_execute(cmd->second, app); + if (retvalue == SUBPROCESS) + return (SUBPROCESS); + } + return (PARENTPROCESS); } -void connec_cmd_execute(t_connec_cmd *cmd, t_minishell *app, - bool should_exit) +t_subprocess connec_cmd_execute(t_connec_cmd *cmd, t_minishell *app) { if (cmd == NULL) - return ; + return (PARENTPROCESS); if (cmd->connector == FT_AND) - connec_and_cmd_execute(cmd, app); + return (connec_and_cmd_execute(cmd, app)); if (cmd->connector == FT_OR) - connec_or_cmd_execute(cmd, app); + return (connec_or_cmd_execute(cmd, app)); if (cmd->connector == FT_PIPE) - connec_pipe_cmd_execute(cmd, app); - if (should_exit) - exit(app->last_return_value); + return (connec_pipe_cmd_execute(cmd, app)); + return (PARENTPROCESS); } diff --git a/src/executing/connec_cmd/connec_cmd_execute.h b/src/executing/connec_cmd/connec_cmd_execute.h index ac85083..944b3bd 100644 --- a/src/executing/connec_cmd/connec_cmd_execute.h +++ b/src/executing/connec_cmd/connec_cmd_execute.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/07 10:38:19 by khais #+# #+# */ -/* Updated: 2025/04/16 16:56:32 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:10:00 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,7 +15,6 @@ # include "../../minishell.h" -void connec_cmd_execute(t_connec_cmd *cmd, t_minishell *app, - bool should_exit); +t_subprocess connec_cmd_execute(t_connec_cmd *cmd, t_minishell *app); #endif // CONNEC_CMD_EXECUTE_H diff --git a/src/executing/connec_cmd/connec_pipe_cmd_execute.c b/src/executing/connec_cmd/connec_pipe_cmd_execute.c index c47bec6..74f204c 100644 --- a/src/executing/connec_cmd/connec_pipe_cmd_execute.c +++ b/src/executing/connec_cmd/connec_pipe_cmd_execute.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/11 12:01:29 by khais #+# #+# */ -/* Updated: 2025/04/28 15:04:17 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:11:23 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -46,27 +46,28 @@ static void close_and_wait(t_minishell *app, int pid1, int pid2, int pipefd[2]) do_waitpid(app, pid2); } -void connec_pipe_cmd_execute(t_connec_cmd *cmd, t_minishell *app) +t_subprocess connec_pipe_cmd_execute(t_connec_cmd *cmd, t_minishell *app) { int pid1; int pid2; int pipefd[2]; if (do_pipe(pipefd, app) < 0) - return ; + return (PARENTPROCESS); pid1 = fork(); if (pid1 == 0) { dup_and_close(pipefd[1], STDOUT_FILENO, pipefd); - cmd_execute(cmd->first, app, true); - return ; + cmd_execute(cmd->first, app); + return (SUBPROCESS); } pid2 = fork(); if (pid2 == 0) { dup_and_close(pipefd[0], STDIN_FILENO, pipefd); - cmd_execute(cmd->second, app, true); + cmd_execute(cmd->second, app); + return (SUBPROCESS); } - else - close_and_wait(app, pid1, pid2, pipefd); + close_and_wait(app, pid1, pid2, pipefd); + return (PARENTPROCESS); } diff --git a/src/executing/connec_cmd/connec_pipe_cmd_execute.h b/src/executing/connec_cmd/connec_pipe_cmd_execute.h index 7bababc..2ab8618 100644 --- a/src/executing/connec_cmd/connec_pipe_cmd_execute.h +++ b/src/executing/connec_cmd/connec_pipe_cmd_execute.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/11 11:56:45 by khais #+# #+# */ -/* Updated: 2025/04/28 15:04:23 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:09:42 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,6 +15,6 @@ # include "../../minishell.h" -void connec_pipe_cmd_execute(t_connec_cmd *cmd, t_minishell *app); +t_subprocess connec_pipe_cmd_execute(t_connec_cmd *cmd, t_minishell *app); #endif // CONNEC_PIPE_CMD_EXECUTE_H diff --git a/src/executing/group_cmd/group_cmd_execute.c b/src/executing/group_cmd/group_cmd_execute.c index addc97b..8cd8771 100644 --- a/src/executing/group_cmd/group_cmd_execute.c +++ b/src/executing/group_cmd/group_cmd_execute.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/04 19:50:42 by khais #+# #+# */ -/* Updated: 2025/04/28 12:56:52 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:11:00 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -17,22 +17,19 @@ #include "../simple_cmd/std_fds.h" #include -void group_cmd_execute(t_group_cmd *cmd, t_minishell *app, bool should_exit) +t_subprocess group_cmd_execute(t_group_cmd *cmd, t_minishell *app) { - int pid; - t_std_fds fds; + int pid; + t_std_fds fds; pid = fork(); if (pid == 0) { handle_redirections(cmd->redirects, app, &fds); - cmd_execute(cmd->cmd, app, true); + cmd_execute(cmd->cmd, app); restore_std_fds(&fds); + return (SUBPROCESS); } - else - { - do_waitpid(app, pid); - } - if (should_exit) - exit(app->last_return_value); + do_waitpid(app, pid); + return (PARENTPROCESS); } diff --git a/src/executing/group_cmd/group_cmd_execute.h b/src/executing/group_cmd/group_cmd_execute.h index a8c3c47..651e0b7 100644 --- a/src/executing/group_cmd/group_cmd_execute.h +++ b/src/executing/group_cmd/group_cmd_execute.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/04 19:42:36 by khais #+# #+# */ -/* Updated: 2025/04/16 16:45:40 by khais ### ########.fr */ +/* Updated: 2025/04/29 14:40:03 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,6 +15,6 @@ # include "../../minishell.h" -void group_cmd_execute(t_group_cmd *cmd, t_minishell *app, bool should_exit); +t_subprocess group_cmd_execute(t_group_cmd *cmd, t_minishell *app); #endif // GROUP_CMD_EXECUTE_H diff --git a/src/executing/simple_cmd/simple_cmd_execute.c b/src/executing/simple_cmd/simple_cmd_execute.c index 985cb33..5e6de5b 100644 --- a/src/executing/simple_cmd/simple_cmd_execute.c +++ b/src/executing/simple_cmd/simple_cmd_execute.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/03/27 16:21:56 by khais #+# #+# */ -/* Updated: 2025/04/28 15:57:23 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:10:35 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -72,47 +72,46 @@ static t_simple_cmd *post_process_command(t_simple_cmd *cmd, t_minishell *app) return (cmd); } -static void exec_external_cmd(t_simple_cmd *cmd, t_minishell *app, +static t_subprocess exec_external_cmd(t_simple_cmd *cmd, t_minishell *app, t_std_fds *fds) { char *exe; int pid; if (cmd->words == NULL) - return (restore_std_fds(fds)); + return (restore_std_fds(fds), PARENTPROCESS); exe = get_cmdpath(cmd->words->word->word, app); if (exe != NULL) { pid = fork(); if (pid == 0) - execute_subprocess(exe, cmd, app, fds); + return (execute_subprocess(exe, cmd, app, fds)); free(exe); do_waitpid(app, pid); } restore_std_fds(fds); if (exe == NULL) command_not_found(cmd, app); + return (PARENTPROCESS); } -void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app, - bool should_exit) +t_subprocess simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) { t_std_fds fds; if (cmd == NULL) - return ; + return (PARENTPROCESS); ft_errno(FT_ESUCCESS); if (post_process_command(cmd, app) == NULL) { if (ft_errno_get() != FT_ESUCCESS) ft_dprintf(STDERR_FILENO, "minishell: post-processing error\n"); - return ; + return (PARENTPROCESS); } simple_cmd_execute_debug(cmd, app); if (handle_redirections(cmd->redirections, app, &fds) == NULL) - return ; + return (PARENTPROCESS); if (execute_builtin(cmd, app, &fds) == BUILTIN_INVALID) - exec_external_cmd(cmd, app, &fds); - if (should_exit) - exit(app->last_return_value); + return (exec_external_cmd(cmd, app, &fds)); + return (PARENTPROCESS); } diff --git a/src/executing/simple_cmd/simple_cmd_execute.h b/src/executing/simple_cmd/simple_cmd_execute.h index befd501..4469ab6 100644 --- a/src/executing/simple_cmd/simple_cmd_execute.h +++ b/src/executing/simple_cmd/simple_cmd_execute.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/03 14:35:14 by khais #+# #+# */ -/* Updated: 2025/04/16 16:43:41 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:10:37 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,8 +15,7 @@ # include "../../minishell.h" -void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app, - bool should_exit); +t_subprocess simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app); typedef enum e_builtin_type { diff --git a/src/executing/simple_cmd/subprocess.c b/src/executing/simple_cmd/subprocess.c index 3f670c2..dcb3ee7 100644 --- a/src/executing/simple_cmd/subprocess.c +++ b/src/executing/simple_cmd/subprocess.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/02 18:19:23 by khais #+# #+# */ -/* Updated: 2025/04/28 12:48:18 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:14:32 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -71,15 +71,11 @@ static int ft_execve(char *exe, char **argv, char **envp) return (retvalue); } -void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app, - t_std_fds *fds) +t_subprocess execute_subprocess(char *exe, t_simple_cmd *cmd, + t_minishell *app, t_std_fds *fds) { - int retvalue; - std_fds_close(fds); - retvalue = ft_execve(exe, argv_from_wordlist(cmd->words), + app->last_return_value = ft_execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); - simple_cmd_destroy(cmd); - env_destroy(app->env); - exit(retvalue); + return (SUBPROCESS); } diff --git a/src/executing/simple_cmd/subprocess.h b/src/executing/simple_cmd/subprocess.h index a2bccbd..0580225 100644 --- a/src/executing/simple_cmd/subprocess.h +++ b/src/executing/simple_cmd/subprocess.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/02 18:17:48 by khais #+# #+# */ -/* Updated: 2025/04/28 12:42:06 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:14:09 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -15,7 +15,7 @@ # include "../../minishell.h" -void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app, - t_std_fds *fds); +t_subprocess execute_subprocess(char *exe, t_simple_cmd *cmd, + t_minishell *app, t_std_fds *fds); #endif // SUBPROCESS_H diff --git a/src/get_command.c b/src/get_command.c index 76acb82..6f3af80 100644 --- a/src/get_command.c +++ b/src/get_command.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/02/19 18:03:11 by khais #+# #+# */ -/* Updated: 2025/04/09 14:03:39 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:04:59 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -67,7 +67,11 @@ char *get_command(t_minishell *app) char *line; if (isatty(STDIN_FILENO)) + { + if (app->debug) + ft_printf("pid: %d\n", getpid()); line = readline("$ "); + } else line = strip_newline(get_next_line(STDIN_FILENO)); if (line != NULL && line[0] != '\0') diff --git a/src/minishell.c b/src/minishell.c index dc24998..6cb82b7 100644 --- a/src/minishell.c +++ b/src/minishell.c @@ -3,10 +3,10 @@ /* ::: :::::::: */ /* minishell.c :+: :+: :+: */ /* +:+ +:+ +:+ */ -/* By: jguelen +#+ +:+ +#+ */ +/* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ -/* Created: 2025/02/06 13:44:06 by kcolin #+# #+# */ -/* Updated: 2025/04/24 14:26:57 by jguelen ### ########.fr */ +/* Created: 2025/04/29 15:13/45 by khais #+# #+# */ +/* Updated: 2025/04/29 15:13:45 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -29,13 +29,16 @@ /* ** execute command */ -static void execute_command(t_cmd *cmd, t_minishell *app) +static t_subprocess execute_command(t_cmd *cmd, t_minishell *app) { + int retvalue; + if (app->exec == false) - return ; + return (PARENTPROCESS); set_exec_mode_sig_handling(); - cmd_execute(cmd, app, false); + retvalue = cmd_execute(cmd, app); set_interactive_mode_sig_handling(); + return (retvalue); } static void debug_command(t_cmd *cmd, t_minishell *app) @@ -77,6 +80,7 @@ int main(int argc, char *argv[], char **envp) char *line; t_cmd *cmd; t_minishell app; + int retvalue; (void)argc; (void)argv; @@ -87,13 +91,12 @@ int main(int argc, char *argv[], char **envp) cmd = minishell_parse(&app, line); free(line); debug_command(cmd, &app); - execute_command(cmd, &app); + retvalue = execute_command(cmd, &app); cmd_destroy(cmd); + if (retvalue == SUBPROCESS) + break ; if (g_signum != 0) - { readline_reset(); - g_signum = 0; - } line = get_command(&app); } env_destroy(app.env); diff --git a/src/minishell.h b/src/minishell.h index e1dd053..015555d 100644 --- a/src/minishell.h +++ b/src/minishell.h @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/09 14:02:47 by khais #+# #+# */ -/* Updated: 2025/04/28 12:11:44 by khais ### ########.fr */ +/* Updated: 2025/04/29 14:30:32 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -20,6 +20,12 @@ # include # include +typedef enum e_subprocess +{ + SUBPROCESS, + PARENTPROCESS, +} t_subprocess; + typedef struct s_std_fds { int stdin; diff --git a/src/sig/sig_handlers.c b/src/sig/sig_handlers.c index 14850e1..588c831 100644 --- a/src/sig/sig_handlers.c +++ b/src/sig/sig_handlers.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/04/17 12:01:39 by khais #+# #+# */ -/* Updated: 2025/04/17 12:03:04 by khais ### ########.fr */ +/* Updated: 2025/04/29 15:13:42 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -36,6 +36,7 @@ void readline_reset(void) rl_replace_line("", 0); ft_printf("\n"); rl_redisplay(); + g_signum = 0; } /*