git/list[1] front-page[2] threads[3] people[4] search[5] about
 

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
Previous: Erik Faye-Lund
Message 16 of 16 in “replace signal() with sigaction()”
  1. 0/5 replace signal() with sigaction()Jeremiah Mahler, May 28, 2014
  2. 1/5 progress.c: replace signal() with sigaction()Jeremiah Mahler, May 28, 2014
  3. Chris PackhamMay 28, 2014
  4. David KastrupMay 28, 2014
  5. Erik Faye-LundMay 28, 2014
  6. David KastrupMay 28, 2014
  7. Junio C HamanoMay 28, 2014
  8. Jeremiah MahlerMay 28, 2014
  9. 2/5 daemon.c run_service(): replace signal() with sigaction()Jeremiah Mahler, May 28, 2014
  10. 3/5 daemon.c child_handler(): replace signal() with sigaction()Jeremiah Mahler, May 28, 2014
  11. 4/5 daemon.c service_loop(): replace signal() with sigaction()Jeremiah Mahler, May 28, 2014
  12. 5/5 connect.c: replace signal() with sigaction()Jeremiah Mahler, May 28, 2014
  13. Johannes SixtMay 28, 2014
  14. Chris PackhamMay 28, 2014
  15. Erik Faye-LundMay 28, 2014
  16. Jeremiah MahlerMay 28, 2014

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.