Skip to content

Commit 340cf49

Browse files
LeaYehLeaYeh
authored
Merge pull request #356 from LeaYeh/fix-sigpipe-builtin
2 parents 2fa4b0e + adb35f2 commit 340cf49

File tree

19 files changed

+89
-88
lines changed

19 files changed

+89
-88
lines changed

‎README.md

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ https://starchart.cc/LeaYeh/minishell
9595
| Cross-end | Signal | Signal Handling | Handled `ctrl-C` as `SIGINT` as bash behavior. | ✅ |
9696
| | | | Handled `ctrl-D` as `EOF` as bash behavior. | ✅ |
9797
| | | | Handled `ctrl-\\` as `SIGQUIT` as bash behavior. | ✅ |
98-
| | | Exception Handling | Ignored `SIGPIPE` in internal process and set it back as default in external commands sub-process. | ✅ |
98+
| | | Exception Handling | Handled `SIGPIPE` in internal process and set it back as default in external commands sub-process. | ✅ |
9999
| | | | Used `SIGUSR1` and `SIGTERM` to raise internal critical error to all related process and handle it depends on scenario. | ✅ |
100100

101101

@@ -552,9 +552,8 @@ Reference: https://tldp.org/LDP/abs/html/internal.html
552552
- Before executing an external command (`handle_external_cmd`), we reset the signal handlers to their default settings (`SIG_DEFAULT`). This ensures that the external command can handle signals according to its own logic without interference from the shell's signal handling.
553553
- By resetting the signal handlers, we allow the external command to have complete control over how it handles signals, providing flexibility and adherence to Unix signal handling conventions.
554554
4. Handling SIGPIPE Signal for Builtin Commands:
555-
- For builtin commands (`handle_builtin`), especially those involved in pipeline operations, we need to handle the `SIGPIPE` signal differently.
556-
- We set up the signal handler to ignore (`SIG_IGNORE`) the `SIGPIPE` signal. This prevents the shell from terminating if a builtin command attempts to write to a broken pipeline, which is the default behavior. Instead, the shell continues execution as expected.
557-
- Ignoring the `SIGPIPE` signal for builtin commands ensures seamless execution and prevents unintended termination due to broken pipes, which can occur during pipeline operations.
555+
- For builtin commands (`handle_builtin`), especially those involved in pipeline operations, we decided to handle the `SIGPIPE` signal ourselves to stay consistent with the 42 practice of no resource _leaks_ at exit.
556+
- We set up the signal handler to catch the `SIGPIPE` signal. This prevents the shell from terminating immediately if a builtin command attempts to write to a broken pipeline, which would be the default behavior of a `SIGPIPE` signal. Instead, the shell frees all its resources manually and then exits cleanly.
558557

559558
#### Exception Handling
560559

‎include/builtins.h

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,10 @@
1616
# include "defines.h"
1717

1818
int exec_env(char *env[]);
19-
int exec_echo(char *args[]);
20-
int exec_pwd(void);
21-
int exec_cd(char *args[], t_list **env_list);
22-
int exec_export(char *args[], t_list **env_list);
19+
int exec_echo(t_sh *shell, char *args[]);
20+
int exec_pwd(t_sh *shell);
21+
int exec_cd(t_sh *shell, char *args[], t_list **env_list);
22+
int exec_export(t_sh *shell, char *args[], t_list **env_list);
2323
int exec_unset(char *args[], t_list **env_list);
2424
void exec_exit(t_sh *shell, char *args[]);
2525
int exec_easter_egg(void);

‎include/cd.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,6 @@ char *get_target_dir(char *args[], t_list *env_list);
4646
int set_final_path(char **final_path, char **new_pwd, char *target_dir);
4747

4848
/* update_pwd_env.c */
49-
bool update_pwd_env(t_list **env_list, char *new_pwd);
49+
bool update_pwd_env(t_list **env_list, char **new_pwd);
5050

5151
#endif

‎include/defines.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -393,6 +393,7 @@ typedef struct s_shell
393393
t_list *token_list;
394394
t_list_d *cmd_table_list;
395395
t_fct *final_cmd_table;
396+
void *builtin_allocation;
396397
} t_sh;
397398

398399
#endif

‎include/export.h

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,6 @@
1616
# include "defines.h"
1717

1818
/* print_exported_env.c */
19-
int print_exported_env(t_list *env_list);
19+
int print_exported_env(t_sh *shell, t_list *env_list);
2020

2121
#endif

‎include/utils.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,7 @@ char *get_value_from_env(char *env[], char *key);
7171
bool is_key_in_env(char *env[], char *key);
7272

