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

[FIX] Fix newline after program with exit code 130 #386

Merged
merged 5 commits into from
Nov 19, 2024
Merged

Conversation

itislu
Copy link
Collaborator

@itislu itislu commented Nov 14, 2024


Known issues

@itislu itislu added the bug Something isn't working label Nov 14, 2024
@itislu itislu added this to the Signal Handling milestone Nov 14, 2024
@itislu itislu marked this pull request as ready for review November 14, 2024 23:36
@itislu itislu marked this pull request as draft November 15, 2024 00:20
@itislu
Copy link
Collaborator Author

itislu commented Nov 15, 2024

Something is wrong with the CI shell scripts...

image


Bash also seems to put it to stderr.
Tested with:
Terminal 1 (stdout): `tty` -> `/dev/pts/*`
Terminal 2 (stderr): `bash >/dev/pts/*`
`./return_130 || echo 123` did not print `123`.
It could happen bc of a certain order of including headers that `_DEFAULT_SOURCE` was redefined.
@itislu itislu marked this pull request as ready for review November 15, 2024 16:40
@itislu itislu requested a review from LeaYeh November 15, 2024 16:40
Copy link
Owner

@LeaYeh LeaYeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe need to check tomorrow

source/backend/executor/wait_process.c Show resolved Hide resolved
source/backend/executor/wait_process.c Show resolved Hide resolved
Copy link
Owner

@LeaYeh LeaYeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get the point why remove the sig checking

@LeaYeh
Copy link
Owner

LeaYeh commented Nov 16, 2024

And I feel sometimes you over norminattee make the code confusing me.
For example, is the wait child pid and different than wait pid? Why need a sub function to take the function jump but not gain any benefit?

@itislu
Copy link
Collaborator Author

itislu commented Nov 16, 2024

I'm happy to do a code review on campus whenever you have time!

@itislu itislu requested a review from LeaYeh November 19, 2024 17:36
@LeaYeh LeaYeh merged commit cc9d9dc into main Nov 19, 2024
41 checks passed
@itislu itislu deleted the fix-issue-257 branch November 19, 2024 22:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

[BUG] minishell running in minishell prints too many newlines after exiting, also in subshells
2 participants