[pacman-dev] [PATCH 01/11] handle_unlock: return 0 if lockfile == NULL
Returning -1 is useless since we don't provide any way to determine why it failed. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/handle.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index 98420b0..b2a10c8 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -129,7 +129,7 @@ int _alpm_handle_lock(alpm_handle_t *handle) /** Remove a lock file */ int _alpm_handle_unlock(alpm_handle_t *handle) { - ASSERT(handle->lockfile != NULL, return -1); + ASSERT(handle->lockfile != NULL, return 0); ASSERT(handle->lockfd >= 0, return 0); close(handle->lockfd); -- 2.6.3
alpm_unlock is a limited version of alpm_release that does nothing but the actual unlinking of the lock file and is therefore safe to call from signal handlers. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- lib/libalpm/alpm.h | 1 + lib/libalpm/handle.c | 19 +++++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) diff --git a/lib/libalpm/alpm.h b/lib/libalpm/alpm.h index 3049f2f..6cbcd24 100644 --- a/lib/libalpm/alpm.h +++ b/lib/libalpm/alpm.h @@ -1559,6 +1559,7 @@ char *alpm_compute_sha256sum(const char *filename); alpm_handle_t *alpm_initialize(const char *root, const char *dbpath, alpm_errno_t *err); int alpm_release(alpm_handle_t *handle); +int alpm_unlock(alpm_handle_t *handle); enum alpm_caps { ALPM_CAPABILITY_NLS = (1 << 0), diff --git a/lib/libalpm/handle.c b/lib/libalpm/handle.c index b2a10c8..0d8e292 100644 --- a/lib/libalpm/handle.c +++ b/lib/libalpm/handle.c @@ -126,8 +126,13 @@ int _alpm_handle_lock(alpm_handle_t *handle) return (handle->lockfd >= 0 ? 0 : -1); } -/** Remove a lock file */ -int _alpm_handle_unlock(alpm_handle_t *handle) +/** Remove the database lock file + * @param handle the context handle + * @return 0 on success, -1 on error + * + * @note Safe to call from inside signal handlers. + */ +int SYMEXPORT alpm_unlock(alpm_handle_t *handle) { ASSERT(handle->lockfile != NULL, return 0); ASSERT(handle->lockfd >= 0, return 0); @@ -136,6 +141,15 @@ int _alpm_handle_unlock(alpm_handle_t *handle) handle->lockfd = -1; if(unlink(handle->lockfile) != 0) { + RET_ERR(handle, ALPM_ERR_SYSTEM, -1); + } else { + return 0; + } +} + +int _alpm_handle_unlock(alpm_handle_t *handle) +{ + if(alpm_unlock(handle) != 0) { if(errno == ENOENT) { _alpm_log(handle, ALPM_LOG_WARNING, _("lock file missing %s\n"), handle->lockfile); @@ -150,6 +164,7 @@ int _alpm_handle_unlock(alpm_handle_t *handle) return -1; } } + return 0; } -- 2.6.3
Memory allocation/deallocation functions are not safe to call from signal handlers. Just remove the lock file if there is one and exit immediately. Fixes: FS#46375, FS#45995, FS#47011 Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 94685a7..1d4459e 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -336,10 +336,10 @@ static void handler(int signum) } /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman * SIGTERM: release no matter what */ - alpm_trans_release(config->handle); + alpm_unlock(config->handle); /* output a newline to be sure we clear any line we may be on */ xwrite(out, "\n", 1); - cleanup(128 + signum); + _Exit(128 + signum); } static void invalid_opt(int used, const char *opt1, const char *opt2) -- 2.6.3
On SIGTERM pacman was exiting immediately, even in the middle of a transaction. In this case we should leave the lock file in place as an indication that the database may not be in a consistent state. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 1d4459e..d6ceeae 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -334,8 +334,7 @@ static void handler(int signum) columns_cache_reset(); return; } - /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman - * SIGTERM: release no matter what */ + /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ alpm_unlock(config->handle); /* output a newline to be sure we clear any line we may be on */ xwrite(out, "\n", 1); @@ -1138,7 +1137,7 @@ int main(int argc, char *argv[]) int ret = 0; size_t i; struct sigaction new_action, old_action; - const int signals[] = { SIGHUP, SIGINT, SIGTERM, SIGSEGV, SIGWINCH }; + const int signals[] = { SIGHUP, SIGINT, SIGSEGV, SIGWINCH }; uid_t myuid = getuid(); /* Set signal handlers */ -- 2.6.3
Our signal handler provides a way to gracefully interrupt a transaction and should always be set. The check appears to have originally been copied directly from the glibc manual. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index d6ceeae..05b8cd3 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1136,7 +1136,7 @@ int main(int argc, char *argv[]) { int ret = 0; size_t i; - struct sigaction new_action, old_action; + struct sigaction new_action; const int signals[] = { SIGHUP, SIGINT, SIGSEGV, SIGWINCH }; uid_t myuid = getuid(); @@ -1148,11 +1148,7 @@ int main(int argc, char *argv[]) /* assign our handler to any signals we care about */ for(i = 0; i < ARRAYSIZE(signals); i++) { - int signal = signals[i]; - sigaction(signal, NULL, &old_action); - if(old_action.sa_handler != SIG_IGN) { - sigaction(signal, &new_action, NULL); - } + sigaction(signals[i], &new_action, NULL); } /* i18n init */ -- 2.6.3
signal(7) lists a set of functions that can safely be called from within a signal handler. Even fileno and strlen are not guaranteed to be safe. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 05b8cd3..ba86dec 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -311,21 +311,19 @@ static ssize_t xwrite(int fd, const void *buf, size_t count) */ static void handler(int signum) { - int out = fileno(stdout); - int err = fileno(stderr); - const char *msg; if(signum == SIGSEGV) { - msg = "\nerror: segmentation fault\n" + const char msg[] = "\nerror: segmentation fault\n" "Please submit a full bug report with --debug if appropriate.\n"; - xwrite(err, msg, strlen(msg)); + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); exit(signum); } else if(signum == SIGINT || signum == SIGHUP) { if(signum == SIGINT) { - msg = "\nInterrupt signal received\n"; + const char msg[] = "\nInterrupt signal received\n"; + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); } else { - msg = "\nHangup signal received\n"; + const char msg[] = "\nHangup signal received\n"; + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); } - xwrite(err, msg, strlen(msg)); if(alpm_trans_interrupt(config->handle) == 0) { /* a transaction is being interrupted, don't exit pacman yet. */ return; @@ -337,7 +335,7 @@ static void handler(int signum) /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ alpm_unlock(config->handle); /* output a newline to be sure we clear any line we may be on */ - xwrite(out, "\n", 1); + xwrite(STDOUT_FILENO, "\n", 1); _Exit(128 + signum); } -- 2.6.3
Signals are special because they run asynchronously, making them non-trivial to handle correctly. Move the handlers a separate file to offset them from the normal code and make them easier to separate into individual functions without further cluttering pacman.c Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/Makefile.am | 1 + src/pacman/pacman.c | 60 ++------------------------------ src/pacman/sighandler.c | 92 +++++++++++++++++++++++++++++++++++++++++++++++++ src/pacman/sighandler.h | 27 +++++++++++++++ 4 files changed, 122 insertions(+), 58 deletions(-) create mode 100644 src/pacman/sighandler.c create mode 100644 src/pacman/sighandler.h diff --git a/src/pacman/Makefile.am b/src/pacman/Makefile.am index b07c670..14572cd 100644 --- a/src/pacman/Makefile.am +++ b/src/pacman/Makefile.am @@ -40,6 +40,7 @@ pacman_SOURCES = \ pacman.h pacman.c \ query.c \ remove.c \ + sighandler.h sighandler.c \ sync.c \ callback.h callback.c \ upgrade.c \ diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index ba86dec..0f115c2 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -30,7 +30,6 @@ #include <limits.h> #include <getopt.h> #include <string.h> -#include <signal.h> #include <unistd.h> #include <sys/types.h> #include <sys/utsname.h> /* uname */ @@ -45,6 +44,7 @@ #include "pacman.h" #include "util.h" #include "conf.h" +#include "sighandler.h" /* list of targets specified on command line */ static alpm_list_t *pm_targets; @@ -294,51 +294,6 @@ static void cleanup(int ret) exit(ret); } -/** Write function that correctly handles EINTR. - */ -static ssize_t xwrite(int fd, const void *buf, size_t count) -{ - ssize_t ret; - do { - ret = write(fd, buf, count); - } while(ret == -1 && errno == EINTR); - return ret; -} - -/** Catches thrown signals. Performs necessary cleanup to ensure database is - * in a consistent state. - * @param signum the thrown signal - */ -static void handler(int signum) -{ - if(signum == SIGSEGV) { - const char msg[] = "\nerror: segmentation fault\n" - "Please submit a full bug report with --debug if appropriate.\n"; - xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); - exit(signum); - } else if(signum == SIGINT || signum == SIGHUP) { - if(signum == SIGINT) { - const char msg[] = "\nInterrupt signal received\n"; - xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); - } else { - const char msg[] = "\nHangup signal received\n"; - xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); - } - if(alpm_trans_interrupt(config->handle) == 0) { - /* a transaction is being interrupted, don't exit pacman yet. */ - return; - } - } else if(signum == SIGWINCH) { - columns_cache_reset(); - return; - } - /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ - alpm_unlock(config->handle); - /* output a newline to be sure we clear any line we may be on */ - xwrite(STDOUT_FILENO, "\n", 1); - _Exit(128 + signum); -} - static void invalid_opt(int used, const char *opt1, const char *opt2) { if(used) { @@ -1134,20 +1089,9 @@ int main(int argc, char *argv[]) { int ret = 0; size_t i; - struct sigaction new_action; - const int signals[] = { SIGHUP, SIGINT, SIGSEGV, SIGWINCH }; uid_t myuid = getuid(); - /* Set signal handlers */ - /* Set up the structure to specify the new action. */ - new_action.sa_handler = handler; - sigemptyset(&new_action.sa_mask); - new_action.sa_flags = SA_RESTART; - - /* assign our handler to any signals we care about */ - for(i = 0; i < ARRAYSIZE(signals); i++) { - sigaction(signals[i], &new_action, NULL); - } + install_signal_handlers(); /* i18n init */ #if defined(ENABLE_NLS) diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c new file mode 100644 index 0000000..d488ece --- /dev/null +++ b/src/pacman/sighandler.c @@ -0,0 +1,92 @@ +/* + * sighandler.c + * + * Copyright (c) 2015 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#include <errno.h> +#include <signal.h> +#include <unistd.h> + +#include <alpm.h> + +#include "conf.h" +#include "sighandler.h" +#include "util.h" + +/** Write function that correctly handles EINTR. + */ +static ssize_t xwrite(int fd, const void *buf, size_t count) +{ + ssize_t ret; + do { + ret = write(fd, buf, count); + } while(ret == -1 && errno == EINTR); + return ret; +} + +/** Catches thrown signals. Performs necessary cleanup to ensure database is + * in a consistent state. + * @param signum the thrown signal + */ +static void handler(int signum) +{ + if(signum == SIGSEGV) { + const char msg[] = "\nerror: segmentation fault\n" + "Please submit a full bug report with --debug if appropriate.\n"; + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); + exit(signum); + } else if(signum == SIGINT || signum == SIGHUP) { + if(signum == SIGINT) { + const char msg[] = "\nInterrupt signal received\n"; + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); + } else { + const char msg[] = "\nHangup signal received\n"; + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); + } + if(alpm_trans_interrupt(config->handle) == 0) { + /* a transaction is being interrupted, don't exit pacman yet. */ + return; + } + } else if(signum == SIGWINCH) { + columns_cache_reset(); + return; + } + /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ + alpm_unlock(config->handle); + /* output a newline to be sure we clear any line we may be on */ + xwrite(STDOUT_FILENO, "\n", 1); + _Exit(128 + signum); +} + +void install_signal_handlers(void) +{ + struct sigaction new_action; + const int signals[] = { SIGHUP, SIGINT, SIGSEGV, SIGWINCH }; + size_t i; + /* Set signal handlers */ + /* Set up the structure to specify the new action. */ + new_action.sa_handler = handler; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = SA_RESTART; + + /* assign our handler to any signals we care about */ + for(i = 0; i < ARRAYSIZE(signals); i++) { + sigaction(signals[i], &new_action, NULL); + } +} + +/* vim: set noet: */ diff --git a/src/pacman/sighandler.h b/src/pacman/sighandler.h new file mode 100644 index 0000000..f7abc9e --- /dev/null +++ b/src/pacman/sighandler.h @@ -0,0 +1,27 @@ +/* + * sighandler.h + * + * Copyright (c) 2015 Pacman Development Team <pacman-dev@archlinux.org> + * + * This program is free software; you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation; either version 2 of the License, or + * (at your option) any later version. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see <http://www.gnu.org/licenses/>. + */ + +#ifndef _PM_SIGHANDLER_H +#define _PM_SIGHANDLER_H + +void install_signal_handlers(void); + +#endif /* _PM_SIGHANDLER_H */ + +/* vim: set noet: */ -- 2.6.3
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 5 ++++- src/pacman/sighandler.c | 21 +++++++++++++++++---- src/pacman/sighandler.h | 1 + 3 files changed, 22 insertions(+), 5 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 0f115c2..e1f9039 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1107,9 +1107,12 @@ int main(int argc, char *argv[]) cleanup(1); } - /* disable progressbar if the output is redirected */ if(!isatty(fileno(stdout))) { + /* disable progressbar if the output is redirected */ config->noprogressbar = 1; + } else { + /* install signal handler to update output width */ + install_winch_handler(); } /* Priority of options: diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index d488ece..76f7e9d 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -61,9 +61,6 @@ static void handler(int signum) /* a transaction is being interrupted, don't exit pacman yet. */ return; } - } else if(signum == SIGWINCH) { - columns_cache_reset(); - return; } /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ alpm_unlock(config->handle); @@ -72,11 +69,27 @@ static void handler(int signum) _Exit(128 + signum); } +static void winch_handler(int signum) +{ + (void)signum; /* suppress unused variable warnings */ + columns_cache_reset(); +} + +void install_winch_handler(void) +{ + struct sigaction new_action; + new_action.sa_handler = winch_handler; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = SA_RESTART; + sigaction(SIGWINCH, &new_action, NULL); +} + void install_signal_handlers(void) { struct sigaction new_action; - const int signals[] = { SIGHUP, SIGINT, SIGSEGV, SIGWINCH }; + const int signals[] = { SIGHUP, SIGINT, SIGSEGV }; size_t i; + /* Set signal handlers */ /* Set up the structure to specify the new action. */ new_action.sa_handler = handler; diff --git a/src/pacman/sighandler.h b/src/pacman/sighandler.h index f7abc9e..bcf7d88 100644 --- a/src/pacman/sighandler.h +++ b/src/pacman/sighandler.h @@ -20,6 +20,7 @@ #ifndef _PM_SIGHANDLER_H #define _PM_SIGHANDLER_H +void install_winch_handler(void); void install_signal_handlers(void); #endif /* _PM_SIGHANDLER_H */ -- 2.6.3
Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 1 + src/pacman/sighandler.c | 44 +++++++++++++++++++++++++++----------------- src/pacman/sighandler.h | 1 + 3 files changed, 29 insertions(+), 17 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index e1f9039..6a0eb97 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1091,6 +1091,7 @@ int main(int argc, char *argv[]) size_t i; uid_t myuid = getuid(); + install_segv_handler(); install_signal_handlers(); /* i18n init */ diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index 76f7e9d..36f87d7 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -44,23 +44,16 @@ static ssize_t xwrite(int fd, const void *buf, size_t count) */ static void handler(int signum) { - if(signum == SIGSEGV) { - const char msg[] = "\nerror: segmentation fault\n" - "Please submit a full bug report with --debug if appropriate.\n"; + if(signum == SIGINT) { + const char msg[] = "\nInterrupt signal received\n"; xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); - exit(signum); - } else if(signum == SIGINT || signum == SIGHUP) { - if(signum == SIGINT) { - const char msg[] = "\nInterrupt signal received\n"; - xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); - } else { - const char msg[] = "\nHangup signal received\n"; - xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); - } - if(alpm_trans_interrupt(config->handle) == 0) { - /* a transaction is being interrupted, don't exit pacman yet. */ - return; - } + } else { + const char msg[] = "\nHangup signal received\n"; + xwrite(STDERR_FILENO, msg, ARRAYSIZE(msg) - 1); + } + if(alpm_trans_interrupt(config->handle) == 0) { + /* a transaction is being interrupted, don't exit pacman yet. */ + return; } /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ alpm_unlock(config->handle); @@ -69,6 +62,23 @@ static void handler(int signum) _Exit(128 + signum); } +static void segv_handler(int signum) +{ + const char msg[] = "\nerror: segmentation fault\n" + "Please submit a full bug report with --debug if appropriate.\n"; + xwrite(STDERR_FILENO, msg, sizeof(msg) - 1); + _Exit(signum); +} + +void install_segv_handler(void) +{ + struct sigaction new_action; + new_action.sa_handler = segv_handler; + sigemptyset(&new_action.sa_mask); + new_action.sa_flags = SA_RESTART; + sigaction(SIGSEGV, &new_action, NULL); +} + static void winch_handler(int signum) { (void)signum; /* suppress unused variable warnings */ @@ -87,7 +97,7 @@ void install_winch_handler(void) void install_signal_handlers(void) { struct sigaction new_action; - const int signals[] = { SIGHUP, SIGINT, SIGSEGV }; + const int signals[] = { SIGHUP, SIGINT }; size_t i; /* Set signal handlers */ diff --git a/src/pacman/sighandler.h b/src/pacman/sighandler.h index bcf7d88..037d54f 100644 --- a/src/pacman/sighandler.h +++ b/src/pacman/sighandler.h @@ -20,6 +20,7 @@ #ifndef _PM_SIGHANDLER_H #define _PM_SIGHANDLER_H +void install_segv_handler(void); void install_winch_handler(void); void install_signal_handlers(void); -- 2.6.3
Delays handler setup until after config is set to a valid value to avoid a segmentation fault. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 3 ++- src/pacman/sighandler.c | 34 ++++++++++++++-------------------- src/pacman/sighandler.h | 2 +- 3 files changed, 17 insertions(+), 22 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 6a0eb97..4237b4d 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -1092,7 +1092,6 @@ int main(int argc, char *argv[]) uid_t myuid = getuid(); install_segv_handler(); - install_signal_handlers(); /* i18n init */ #if defined(ENABLE_NLS) @@ -1108,6 +1107,8 @@ int main(int argc, char *argv[]) cleanup(1); } + install_soft_interrupt_handler(); + if(!isatty(fileno(stdout))) { /* disable progressbar if the output is redirected */ config->noprogressbar = 1; diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index 36f87d7..e88375a 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -42,7 +42,7 @@ static ssize_t xwrite(int fd, const void *buf, size_t count) * in a consistent state. * @param signum the thrown signal */ -static void handler(int signum) +static void soft_interrupt_handler(int signum) { if(signum == SIGINT) { const char msg[] = "\nInterrupt signal received\n"; @@ -55,13 +55,25 @@ static void handler(int signum) /* a transaction is being interrupted, don't exit pacman yet. */ return; } - /* SIGINT/SIGHUP: no committing transaction, release it now and then exit pacman */ alpm_unlock(config->handle); /* output a newline to be sure we clear any line we may be on */ xwrite(STDOUT_FILENO, "\n", 1); _Exit(128 + signum); } +void install_soft_interrupt_handler(void) +{ + struct sigaction new_action; + new_action.sa_handler = soft_interrupt_handler; + new_action.sa_flags = SA_RESTART; + sigemptyset(&new_action.sa_mask); + sigaddset(&new_action.sa_mask, SIGINT); + sigaddset(&new_action.sa_mask, SIGHUP); + + sigaction(SIGINT, &new_action, NULL); + sigaction(SIGHUP, &new_action, NULL); +} + static void segv_handler(int signum) { const char msg[] = "\nerror: segmentation fault\n" @@ -94,22 +106,4 @@ void install_winch_handler(void) sigaction(SIGWINCH, &new_action, NULL); } -void install_signal_handlers(void) -{ - struct sigaction new_action; - const int signals[] = { SIGHUP, SIGINT }; - size_t i; - - /* Set signal handlers */ - /* Set up the structure to specify the new action. */ - new_action.sa_handler = handler; - sigemptyset(&new_action.sa_mask); - new_action.sa_flags = SA_RESTART; - - /* assign our handler to any signals we care about */ - for(i = 0; i < ARRAYSIZE(signals); i++) { - sigaction(signals[i], &new_action, NULL); - } -} - /* vim: set noet: */ diff --git a/src/pacman/sighandler.h b/src/pacman/sighandler.h index 037d54f..51f347b 100644 --- a/src/pacman/sighandler.h +++ b/src/pacman/sighandler.h @@ -22,7 +22,7 @@ void install_segv_handler(void); void install_winch_handler(void); -void install_signal_handlers(void); +void install_soft_interrupt_handler(void); #endif /* _PM_SIGHANDLER_H */ -- 2.6.3
The soft interrupt handler dereferences config, causing a segfault if it is called during cleanup. Signed-off-by: Andrew Gregory <andrew.gregory.8@gmail.com> --- src/pacman/pacman.c | 1 + src/pacman/sighandler.c | 10 ++++++++++ src/pacman/sighandler.h | 1 + 3 files changed, 12 insertions(+) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index 4237b4d..2380bd1 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -279,6 +279,7 @@ static void setuseragent(void) */ static void cleanup(int ret) { + remove_soft_interrupt_handler(); if(config) { /* free alpm library resources */ if(config->handle && alpm_release(config->handle) == -1) { diff --git a/src/pacman/sighandler.c b/src/pacman/sighandler.c index e88375a..3f18b5c 100644 --- a/src/pacman/sighandler.c +++ b/src/pacman/sighandler.c @@ -74,6 +74,16 @@ void install_soft_interrupt_handler(void) sigaction(SIGHUP, &new_action, NULL); } +void remove_soft_interrupt_handler(void) +{ + struct sigaction new_action; + sigemptyset(&new_action.sa_mask); + new_action.sa_handler = SIG_DFL; + new_action.sa_flags = 0; + sigaction(SIGINT, &new_action, NULL); + sigaction(SIGHUP, &new_action, NULL); +} + static void segv_handler(int signum) { const char msg[] = "\nerror: segmentation fault\n" diff --git a/src/pacman/sighandler.h b/src/pacman/sighandler.h index 51f347b..b963357 100644 --- a/src/pacman/sighandler.h +++ b/src/pacman/sighandler.h @@ -23,6 +23,7 @@ void install_segv_handler(void); void install_winch_handler(void); void install_soft_interrupt_handler(void); +void remove_soft_interrupt_handler(void); #endif /* _PM_SIGHANDLER_H */ -- 2.6.3
This whole patchset is good. Thanks for doing the clean-up! A
participants (2)
-
Allan McRae
-
Andrew Gregory