Exit with the intended status before the signal handler is installed - #138
Open
BKPepe wants to merge 1 commit into
Open
Exit with the intended status before the signal handler is installed#138BKPepe wants to merge 1 commit into
BKPepe wants to merge 1 commit into
Conversation
olsr_exit() ends the process with raise(SIGTERM) so that the orderly
shutdown in olsr_shutdown() runs, and falls through to exit(val) "in
case the signal handler was not setup yet". That fall-through is
unreachable: raise() with the default disposition terminates the
process, so exit(val) never executes.
main() calls olsr_exit() twenty times before signal(SIGTERM,
olsr_shutdown) is installed, so all of those paths report 143
instead of the value they pass:
$ olsrd -v; echo $?
*** olsr.org - pre-0.9.9-git_3653c47-hash_426688c ***
Terminated: 15
143
$ olsrd -f /nonexistent.conf; echo $?
Terminated: 15
143
The second one matters beyond cosmetics: a configuration error is
indistinguishable from a successful run for anything that checks the
exit status, and EXIT_FAILURE never reaches the caller.
Record whether the handler has been installed and only take the
signal path once it has. With this, -v and -h exit 0 and the early
error paths exit with EXIT_FAILURE; behaviour after the handler is
installed is unchanged.
Signed-off-by: Josef Schlehofer <pepe.schlehofer@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
olsr_exit()callsraise(SIGTERM)so thatolsr_shutdown()can perform an orderly shutdown, then falls through toexit(val)"in case the signal handler was not setup yet". With the defaultSIGTERMdisposition, however,raise()terminates the process, soexit(val)is never reached.main()callsolsr_exit()before installing theSIGTERMhandler, causing these paths to exit with 143 instead of their intended status. For example,olsrd -f /nonexistent.confcurrently exits with 143 instead ofEXIT_FAILURE.The patch tracks whether the handler has been installed and only uses the signal path afterwards. Behaviour after handler installation is unchanged.
Verified on macOS (
make/Makefile.osx):-vand-hnow exit 0, and an invalid configuration exits 1.Noticed while adding a CI version check for the OpenWrt package (openwrt/routing#1191).