7373
bool append_env_node(
74-
t_list **env_list, char *key, char *value, t_expt export);
74+
t_list **env_list, char **key, char **value, t_expt export);
7575
void free_env_node(t_env *env);
7676
bool is_key_in_env_list(t_list *env_list, char *key);
7777
char *get_value_from_env_list(t_list *env_list, char *key);
@@ -82,7 +82,7 @@ bool process_str_to_env_list(
8282
char *str, t_list **env_list, t_expt export);
8383
void remove_env_node(t_list **env_list, char *key, char *value);
8484
bool replace_env_value(
85-
t_list *env_list, char *key, char *value, char **old_value);
85+
t_list *env_list, char *key, char **value, char **old_value);
8686

8787
/* Expansion utils */
8888
int expand_list(

‎source/backend/builtins/cd/cd.c

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -14,26 +14,26 @@
1414
#include "clean.h"
1515
#include "utils.h"
1616

17-
int exec_cd(char *args[], t_list **env_list)
17+
int exec_cd(t_sh *shell, char *args[], t_list **env_list)
1818
{
1919
char *final_path;
20-
char *new_pwd;
2120
int ret;
2221
char *target_dir;
2322

2423
target_dir = get_target_dir(args, *env_list);
2524
if (!target_dir)
2625
return (GENERAL_ERROR);
27-
ret = set_final_path(&final_path, &new_pwd, target_dir);
26+
ret = set_final_path(
27+
&final_path, (char **)&shell->builtin_allocation, target_dir);
2828
if (ret != SUCCESS)
2929
return (ret);
3030
if (chdir(final_path) == -1)
31-
return (free(final_path), free(new_pwd),
31+
return (free(final_path), ft_free_and_null(&shell->builtin_allocation),
3232
handle_cd_error(errno, target_dir));
3333
free(final_path);
3434
if (args[1] && ft_strcmp(args[1], "-") == 0)
3535
ft_printf("%s\n", target_dir);
36-
if (!update_pwd_env(env_list, new_pwd))
37-
return (free(new_pwd), MALLOC_ERROR);
36+
if (!update_pwd_env(env_list, (char **)&shell->builtin_allocation))
37+
return (ft_free_and_null(&shell->builtin_allocation), MALLOC_ERROR);
3838
return (EXIT_SUCCESS);
3939
}

‎source/backend/builtins/cd/env_pwd_update.c

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -13,16 +13,16 @@
1313
#include "cd.h"
1414
#include "utils.h"
1515

16-
static bool handle_existing_pwd(t_list **env_list, char *prev_pwd);
17-
static bool handle_non_existing_pwd(t_list **env_list, char *new_pwd);
16+
static bool handle_existing_pwd(t_list **env_list, char **prev_pwd);
17+
static bool handle_non_existing_pwd(t_list **env_list, char **new_pwd);
1818

19-
bool update_pwd_env(t_list **env_list, char *new_pwd)
19+
bool update_pwd_env(t_list **env_list, char **new_pwd)
2020
{
2121
char *prev_pwd;
2222

2323
if (replace_env_value(*env_list, "PWD", new_pwd, &prev_pwd))
2424
{
25-
if (!handle_existing_pwd(env_list, prev_pwd))
25+
if (!handle_existing_pwd(env_list, &prev_pwd))
2626
return (free(prev_pwd), false);
2727
}
2828
else
@@ -31,7 +31,7 @@ bool update_pwd_env(t_list **env_list, char *new_pwd)
3131
return (true);
3232
}
3333

34-
static bool handle_existing_pwd(t_list **env_list, char *prev_pwd)
34+
static bool handle_existing_pwd(t_list **env_list, char **prev_pwd)
3535
{
3636
char *key;
3737
char *prev_oldpwd;
@@ -43,20 +43,20 @@ static bool handle_existing_pwd(t_list **env_list, char *prev_pwd)
4343
key = ft_strdup("OLDPWD");
4444
if (!key)
4545
return (false);
46-
if (!append_env_node(env_list, key, prev_pwd, EXPORT_NO))
46+
if (!append_env_node(env_list, &key, prev_pwd, EXPORT_NO))
4747
return (free(key), false);
4848
}
4949
return (true);
5050
}
5151

52-
static bool handle_non_existing_pwd(t_list **env_list, char *new_pwd)
52+
static bool handle_non_existing_pwd(t_list **env_list, char **new_pwd)
5353
{
5454
char *key;
5555

5656
key = ft_strdup("PWD");
5757
if (!key)
5858
return (false);
59-
if (!append_env_node(env_list, key, new_pwd, EXPORT_NO))
59+
if (!append_env_node(env_list, &key, new_pwd, EXPORT_NO))
6060
return (free(key), false);
6161
remove_env_node(env_list, "OLDPWD", NULL);
6262
return (true);

‎source/backend/builtins/echo.c

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -16,22 +16,21 @@ static bool is_newline_option(char *args[], int *i);
1616
static char *combine_args(char *args[], bool end_with_newline);
1717
static int get_combined_args_len(char *args[], bool end_with_newline);
1818

19-
int exec_echo(char *args[])
19+
int exec_echo(t_sh *shell, char *args[])
2020
{
2121
int i;
2222
bool end_with_newline;
23-
char *combined_str;
2423

2524
i = 1;
2625
if (is_newline_option(args, &i))
2726
end_with_newline = false;
2827
else
2928
end_with_newline = true;
30-
combined_str = combine_args(args + i, end_with_newline);
31-
if (!combined_str)
29+
shell->builtin_allocation = combine_args(args + i, end_with_newline);
30+
if (!shell->builtin_allocation)
3231
return (MALLOC_ERROR);
33-
ft_printf("%s", combined_str);
34-
free(combined_str);
32+
ft_printf("%s", shell->builtin_allocation);
33+
ft_free_and_null(&shell->builtin_allocation);
3534
return (EXIT_SUCCESS);
3635
}
3736

‎source/backend/builtins/export/export.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,13 +16,13 @@
1616
static bool handle_var_export(char *str, t_list **env_list);
1717
static void change_export_flag(t_list *env_list, char *key, t_expt export);
1818

19-
int exec_export(char *args[], t_list **env_list)
19+
int exec_export(t_sh *shell, char *args[], t_list **env_list)
2020
{
2121
int i;
2222
int ret;
2323

2424
if (get_array_len(args) < 2)
25-
return (print_exported_env(*env_list));
25+
return (print_exported_env(shell, *env_list));
2626
ret = SUCCESS;
2727
i = 1;
2828
while (args[i])
@@ -51,7 +51,7 @@ static bool handle_var_export(char *str, t_list **env_list)
5151
{
5252
if (!extract_env_value(&value, str))
5353
return (free(key), false);
54-
if (value && replace_env_value(*env_list, key, value, &old_value))
54+
if (value && replace_env_value(*env_list, key, &value, &old_value))
5555
free(old_value);
5656
change_export_flag(*env_list, key, EXPORT_YES);
5757
free(key);

‎source/backend/builtins/export/export_output.c

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -32,24 +32,24 @@ static t_env *get_next_env_node(t_list *env_list, char *prev_key);
3232
* Malloc once, then use ft_snprintf to fill.
3333
* Then print once.
3434
*/
35-
int print_exported_env(t_list *env_list)
35+
int print_exported_env(t_sh *shell, t_list *env_list)
3636
{
37-
char *export_printout;
38-
int format_len;
39-
int prefix_len;
40-
int total_len;
37+
int format_len;
38+
int prefix_len;
39+
int total_len;
4140

4241
prefix_len = ft_strlen(EXPORT_PREFIX);
4342
format_len = ft_strlen("=\"\"\n");
4443
total_len = get_total_export_printout_len(env_list, prefix_len, format_len);
4544
if (total_len == 0)
4645
return (SUCCESS);
47-
export_printout = (char *)malloc(total_len + 1);
48-
if (!export_printout)
46+
shell->builtin_allocation = malloc(total_len + 1);
47+
if (!shell->builtin_allocation)
4948
return (MALLOC_ERROR);
50-
fill_export_printout(env_list, export_printout, prefix_len, format_len);
51-
ft_printf("%s", export_printout);
52-
free(export_printout);
49+
fill_export_printout(
50+
env_list, shell->builtin_allocation, prefix_len, format_len);
51+
ft_printf("%s", shell->builtin_allocation);
52+
ft_free_and_null(&shell->builtin_allocation);
5353
return (SUCCESS);
5454
}
5555

‎source/backend/builtins/pwd.c

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,10 @@
1212

1313
#include "utils.h"
1414

15-
int exec_pwd(void)
15+
int exec_pwd(t_sh *shell)
1616
{
17-
char *pwd;
18-
19-
pwd = getcwd(NULL, 0);
20-
if (!pwd)
17+
shell->builtin_allocation = getcwd(NULL, 0);
18+
if (!shell->builtin_allocation)
2119
{
2220
print_error("%s: %s: ", PROGRAM_NAME, "pwd");
2321
perror(NULL);
@@ -26,7 +24,7 @@ int exec_pwd(void)
2624
else
2725
return (GENERAL_ERROR);
2826
}
29-
ft_printf("%s\n", pwd);
30-
free(pwd);
27+
ft_printf("%s\n", shell->builtin_allocation);
28+
ft_free_and_null(&shell->builtin_allocation);
3129
return (EXIT_SUCCESS);
3230
}

‎source/backend/executor/builtin_cmd.c

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -87,27 +87,25 @@ static void exec_builtin_cmd(t_sh *shell)
8787
{
8888
t_fct *final_cmd_table;
8989

90-
setup_signal(SIGPIPE, SIG_IGNORE);
9190
final_cmd_table = shell->final_cmd_table;
9291
if (ft_strcmp(final_cmd_table->simple_cmd[0], "env") == 0)
9392
shell->exit_code = exec_env(final_cmd_table->env);
9493
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "unset") == 0)
9594
shell->exit_code = exec_unset(final_cmd_table->simple_cmd,
9695
&shell->env_list);
9796
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "echo") == 0)
98-
shell->exit_code = exec_echo(final_cmd_table->simple_cmd);
97+
shell->exit_code = exec_echo(shell, final_cmd_table->simple_cmd);
9998
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "pwd") == 0)
100-
shell->exit_code = exec_pwd();
99+
shell->exit_code = exec_pwd(shell);
101100
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "cd") == 0)
102-
shell->exit_code = exec_cd(final_cmd_table->simple_cmd,
103-
&shell->env_list);
101+
shell->exit_code = exec_cd(shell,
102+
final_cmd_table->simple_cmd, &shell->env_list);
104103
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "export") == 0)
105-
shell->exit_code = exec_export(final_cmd_table->simple_cmd,
106-
&shell->env_list);
104+
shell->exit_code = exec_export(shell,
105+
final_cmd_table->simple_cmd, &shell->env_list);
107106
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "exit") == 0)
108107
exec_exit(shell, final_cmd_table->simple_cmd);
109108
else if (ft_strcmp(final_cmd_table->simple_cmd[0], "~") == 0 && \
110109
shell->is_interactive)
111110
shell->exit_code = exec_easter_egg();
112-
setup_signal(SIGPIPE, SIG_STANDARD);
113111
}

