Compare commits

...

4 commits

Author SHA1 Message Date
411ceaf367
simple_cmd refactor: put subprocess functions in own file
This clears some space for new functions later
2025-04-02 18:22:14 +02:00
52e109d0b5
fix: do not read ahead in STDIN
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.
2025-04-02 14:57:09 +02:00
77015ce27e
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
2025-04-01 18:41:32 +02:00
2fc47345f0
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
2025-04-01 18:08:34 +02:00
6 changed files with 84 additions and 21 deletions

View file

@ -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 \
@ -100,6 +101,7 @@ all: $(NAME)
$(NAME): $(minishell_objs) $(LIBFT)
$(CC) $(CFLAGS) -o $@ $(minishell_objs) $(LINCLUDE) $(LDLIBS)
$(LIBFT): CFLAGS+=-DBUFFER_SIZE=1
$(LIBFT):
+$(MAKE) -C $(LIBFTDIR)

View file

@ -6,32 +6,18 @@
/* By: khais <marvin@42.fr> +#+ +:+ +#+ */
/* +#+#+#+#+#+ +#+ */
/* Created: 2025/03/27 16:21:56 by khais #+# #+# */
/* Updated: 2025/04/01 16:37:31 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 <unistd.h>
#include <sys/wait.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);
}
#include <stdio.h>
static void command_not_found(t_simple_cmd *cmd)
{
@ -56,7 +42,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 ;

View file

@ -0,0 +1,50 @@
/* ************************************************************************** */
/* */
/* ::: :::::::: */
/* subprocess.c :+: :+: :+: */
/* +:+ +:+ +:+ */
/* By: khais <marvin@42.fr> +#+ +:+ +#+ */
/* +#+#+#+#+#+ +#+ */
/* 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 <stdio.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 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);
}

View file

@ -0,0 +1,20 @@
/* ************************************************************************** */
/* */
/* ::: :::::::: */
/* subprocess.h :+: :+: :+: */
/* +:+ +:+ +:+ */
/* By: khais <marvin@42.fr> +#+ +:+ +#+ */
/* +#+#+#+#+#+ +#+ */
/* 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

View file

@ -6,7 +6,7 @@
/* By: khais <marvin@42.fr> +#+ +:+ +#+ */
/* +#+#+#+#+#+ +#+ */
/* 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);
}

View file

@ -335,6 +335,13 @@ status=1
hello=hi
EOF
when_run <<EOF "empty command is not found"
""
EOF
expecting <<EOF
minishell: : command not found
EOF
when_run <<EOF "quoted parentheses are not operators"
echo unclosed '('
EOF