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

Improve exit status #253

Merged
merged 7 commits into from Aug 3, 2019

Conversation

@droe
Copy link
Owner

commented Aug 2, 2019

This pull request is meant to address #252 in the following ways:

  • sslsplit will now propagate the privsep child process' exit status to the parent process
  • exit status will now be 0 for regular shutdown via SIGINT or SIGTERM
  • exit status will now be 128 + signal number if we shut down after catching a different signal that we consider a non-regular shutdown (currently only SIGHUP and SIGQUIT)
  • exit status 1..127 are used for error conditions such as config errors or runtime errors; currently exit status 1 is used for all errors but we could change that in the future to e.g. differentiate fatal config errors from runtime errors

Signals that are non-fatal, such as SIGUSR1, will not lead to a shutdown. Some signals not handled by sslsplit, such as SIGKILL or SIGSEGV, will lead to a termination by the kernel, meaning that the process interested in the exit status will use wait(2) to acquire the signal. In the special case of sslsplit running directly from a shell, most shells will translate the signal number to an exit status using the same scheme of 128 + signal number.

All of this is only relevant when not running sslsplit as a daemon.

@droe droe added the feature label Aug 2, 2019
@droe droe added this to the 0.5.5 milestone Aug 2, 2019
@droe droe requested a review from sonertari Aug 2, 2019
@droe droe self-assigned this Aug 2, 2019
@sonertari

This comment has been minimized.

Copy link
Collaborator

commented Aug 2, 2019

If I kill sslsplit with SIGINT or SIGTERM signals, it exits with status 1 (e.g. echo $?). But the second bullet claims that 0 should be returned. Is this expected? I guess I am missing something.

@droe

This comment has been minimized.

Copy link
Owner Author

commented Aug 2, 2019

You're not missing anything, I broke my change during refactoring. Fixed.

@sonertari

This comment has been minimized.

Copy link
Collaborator

commented Aug 2, 2019

Appendix E. Exit Codes With Special Meanings reads "Control-C is fatal error signal 2", so I guess they expect us to return 130 upon SIGINT. But I like it this way. As you wish.

Copy link
Collaborator

left a comment

lgtm now

@droe droe merged commit 08c9d3f into develop Aug 3, 2019
2 checks passed
2 checks passed
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
droe added a commit that referenced this pull request Aug 3, 2019
Issue:		#252
Pull req:	#253
Reviewed by:	@sonertari
@droe

This comment has been minimized.

Copy link
Owner Author

commented Aug 3, 2019

Thank you, Soner!

@droe droe deleted the exit-status branch Aug 3, 2019
@droe droe added released and removed merged-to-develop labels Aug 30, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.