On Fri, 04 Sep 2015 15:04:48 +0200, Jeff King wrote: > > 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 <ti...@suse.de> > > 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).
Heh, I'd like to leave the rest for professionals :) > 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. :) Or, we may have a global in_signal flag. Of course, this is bad if git itself is multi-threaded, though. thanks, Takashi -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
