[systemd-devel] [PATCH 1/2] sd-event: be more careful when enabling/disabling signals

[Zbigniew Jędrzejewski-Szmek]

On Wed, Oct 08, 2014 at 09:44:33PM +0200, Lennart Poettering wrote:
> On Sun, 05.10.14 20:42, Zbigniew Jędrzejewski-Szmek (zbyszek at in.waw.pl) wrote:
> 
> >  
> > @@ -626,11 +634,13 @@ static void source_disconnect(sd_event_source *s) {
> >  
> >          case SOURCE_SIGNAL:
> >                  if (s->signal.sig > 0) {
> > -                        if (s->signal.sig != SIGCHLD || s->event->n_enabled_child_sources == 0)
> > -                                assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
> > -
> >                          if (s->event->signal_sources)
> >                                  s->event->signal_sources[s->signal.sig] = NULL;
> > +
> > +                        /* If the signal was on and now it is off... */
> > +                        if (s->enabled != SD_EVENT_OFF && !need_signal(s->event, s->signal.sig)) {
> > +                                assert_se(sigdelset(&s->event->sigset, s->signal.sig) == 0);
> > +                        }
> 
> Coding style: the extra {} should go.
This was in preparation for the next patch, which adds more the body.
Wonders of 'git add -p' :)

> >                                  s->event->n_enabled_child_sources--;
> > -                        }
> >  
> > -                        if (!s->event->signal_sources || !s->event->signal_sources[SIGCHLD])
> > -                                assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
> > +                                /* We know the signal was on, if it is off now... */
> > +                                if (!need_signal(s->event, SIGCHLD)) {
> > +                                        assert_se(sigdelset(&s->event->sigset, SIGCHLD) == 0);
> > +                                }
> > +                        }
> 
> Same here.
> 
> Patch looks good to me. If all tests pass and the system still boots
> looks good to commit to me.
Yeah, it boots. Pushed.

Zbyszek