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 a pipe issue and return code #619

Merged
merged 3 commits into from Apr 19, 2021
Merged

Conversation

gfphoenix78
Copy link
Contributor

@gfphoenix78 gfphoenix78 commented Mar 11, 2021

Between closing the pipe files and waiting the program to exit,
the program may write to the closed pipe files, which causes
the program got the signal SIGPIPE and exit abnormally.
A safer way is to close these pipe files after the program does
exit.

Another issue is to extract the return code. WEXITSTATUS(status)
should be only employed if WIFEXITED returned true. Similarly,
if WIFSIGNALED(status) is true, set the 128+signo as the return code.

@ghost
Copy link

ghost commented Mar 11, 2021

CLA assistant check
All CLA requirements met.

Between closing the pipe files and waiting the program to exit,
the program may write to the closed pipe files, which causes
the program got the signal SIGPIPE and exit abnormally.
A safer way is to close these pipe files after the program does
exit.

Another issue is to extract the return code. `WEXITSTATUS(status)`
should be only employed if WIFEXITED returned true. Similarly,
if `WIFSIGNALED(status)` is true, set the signo as the return code.
src/bin/lib/subcommands.c/runprogram.h Outdated Show resolved Hide resolved
@gfphoenix78
Copy link
Contributor Author

@DimCitus I've updated the code according to your suggestion. Could you take some time to review it?

@DimCitus DimCitus merged commit 3eaded2 into hapostgres:master Apr 19, 2021
@DimCitus
Copy link
Collaborator

Hi @gfphoenix78 ; I'm just back from some days off and merged your PR. Thanks for your contribution!

@DimCitus DimCitus added the bug Something isn't working label Apr 19, 2021
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
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants