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).

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 majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to