Re: [PATCH 0/5] replace signal() with sigaction()
- From
Jeremiah Mahler <jmmahler@gmail.com>
- Date
- May 28, 2014, 16:11 UTC
- Message-ID
- <20140528161100.GB7076@hudson.localdomain>
- In-Reply-To
- <53859307.8080506@viscovery.net>
On Wed, May 28, 2014 at 09:40:55AM +0200, Johannes Sixt wrote:
Show 29 quoted lines
> Am 5/28/2014 8:14, schrieb Jeremiah Mahler:
> > From signal(2)
> >
> > The behavior of signal() varies across UNIX versions, and has also var‐
> > ied historically across different versions of Linux. Avoid its use:
> > use sigaction(2) instead. See Portability below.
> >
> > This patch set replaces calls to signal() with sigaction() in all files
> > except sigchain.c. sigchain.c is a bit more complicated than the others
> > and will be done in a separate patch.
>
> In compat/mingw.c we have:
>
> int sigaction(int sig, struct sigaction *in, struct sigaction *out)
> {
> if (sig != SIGALRM)
> return errno = EINVAL,
> error("sigaction only implemented for SIGALRM");
> if (out != NULL)
> return errno = EINVAL,
> error("sigaction: param 3 != NULL not implemented");
>
> timer_fn = in->sa_handler;
> return 0;
> }
>
> Notice "only implemented for SIGALRM". Are adding the missing signals
> somewhere (here or in a later patch)?
> Good catch Johannes, I did not account for this.
If I am reading this correctly, sigaction() can only be used for SIGALRM, and no other signals. But signal() it would have worked for these other signals.
My first idea is why not call signal() from within this sigaction() call? The handler could be read from the struct sigaction passed in.
Show 16 quoted lines
> > Jeremiah Mahler (5): > > progress.c: replace signal() with sigaction() > > daemon.c run_service(): replace signal() with sigaction() > > daemon.c child_handler(): replace signal() with sigaction() > > daemon.c service_loop(): replace signal() with sigaction() > > connect.c: replace signal() with sigaction() > > > > connect.c | 5 ++++- > > daemon.c | 15 ++++++++++++--- > > progress.c | 6 +++++- > > 3 files changed, 21 insertions(+), 5 deletions(-) > > Isn't this a bit too much of code churn, given that there were no bug > reports where signal handling is identified as the culprit despite > the warning you cited above? >
We might get bugs from continuing to use signal(). We might get bugs from changing to sigaction(). I feel more comfortable using sigaction() since its man page is so adamant about not using signal().
-- Jeremiah Mahler jmmahler@gmail.com http://github.com/jmahler