‎source/backend/executor/external_cmd.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ static void reset_external_signal_handler(void)
6464
setup_signal(SIGTERM, SIG_DEFAULT);
6565
setup_signal(SIGUSR1, SIG_DEFAULT);
6666
setup_signal(SIGQUIT, SIG_DEFAULT);
67+
setup_signal(SIGPIPE, SIG_DEFAULT);
6768
}
6869

6970
static void handle_exec_error(t_sh *shell, char *exec_path)

‎source/shell/clean.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ static void clean_shell(t_sh *shell)
5252
ft_lstiter_d(shell->cmd_table_list, (void *)remove_heredoc_files);
5353
ft_lstclear_d(&shell->cmd_table_list, (void *)free_cmd_table);
5454
free_final_cmd_table(&shell->final_cmd_table);
55+
ft_free_and_null(&shell->builtin_allocation);
5556
rl_clear_history();
5657
}
5758

‎source/shell/default_env_list.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ static bool add_default_oldpwd_env_node(t_list **env_list)
4040
if (!key)
4141
return (false);
4242
value = NULL;
43-
if (!append_env_node(env_list, key, value, EXPORT_YES))
43+
if (!append_env_node(env_list, &key, &value, EXPORT_YES))
4444
return (free(key), false);
4545
return (true);
4646
}
@@ -56,7 +56,7 @@ static bool add_default_pwd_env_node(t_list **env_list)
5656
value = getcwd(NULL, 0);
5757
if (!value)
5858
return (free(key), false);
59-
if (!append_env_node(env_list, key, value, EXPORT_YES))
59+
if (!append_env_node(env_list, &key, &value, EXPORT_YES))
6060
return (free(key), free(value), false);
6161
return (true);
6262
}

