Re: [PATCH 0/5] replace signal() with sigaction()

[Jeremiah Mahler <jmmahler@gmail.com>]

On Wed, May 28, 2014 at 09:40:55AM +0200, Johannes Sixt wrote:
> 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.

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