[Toybox] [PATCH] Factor out dhcpd's and syslogd's main loops into a library function

[Rob Landley]

On 09/10/2013 04:22:41 PM, Felix Janda wrote:
> Rob Landley wrote:
> > And yet a call to pselect needs its own sigprocmask() call as setup?
> > So... why are we using pselect again?
> 
> Didn't I say so? Before the call to pselect we block everything.

But why do we do that? (So that only kill -9 will kill daemons?)

> Then
> pselect atomically unblocks everything, does the select and afterwards
> blocks everything again. So the only thing that can be interrupted by
> signals is pselect. No need to think about race conditions. The  
> relation
> between pselect and select is the same as between ppoll and poll.

There was no need to think about signal handling race conditions in  
netcat. I haven't read through the dhcpd or syslogd implementation very  
closely yet, but I don't know what problem you're trying to solve here.

> The main reason for not using ppoll is that it is linux only. pselect
> is in POSIX. OpenBSD however doesn't seem to have it... The portable
> way would be to use the self-pipe as before. But then there is even
> more global state...

I don't understand what problem this addresses. I wrote a dnsd that  
didn't need this (although it just handled udp, not tcp). I've written  
my own little web servers, my own little vpn and a star topology  
network redirector... Not remembering ever playing games with signals  
like this for any of them?

> > > +  for (;;) {
> > > +    memcpy(&copy, &readfds, sizeof(copy));
> > > +    ret = pselect(nfds, &copy, 0, 0, 0, &action.sa_mask);
> > > +    if (ret == -1) {
> > > +      if (errno != EINTR) error_exit ("pselect");
> > > +      if (timeout && daemon_sig == SIGALRM) alarm(timeout);
> > > +      sighandler(daemon_sig);
> >
> > Unconditionally calling sighandler(), so we don't have the option  
> of it
> > being null and just getting the default behavior for signals. And  
> the
> > caller can't have its own use of alarm outside this helper.
> 
> Yes, for the broader use it would be good to make the signal handling
> optional (but that's actually the main point of this function). The  
> caller
> can have its own use of alarm if it doesn't use the one built into  
> this
> function.

There's a timeout built into the blocking function. Possibly you have  
to recalculate the timeout before calling the function again if you  
care about next scheduled action instead of just "it took too long".  
This is a heavyweight operation?

> > Ok, net win. (I need a lib/pending.h. Right now adding stuff to
> > lib/pending.c and then yanking it out again affects the commit  
> history
> > of permanent files...)
> 
> I guess using signal() instead of sigaction() would save some lines.  
> But I
> didn't want to mix the new pselect with the old signal().

I still don't understand why you're doing all this stuff with signals.

> > But this patch is doing a lot more than just introducing the new
> > library functions and making stuff use them, so I've got to do  
> _more_
> > review before I can come to an actual conclusion about whether or  
> not
> > to apply it...
> 
> It is really just moving stuff around. Luckily the local variables in  
> the
> main functions nicely broke up into the three seperate functions each.
> Anyway, I still need to improve the restarting of syslogd.
> 
> > I really do appreciate this work. Sorry to give you such a hard time
> > about it. I just want to get it _right_...
> 
> No problem. Please feel free to wait with coming back to this until  
> after
> the (impending?) release.

I have an extra day in ohio (the linuxfest sunday content wasn't worth  
staying for), so I'm trying to catch up on my email a bit and  
downloading the LFS 7.4 files in the background...

Rob
 1379258598.0