Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[REFACTOR] Exit for SIGPIPE signal without leaks #356

Merged
merged 6 commits into from
Jul 12, 2024
Merged

Conversation

itislu
Copy link
Collaborator

@itislu itislu commented Jul 4, 2024

By setting up a signal handler that cleans and exits for SIGPIPE, we exit with the first write() to a broken pipe without trying over and over again.
To avoid memory leaks, allocations in builtins that have not been freed before a call to write() have to be safed into the shell struct so that they can be freed in the signal handler.


Also:

  • Use ft_bzero() to init shell struct to 0.
  • Shorten abort message.
  • Make environment list operation functions more memory save.
  • Fix sa.sa_flags not matching with the function pointers.

@itislu itislu added the enhancement Enhancement of an existing feature label Jul 4, 2024
@itislu itislu force-pushed the fix-sigpipe-builtin branch 8 times, most recently from f1a9198 to eba08ef Compare July 5, 2024 21:08
@itislu
Copy link
Collaborator Author

itislu commented Jul 5, 2024

Aahh it's fruststrating that the tester behaves differently in the GitHub Action environment...
I just cannot figure out why our minishell doesn't print the SIGPIPE error messages from external commands even though the signal handler for SIGPIPE gets reset to SIG_DFL.
I also cannot reproduce when bash prints them in the first place, because locally it never does even though the same version of bash is installed.

@itislu itislu force-pushed the fix-sigpipe-builtin branch 9 times, most recently from 0086930 to 9414521 Compare July 10, 2024 05:05
By setting up a signal handler that cleans and exits for SIGPIPE, we exit with the first `write()` to a broken pipe without trying over and over again.
To avoid memory leaks, allocations in builtins that have not been freed before a call to `write()` have to be safed into the `shell` struct so that they can be freed in the signal handler.
Make environment list operation functions more memory save.
@itislu
Copy link
Collaborator Author

itislu commented Jul 11, 2024

@LeaYeh Please check if the updates to the README about SIGPIPE are okay for you

@LeaYeh LeaYeh merged commit 340cf49 into main Jul 12, 2024
36 checks passed
@LeaYeh LeaYeh deleted the fix-sigpipe-builtin branch July 12, 2024 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Enhancement of an existing feature
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants