On Sun, Oct 18, 2009 at 12:49 AM, Matt Fisher mfisher...@maine.rr.com wrote:
You're correct. I got started on this train of thought after reading
an old USENIX paper[1]. The line I suggested for deletion is identical
to what section 1.2.1 of that paper recommended should always be
included. Recent material continues to recommend such code, but not
specifically in OpenBSD[2],[3].
Since such code is only rarely used in OpenBSD -- 9 occurrences in the
whole tree -- I thought it might be an obsolete or inapplicable
recommendation. One such occurrence was in m4/main.c, so I contacted the
developer who had recently checked in that file.
I did read manuals, study code, consider, and test first. But I knew my
expertise was limited, and that there could be other considerations.
If all you have is a chunk of code that you don't understand the
justification for, you have to work backwards to the problem. This
can turn into the programming equivalent of disproving an existential,
for which having a broad base of experience really helps. How do you
get that base of experience? Well, start by tackling concrete
problems (program behaves incorrectly when I try blah) instead of
jumping straight to the abstract cases.
That, or you need to crank up your methodology and logic. The code in
this case applies only when the process is started with SIGINT set to
SIG_IGN. If the process was set by its parent to ignore SIGINT, why
would it start paying attention to them now by catching them? Doing
that might be fine if that was something that used to be normal
practice but never happened nowadays. So what programs did that in
the past? Didn't the paper say something about running stuff in the
background from the shell with ? So let's check the sh(1) manpage
to see what it says now:
When an asynchronous command is started when job
control is disabled (i.e. in most scripts), the command is started with
signals SIGINT and SIGQUIT ignored
Looks like the shell still does that. A little bit of noodling shows
that this snippet of shell script
#!/bin/sh
( : ; /some/program/here )
Invokes /some/program/here with SIGINT ignored.
So, it looks like running m4 in the background from a shell script
will still result in it being invoked with SIGINT set to SIG_IGN. In
such a case, if you interrupted the shell script using SIGINT,
shouldn't m4 ignore the signal and continue? It would continue if the
shell script exited normally, so why should signaling the script
terminate it? Ergo, I think this code is correct and should be left
as is.
You wonder why only 9 programs in the tree have such code. Well, how
programs does this problem apply to? It doesn't apply to any program
that calls daemon(), for example, as they won't get SIGINTs a
terminal. So rpc.statd's signal handling is fine, for example. It
also doesn't apply to programs that can't be usefully backgrounded, so
'rain' is fine as is.
But it looks like some programs do handle it wrong and could use
fixing. For example, 'sort' appears to be wrong...but before I wrote
that I figured out how to trigger the problem, both to confirm it and
to verify the fix. Make sense?
Philip Guenther
--- sort.c 22 Aug 2007 06:56:40 - 1.36
+++ sort.c 18 Oct 2009 22:44:47 -
@@ -273,7 +273,7 @@ main(int argc, char *argv[])
outfile = outpath = toutpath;
} else if (!(ch = access(outpath, 0))
strncmp(_PATH_DEV, outpath, 5)) {
- struct sigaction act;
+ struct sigaction oact, act;
int sigtable[] = {SIGHUP, SIGINT, SIGPIPE, SIGXCPU, SIGXFSZ,
SIGVTALRM, SIGPROF, 0};
int outfd;
@@ -298,7 +298,9 @@ main(int argc, char *argv[])
act.sa_flags = SA_RESTART;
act.sa_handler = onsig;
for (i = 0; sigtable[i]; ++i) /* always unlink toutpath */
- sigaction(sigtable[i], act, 0);
+ if (sigaction(sigtable[i], NULL, oact) ||
+ oact.sa_handler != SIG_IGN)
+ sigaction(sigtable[i], act, NULL);
} else
outfile = outpath;
if (outfp == NULL (outfp = fopen(outfile, w)) == NULL)