‎source/shell/init.c

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,13 @@
1717

1818
bool init_shell(t_sh *shell)
1919
{
20+
ft_bzero(shell, sizeof(t_sh));
2021
shell->is_interactive = isatty(STDIN_FILENO);
2122
shell->pid = getpid_from_proc();
2223
shell->subshell_pid = -1;
23-
shell->subshell_level = 0;
24-
shell->signal_record = 0;
2524
init_pipe(&shell->old_pipe);
2625
init_pipe(&shell->new_pipe);
2726
shell->exit_code = EXIT_SUCCESS;
28-
shell->child_pid_list = NULL;
29-
shell->env_list = NULL;
30-
shell->token_list = NULL;
31-
shell->final_cmd_table = NULL;
32-
shell->cmd_table_list = NULL;
33-
shell->input_line = NULL;
3427
if (!setup_env_list(shell))
3528
return (false);
3629
handle_signal_std(0, NULL, shell);
@@ -39,5 +32,6 @@ bool init_shell(t_sh *shell)
3932
setup_signal(SIGTERM, SIG_STANDARD);
4033
setup_signal(SIGUSR1, SIG_STANDARD);
4134
setup_signal(SIGQUIT, SIG_IGNORE);
35+
setup_signal(SIGPIPE, SIG_STANDARD);
4236
return (true);
4337
}

0 commit comments

Comments
 (0)