On Fri, Sep 04, 2015 at 11:35:57AM +0200, Takashi Iwai wrote: > > Hmm, is there is any reason to just pass an "in_signal" flag to > > wait_for_pager(), to avoid duplicating the logic? > > Just because wait_for_pager() itself is an atexit hook that can't take > an argument, so we'd need to split to a new function. I don't mind > either way. The revised patch is below. Ah, right. That's unfortunate, but I think I prefer adding the extra wrapper to duplicating the contents of the function. > -- 8< -- > From: Takashi Iwai <tiwai@xxxxxxx> > Subject: [PATCH v2] pager: don't use unsafe functions in signal handlers > [...] This looks good to me. Do you plan on fixing any of the other handlers (you don't have to; I just want to know if somebody is planning to work on it). The pattern of atexit and signal handlers is repeated in several places, and it seems like we will have to add the same in_signal boilerplate in each instance. I wonder if we should provide a global "register_cleanup" that takes a "void (*func)(int in_signal))" function pointer, and: 1. Adds it to a list (ideally in a way that is atomic if we get interrupted while adding to the list). 2. If not already run, registers an atexit() handler and sigchain_push_common for a meta-handler which runs through the list and runs each handler. It's not a _huge_ amount of boilerplate code we'd be saving, but at least conforming to the "in_signal" function template would make people think twice about what they're doing inside the cleanup function. :) -Peff -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html