[pacman-dev] [PATCH] Revamp pacman signal handler
* All errors now go to stdout, so do the same here and simplify the writing of the error message. * Add SIGHUP to the handled signal list, and don't repeat code. * Attempt to release the transaction (e.g. remove the lock file) for all of HUP, INT, and TERM. Signals HUP and INT respects transaction state, TERM will immediately terminate the process. Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/pacman.c | 46 +++++++++++++++++++++++----------------------- 1 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 061d593..40aa444 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -299,26 +299,29 @@ static void handler(int signum) { int out = fileno(stdout); int err = fileno(stderr); + const char *msg; if(signum == SIGSEGV) { - const char *msg1 = "error: segmentation fault\n"; - const char *msg2 = "Internal pacman error: Segmentation fault.\n" + msg = "\nerror: segmentation fault\n" "Please submit a full bug report with --debug if appropriate.\n"; - /* write a error message to out, the rest to err */ - xwrite(out, msg1, strlen(msg1)); - xwrite(err, msg2, strlen(msg2)); + xwrite(err, msg, strlen(msg)); exit(signum); - } else if(signum == SIGINT) { - const char *msg = "\nInterrupt signal received\n"; + } else if(signum == SIGINT || signum == SIGHUP) { + if(signum == SIGINT) { + msg = "\nInterrupt signal received\n"; + } else { + msg = "\nHangup signal received\n"; + } xwrite(err, msg, strlen(msg)); if(alpm_trans_interrupt(config->handle) == 0) { /* a transaction is being interrupted, don't exit pacman yet. */ return; } - /* no commiting transaction, we can release it now and then exit pacman */ - alpm_trans_release(config->handle); - /* output a newline to be sure we clear any line we may be on */ - xwrite(out, "\n", 1); } + /* SIGINT: no commiting transaction, release it now and then exit pacman + * SIGHUP, SIGTERM: release no matter what */ + alpm_trans_release(config->handle); + /* output a newline to be sure we clear any line we may be on */ + xwrite(out, "\n", 1); cleanup(128 + signum); } @@ -757,8 +760,9 @@ static void cl_to_log(int argc, char* argv[]) */ int main(int argc, char *argv[]) { - int ret = 0; + int i, ret = 0; struct sigaction new_action, old_action; + const int signals[] = { SIGHUP, SIGINT, SIGTERM, SIGSEGV }; #if defined(HAVE_GETEUID) && !defined(CYGWIN) /* geteuid undefined in CYGWIN */ uid_t myuid = geteuid(); @@ -775,17 +779,13 @@ int main(int argc, char *argv[]) sigemptyset(&new_action.sa_mask); new_action.sa_flags = 0; - sigaction(SIGINT, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(SIGINT, &new_action, NULL); - } - sigaction(SIGTERM, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(SIGTERM, &new_action, NULL); - } - sigaction(SIGSEGV, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(SIGSEGV, &new_action, NULL); + /* assign our handler to any signals we care about */ + for(i = 0; i < sizeof(signals) / sizeof(signals[0]); i++) { + int signal = signals[i]; + sigaction(signal, NULL, &old_action); + if(old_action.sa_handler != SIG_IGN) { + sigaction(signal, &new_action, NULL); + } } /* i18n init */ -- 1.7.6.3
On Sep 26, 2011 4:29 PM, "Dan McGee" <dan@archlinux.org> wrote:
* All errors now go to stdout
writing of the error message. * Add SIGHUP to the handled signal list, and don't repeat code. * Attempt to release the transaction (e.g. remove the lock file) for all of HUP, INT, and TERM. Signals HUP and INT respects transaction state, TERM will immediately terminate the process.
Signed-off-by: Dan McGee <dan@archlinux.org> --- src/pacman/pacman.c | 46 +++++++++++++++++++++++----------------------- 1 files changed, 23 insertions(+), 23 deletions(-)
diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 061d593..40aa444 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -299,26 +299,29 @@ static void handler(int signum) { int out = fileno(stdout); int err = fileno(stderr); + const char *msg; if(signum == SIGSEGV) { - const char *msg1 = "error: segmentation fault\n"; - const char *msg2 = "Internal pacman error: Segmentation fault.\n" + msg = "\nerror: segmentation fault\n" "Please submit a full bug report with --debug if appropriate.\n"; - /* write a error message to out, the rest to err */ - xwrite(out, msg1, strlen(msg1)); - xwrite(err, msg2, strlen(msg2)); + xwrite(err, msg, strlen(msg)); exit(signum); - } else if(signum == SIGINT) { - const char *msg = "\nInterrupt signal received\n"; + } else if(signum == SIGINT || signum == SIGHUP) { + if(signum == SIGINT) { + msg = "\nInterrupt signal received\n"; + } else { + msg = "\nHangup signal received\n"; + } xwrite(err, msg, strlen(msg)); if(alpm_trans_interrupt(config->handle) == 0) { /* a transaction is being interrupted, don't exit
I think you meant stderr here. Looks good otherwise. Do we want to handle a SIGQUIT which is easily accessible from a shell (ctrl-/)? D , so do the same here and simplify the pacman yet. */
return; } - /* no commiting transaction, we can release it now and
- alpm_trans_release(config->handle); - /* output a newline to be sure we clear any line we may be on */ - xwrite(out, "\n", 1); } + /* SIGINT: no commiting transaction, release it now and then exit
then exit pacman */ pacman
+ * SIGHUP, SIGTERM: release no matter what */ + alpm_trans_release(config->handle); + /* output a newline to be sure we clear any line we may be on */ + xwrite(out, "\n", 1); cleanup(128 + signum); }
@@ -757,8 +760,9 @@ static void cl_to_log(int argc, char* argv[]) */ int main(int argc, char *argv[]) { - int ret = 0; + int i, ret = 0; struct sigaction new_action, old_action; + const int signals[] = { SIGHUP, SIGINT, SIGTERM, SIGSEGV }; #if defined(HAVE_GETEUID) && !defined(CYGWIN) /* geteuid undefined in CYGWIN */ uid_t myuid = geteuid(); @@ -775,17 +779,13 @@ int main(int argc, char *argv[]) sigemptyset(&new_action.sa_mask); new_action.sa_flags = 0;
- sigaction(SIGINT, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(SIGINT, &new_action, NULL); - } - sigaction(SIGTERM, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(SIGTERM, &new_action, NULL); - } - sigaction(SIGSEGV, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(SIGSEGV, &new_action, NULL); + /* assign our handler to any signals we care about */ + for(i = 0; i < sizeof(signals) / sizeof(signals[0]); i++) { + int signal = signals[i]; + sigaction(signal, NULL, &old_action); + if(old_action.sa_handler != SIG_IGN) { + sigaction(signal, &new_action, NULL); + } }
/* i18n init */ -- 1.7.6.3
On Mon, Sep 26, 2011 at 3:59 PM, dave reisner <d@falconindy.com> wrote:
On Sep 26, 2011 4:29 PM, "Dan McGee" <dan@archlinux.org> wrote:
* All errors now go to stdout
I think you meant stderr here. Yes, thanks, updated the message.
Looks good otherwise. Do we want to handle a SIGQUIT which is easily accessible from a shell (ctrl-/)? man 7 signal lists QUIT as a 'Core' as opposed to 'Term' signal, so we might want to leave at least one way of escaping and not calling our signal handler.
-Dan
participants (3)
-
Dan McGee
-
Dan McGee
-
dave reisner