Re: patch to make script(1) exit cleanly on reciept of signal

[Chris Pressey <cpressey@xxxxxxxxxxxxxxx>]

On Wed, 31 Mar 2004 20:27:09 -0800
Chris Pressey <cpressey@xxxxxxxxxxxxxxx> wrote:

> On Wed, 31 Mar 2004 19:47:56 -0800 (PST)
> Matthew Dillon <dillon@xxxxxxxxxxxxxxxxxxxx> wrote:
> 
> > :Chris Pressey <cpressey@xxxxxxxxxxxxxxx> wrote:
> > :> http://catseye.webhop.net/DragonFlyBSD/patch/script.diff
> >
> >     Looks good.  One change and you are done... a flushtime of 0 is 
> >     now illegal (because that will cause the select() to poll and
> >     script to go into a cpu-bound loop), so change the flushtime < 0
> >     test to flushtime <= 0.
> 
> Er - I just realized - I think it's even worse than that.  If the user
> specifies a flushtime of 0 (which is perfectly legal according to the
> man page, and I don't think we want to change that,) tvp is a NULL
> pointer rather than a pointer to our timeout.  Which means our signal
> handler doesn't have any effect at all on select(), because it's
> changing a timeout that isn't used :)
> 
> Thanks for making me think about it - I'll rewrite it, probably using
> your suggestion #1 instead.

OK, inspiration hit me while rethinking this!

Since we already have a pointer that may be pointing to the timeout, or
may be NULL, it occurred to me - why not, instead of having the signal
handler alter the timeout, just have it alter the pointer?

This surely handles the case where the user specified a flushtime of
zero, because the pointer changes rather than the (unused in the case of
flushtime == 0) struct timeval.

I also think (emphasis on THINK) this means that we don't need the
interlock.  The mainline code can change tv to it's heart's content
while the signal handler changes tvp.  Whether the signal happens before
or after the mainline code sets the timeout, when select() is called it
uses tvp, which will point to the timeout (or NULL) if a signal hasn't
happened, and will point to a zero-timeout structure if it has happened.

Unless there's some obscure issue with pointers that I'm not aware of -
I'm pretty confident this handles the race condition correctly, and the
code is simpler, too.

The new patch is at the above URL.

-Chris