From 1df6a6ad9a28a8315dad0f1830570b393f8e1eb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 1 Apr 2025 16:28:06 +0200 Subject: [PATCH 1/5] simple_cmd executing: show error in case execve fails We don't have to check the return status, since execve only returns on failure. Fixes #52 --- src/executing/simple_cmd/simple_cmd_execute.c | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/src/executing/simple_cmd/simple_cmd_execute.c b/src/executing/simple_cmd/simple_cmd_execute.c index e483186..6bcba01 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/01 16:37:31 by khais ### ########.fr */ +/* Updated: 2025/04/01 18:08:02 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -17,6 +17,8 @@ #include "../../env/env_convert.h" #include #include +#include +#include "../../subst/path_split.h" static char **argv_from_wordlist(t_wordlist *wordlist) { @@ -39,6 +41,24 @@ static void command_not_found(t_simple_cmd *cmd) cmd->words->word->word); } +static void ft_execve(char *exe, char **argv, char **envp) +{ + execve(exe, argv, envp); + ft_dprintf(STDERR_FILENO, "minishell: %s: ", argv[0]); + perror(NULL); + free(exe); + path_split_destroy(argv); + path_split_destroy(envp); +} + +static void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app) +{ + ft_execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); + simple_cmd_destroy(cmd); + env_destroy(app->env); + exit(127); +} + void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) { char *exe; @@ -56,7 +76,7 @@ void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) } pid = fork(); if (pid == 0) - execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); + execute_subprocess(exe, cmd, app); free(exe); waitpid(pid, NULL, 0); return ; From e84b88394aa58b5ee4caccf59aea8967fe197119 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Tue, 1 Apr 2025 18:38:31 +0200 Subject: [PATCH 2/5] fix: double free when getting command path for empty command ``` minishell$ "" ``` leads to the following error: ``` ================================================================= ==1158770==ERROR: AddressSanitizer: attempting double-free on 0x5060000020c0 in thread T0: #0 0x5649fcb5b8a8 in free.part.0 asan_malloc_linux.cpp.o #1 0x5649fcbac549 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:119:3 #2 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #3 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #4 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #5 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #6 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #7 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) #8 0x7fb4f2dcd338 in __libc_start_main@GLIBC_2.2.5 (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a338) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) #9 0x5649fca6f394 in _start (/home/khais/src/42/common_env/minishell/minishell+0x2b394) 0x5060000020c0 is located 0 bytes inside of 58-byte region [0x5060000020c0,0x5060000020fa) freed by thread T0 here: #0 0x5649fcb5b8a8 in free.part.0 asan_malloc_linux.cpp.o #1 0x5649fcbac669 in select_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:81:3 #2 0x5649fcbac524 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:116:9 #3 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #4 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #5 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #6 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #7 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #8 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) previously allocated by thread T0 here: #0 0x5649fcb5c897 in malloc (/home/khais/src/42/common_env/minishell/minishell+0x118897) #1 0x5649fcbac6be in alloc_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp_utils.c:34:9 #2 0x5649fcbac4e9 in deal_with_filled_path /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:113:15 #3 0x5649fcbac38b in filepath_from_env /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:165:10 #4 0x5649fcbac220 in get_cmdpath /home/khais/src/42/common_env/minishell/src/subst/simple_filename_exp.c:186:10 #5 0x5649fcba8664 in simple_cmd_execute /home/khais/src/42/common_env/minishell/src/executing/simple_cmd/simple_cmd_execute.c:71:8 #6 0x5649fcba6e4c in execute_command /home/khais/src/42/common_env/minishell/src/minishell.c:49:2 #7 0x5649fcba6ce5 in main /home/khais/src/42/common_env/minishell/src/minishell.c:92:3 #8 0x7fb4f2dcd27d in __libc_start_call_main (/nix/store/nqb2ns2d1lahnd5ncwmn6k84qfd7vx2k-glibc-2.40-36/lib/libc.so.6+0x2a27d) (BuildId: 704cab5816f130c494208e8cc24d87a386ef085b) SUMMARY: AddressSanitizer: double-free asan_malloc_linux.cpp.o in free.part.0 ==1158770==ABORTING ``` Looks like select_path frees filepath sometimes, no idea why. Removing these lines didn't break any tests. I'm pushing this to get a second opinion --- src/subst/simple_filename_exp.c | 4 +--- test.sh | 7 +++++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/src/subst/simple_filename_exp.c b/src/subst/simple_filename_exp.c index 3e1451b..0f8bd66 100644 --- a/src/subst/simple_filename_exp.c +++ b/src/subst/simple_filename_exp.c @@ -6,7 +6,7 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/03/02 13:40:10 by jguelen #+# #+# */ -/* Updated: 2025/03/27 18:39:23 by khais ### ########.fr */ +/* Updated: 2025/04/01 18:38:35 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -77,8 +77,6 @@ static char *select_path(char *filepath, char **oldpath, char **path, else return (free(*oldpath), path_split_destroy(path), filepath); } - if (*oldpath != filepath) - free(filepath); return (NULL); } diff --git a/test.sh b/test.sh index e6fcefd..2b647bc 100755 --- a/test.sh +++ b/test.sh @@ -335,6 +335,13 @@ status=1 hello=hi EOF +when_run < Date: Wed, 2 Apr 2025 14:15:30 +0200 Subject: [PATCH 3/5] fix: do not read ahead in STDIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Open Group Base Specifications Issue 8 IEEE Std 1003.1-2024 sh — shell, the standard command language interpreter says: > When the shell is using standard input and it invokes a command that also uses > standard input, the shell shall ensure that the standard input file pointer > points directly after the command it has read when the command begins > execution. It shall not read ahead in such a manner that any characters > intended to be read by the invoked command are consumed by the shell (whether > interpreted by the shell or not) or that characters that are not read by the > invoked command are not seen by the shell. We used the default BUFFER_SIZE for get_next_line of 1024, which caused us to read ahead farther than was allowed by the Open Group Base Specification. Setting BUFFER_SIZE=1 ensures that we don't read too far ahead, since get_next_line will always immediatly stop once a newline is found. This is for me the simplest way to solve this issue. --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index 36d2a9e..b91baec 100644 --- a/Makefile +++ b/Makefile @@ -100,6 +100,7 @@ all: $(NAME) $(NAME): $(minishell_objs) $(LIBFT) $(CC) $(CFLAGS) -o $@ $(minishell_objs) $(LINCLUDE) $(LDLIBS) +$(LIBFT): CFLAGS+=-DBUFFER_SIZE=1 $(LIBFT): +$(MAKE) -C $(LIBFTDIR) From 3661feefaa992bd9af2a23f386be191c96ee826a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Wed, 2 Apr 2025 18:17:37 +0200 Subject: [PATCH 4/5] simple_cmd refactor: put subprocess functions in own file This clears some space for new functions later --- Makefile | 1 + src/executing/simple_cmd/simple_cmd_execute.c | 38 +------------- src/executing/simple_cmd/subprocess.c | 50 +++++++++++++++++++ src/executing/simple_cmd/subprocess.h | 20 ++++++++ 4 files changed, 73 insertions(+), 36 deletions(-) create mode 100644 src/executing/simple_cmd/subprocess.c create mode 100644 src/executing/simple_cmd/subprocess.h diff --git a/Makefile b/Makefile index b91baec..5ac383a 100644 --- a/Makefile +++ b/Makefile @@ -36,6 +36,7 @@ srcs = \ src/executing/simple_cmd/builtin_pwd.c \ src/executing/simple_cmd/builtins.c \ src/executing/simple_cmd/simple_cmd_execute.c \ + src/executing/simple_cmd/subprocess.c \ src/ft_errno.c \ src/get_command.c \ src/parser/cmdgroup/cmdgroup.c \ diff --git a/src/executing/simple_cmd/simple_cmd_execute.c b/src/executing/simple_cmd/simple_cmd_execute.c index 6bcba01..65612bf 100644 --- a/src/executing/simple_cmd/simple_cmd_execute.c +++ b/src/executing/simple_cmd/simple_cmd_execute.c @@ -6,34 +6,18 @@ /* By: khais +#+ +:+ +#+ */ /* +#+#+#+#+#+ +#+ */ /* Created: 2025/03/27 16:21:56 by khais #+# #+# */ -/* Updated: 2025/04/01 18:08:02 by khais ### ########.fr */ +/* Updated: 2025/04/02 18:19:57 by khais ### ########.fr */ /* */ /* ************************************************************************** */ #include "simple_cmd_execute.h" #include "builtins.h" +#include "subprocess.h" #include "libft.h" #include "../../subst/subst.h" -#include "../../env/env_convert.h" #include #include #include -#include "../../subst/path_split.h" - -static char **argv_from_wordlist(t_wordlist *wordlist) -{ - char **out; - int i; - - out = ft_calloc(wordlist_size(wordlist) + 1, sizeof(char *)); - i = 0; - while (wordlist != NULL) - { - out[i++] = ft_strdup(wordlist->word->word); - wordlist = wordlist->next; - } - return (out); -} static void command_not_found(t_simple_cmd *cmd) { @@ -41,24 +25,6 @@ static void command_not_found(t_simple_cmd *cmd) cmd->words->word->word); } -static void ft_execve(char *exe, char **argv, char **envp) -{ - execve(exe, argv, envp); - ft_dprintf(STDERR_FILENO, "minishell: %s: ", argv[0]); - perror(NULL); - free(exe); - path_split_destroy(argv); - path_split_destroy(envp); -} - -static void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app) -{ - ft_execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); - simple_cmd_destroy(cmd); - env_destroy(app->env); - exit(127); -} - void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) { char *exe; diff --git a/src/executing/simple_cmd/subprocess.c b/src/executing/simple_cmd/subprocess.c new file mode 100644 index 0000000..9d7b882 --- /dev/null +++ b/src/executing/simple_cmd/subprocess.c @@ -0,0 +1,50 @@ +/* ************************************************************************** */ +/* */ +/* ::: :::::::: */ +/* subprocess.c :+: :+: :+: */ +/* +:+ +:+ +:+ */ +/* By: khais +#+ +:+ +#+ */ +/* +#+#+#+#+#+ +#+ */ +/* Created: 2025/04/02 18:19:23 by khais #+# #+# */ +/* Updated: 2025/04/02 18:20:54 by khais ### ########.fr */ +/* */ +/* ************************************************************************** */ + +#include "subprocess.h" +#include "../../env/env_convert.h" +#include "../../parser/simple_cmd/simple_cmd.h" +#include "../../subst/path_split.h" +#include + +static char **argv_from_wordlist(t_wordlist *wordlist) +{ + char **out; + int i; + + out = ft_calloc(wordlist_size(wordlist) + 1, sizeof(char *)); + i = 0; + while (wordlist != NULL) + { + out[i++] = ft_strdup(wordlist->word->word); + wordlist = wordlist->next; + } + return (out); +} + +static void ft_execve(char *exe, char **argv, char **envp) +{ + execve(exe, argv, envp); + ft_dprintf(STDERR_FILENO, "minishell: %s: ", argv[0]); + perror(NULL); + free(exe); + path_split_destroy(argv); + path_split_destroy(envp); +} + +void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app) +{ + ft_execve(exe, argv_from_wordlist(cmd->words), envp_from_env(app->env)); + simple_cmd_destroy(cmd); + env_destroy(app->env); + exit(127); +} diff --git a/src/executing/simple_cmd/subprocess.h b/src/executing/simple_cmd/subprocess.h new file mode 100644 index 0000000..3e877d4 --- /dev/null +++ b/src/executing/simple_cmd/subprocess.h @@ -0,0 +1,20 @@ +/* ************************************************************************** */ +/* */ +/* ::: :::::::: */ +/* subprocess.h :+: :+: :+: */ +/* +:+ +:+ +:+ */ +/* By: khais +#+ +:+ +#+ */ +/* +#+#+#+#+#+ +#+ */ +/* Created: 2025/04/02 18:17:48 by khais #+# #+# */ +/* Updated: 2025/04/02 18:21:30 by khais ### ########.fr */ +/* */ +/* ************************************************************************** */ + +#ifndef SUBPROCESS_H +# define SUBPROCESS_H + +# include "../../minishell.h" + +void execute_subprocess(char *exe, t_simple_cmd *cmd, t_minishell *app); + +#endif // SUBPROCESS_H From f4e9955f75ebde861eb228bcafaaf01a47cc72f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kha=C3=AFs=20COLIN?= Date: Wed, 2 Apr 2025 15:26:11 +0200 Subject: [PATCH 5/5] exec: retain exit status of commands, including if they were signaled --- src/executing/simple_cmd/simple_cmd_execute.c | 18 +++++++++++++++--- test.sh | 15 +++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/executing/simple_cmd/simple_cmd_execute.c b/src/executing/simple_cmd/simple_cmd_execute.c index 65612bf..ad6d5f0 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/02 18:19:57 by khais ### ########.fr */ +/* Updated: 2025/04/02 18:45:47 by khais ### ########.fr */ /* */ /* ************************************************************************** */ @@ -18,13 +18,25 @@ #include #include #include - +#include + static void command_not_found(t_simple_cmd *cmd) { ft_dprintf(STDERR_FILENO, "minishell: %s: command not found\n", cmd->words->word->word); } +static void do_waitpid(t_minishell *app, int pid) +{ + int wstatus; + + waitpid(pid, &wstatus, 0); + if (WIFEXITED(wstatus)) + app->last_return_value = WEXITSTATUS(wstatus); + if (WIFSIGNALED(wstatus)) + app->last_return_value = 128 + WTERMSIG(wstatus); +} + void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) { char *exe; @@ -44,6 +56,6 @@ void simple_cmd_execute(t_simple_cmd *cmd, t_minishell *app) if (pid == 0) execute_subprocess(exe, cmd, app); free(exe); - waitpid(pid, NULL, 0); + do_waitpid(app, pid); return ; } diff --git a/test.sh b/test.sh index 2b647bc..18e4bf6 100755 --- a/test.sh +++ b/test.sh @@ -342,6 +342,21 @@ expecting <