[pacman-dev] Strange cleanup() function usage in pacman.c
Cleanup() is a signal handler. However, we usually pass some return values to it manually, see lines 816 and 822 for example. Or run pacman -V --debug. (parseargs returns with 2 == SIGINT) Is this OK?
On Sun, Mar 09, 2008 at 11:40:48AM +0100, Nagy Gabor wrote:
Cleanup() is a signal handler. However, we usually pass some return values to it manually, see lines 816 and 822 for example. Or run pacman -V --debug. (parseargs returns with 2 == SIGINT) Is this OK?
Huh yes, all these cleanup calls don't look right.. Well, we only listen to SIGINT 2, SIGTERM 15 and SIGSEGV 11, but still we shouldn't call cleanup manually with values higher than 0. What about splitting these two functions : first part is signal(int signum) second part is cleanup(int ret)
This function was used in two different ways : - as a signal handler : the argument was the signal number - called manually for freeing the resources : the argument was the return value So the first part is now handler(int), and the second cleanup(int). Ref: http://www.archlinux.org/pipermail/pacman-dev/2008-March/011388.html Remaining problems : - the return values are messy. for example, 2 can mean both that it was interrupted (SIGINT == 2), or that --help or -V was used (returned by parseargs). - apparently signal is not portable and sigaction should be used instead Signed-off-by: Chantry Xavier <shiningxc@gmail.com> --- src/pacman/pacman.c | 43 +++++++++++++++++++++++++------------------ 1 files changed, 25 insertions(+), 18 deletions(-) diff --git a/src/pacman/pacman.c b/src/pacman/pacman.c index d699768..f46b71c 100644 --- a/src/pacman/pacman.c +++ b/src/pacman/pacman.c @@ -188,11 +188,31 @@ static void setuseragent(void) setenv("HTTP_USER_AGENT", agent, 0); } +/** Free the resources. + * + * @param ret the return value + */ +static void cleanup(int ret) { + /* free alpm library resources */ + if(alpm_release() == -1) { + pm_printf(PM_LOG_ERROR, alpm_strerrorlast()); + } + + /* free memory */ + FREELIST(pm_targets); + if(config) { + config_free(config); + config = NULL; + } + + exit(ret); +} + /** Catches thrown signals. Performs necessary cleanup to ensure database is * in a consistant state. * @param signum the thrown signal */ -static void cleanup(int signum) +static void handler(int signum) { if(signum==SIGSEGV) { @@ -211,20 +231,7 @@ static void cleanup(int signum) /* output a newline to be sure we clear any line we may be on */ printf("\n"); } - - /* free alpm library resources */ - if(alpm_release() == -1) { - pm_printf(PM_LOG_ERROR, alpm_strerrorlast()); - } - - /* free memory */ - FREELIST(pm_targets); - if(config) { - config_free(config); - config = NULL; - } - - exit(signum); + cleanup(signum); } /** Sets all libalpm required paths in one go. Called after the command line @@ -756,9 +763,9 @@ int main(int argc, char *argv[]) #endif /* set signal handlers */ - signal(SIGINT, cleanup); - signal(SIGTERM, cleanup); - signal(SIGSEGV, cleanup); + signal(SIGINT, handler); + signal(SIGTERM, handler); + signal(SIGSEGV, handler); /* i18n init */ #if defined(ENABLE_NLS) -- 1.5.4.2
On Sun, Mar 9, 2008 at 7:05 AM, Chantry Xavier <shiningxc@gmail.com> wrote:
This function was used in two different ways : - as a signal handler : the argument was the signal number - called manually for freeing the resources : the argument was the return value So the first part is now handler(int), and the second cleanup(int). Ref: http://www.archlinux.org/pipermail/pacman-dev/2008-March/011388.html
Remaining problems : - the return values are messy. for example, 2 can mean both that it was interrupted (SIGINT == 2), or that --help or -V was used (returned by parseargs). - apparently signal is not portable and sigaction should be used instead
Signed-off-by: Chantry Xavier <shiningxc@gmail.com> ---
Applied, thanks.
On Sun, Mar 9, 2008 at 6:20 AM, Xavier <shiningxc@gmail.com> wrote:
On Sun, Mar 09, 2008 at 11:40:48AM +0100, Nagy Gabor wrote:
Cleanup() is a signal handler. However, we usually pass some return values to it manually, see lines 816 and 822 for example. Or run pacman -V --debug. (parseargs returns with 2 == SIGINT) Is this OK?
Huh yes, all these cleanup calls don't look right.. Well, we only listen to SIGINT 2, SIGTERM 15 and SIGSEGV 11, but still we shouldn't call cleanup manually with values higher than 0.
What about splitting these two functions : first part is signal(int signum) second part is cleanup(int ret)
+1 -Dan
participants (4)
-
Chantry Xavier
-
Dan McGee
-
Nagy Gabor
-
Xavier