sd-event: update state at the end in event_source_enable

Coverity in CID#1435966 was complaining that s->enabled is not "restored" in
all cases. But the code was actually correct, since it should only be
"restored" in the error paths. But let's still make this prettier by not setting
the state before all operations that may fail are done.

We need to set .enabled for the prioq reshuffling operations, so move those down.

No functional change intended.

diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index bbb2b6bfd74cca0b6e5d5b6f58714943da95a4c8..5f66c379b5a52084a3e790c34d4f947d8df2f8d2 100644 (file)
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -2311,11 +2311,11 @@ static int event_source_disable(sd_event_source *s) {
         return 0;
 }
 
-static int event_source_enable(sd_event_source *s, int m) {
+static int event_source_enable(sd_event_source *s, int enable) {
         int r;
 
         assert(s);
-        assert(IN_SET(m, SD_EVENT_ON, SD_EVENT_ONESHOT));
+        assert(IN_SET(enable, SD_EVENT_ON, SD_EVENT_ONESHOT));
         assert(s->enabled == SD_EVENT_OFF);
 
         /* Unset the pending flag when this event source is enabled */
@@ -2325,31 +2325,16 @@ static int event_source_enable(sd_event_source *s, int m) {
                         return r;
         }
 
-        s->enabled = m;
-
         switch (s->type) {
-
         case SOURCE_IO:
-                r = source_io_register(s, m, s->io.events);
-                if (r < 0) {
-                        s->enabled = SD_EVENT_OFF;
+                r = source_io_register(s, enable, s->io.events);
+                if (r < 0)
                         return r;
-                }
-
-                break;
-
-        case SOURCE_TIME_REALTIME:
-        case SOURCE_TIME_BOOTTIME:
-        case SOURCE_TIME_MONOTONIC:
-        case SOURCE_TIME_REALTIME_ALARM:
-        case SOURCE_TIME_BOOTTIME_ALARM:
-                event_source_time_prioq_reshuffle(s);
                 break;
 
         case SOURCE_SIGNAL:
                 r = event_make_signal_data(s->event, s->signal.sig, NULL);
                 if (r < 0) {
-                        s->enabled = SD_EVENT_OFF;
                         event_gc_signal_data(s->event, &s->priority, s->signal.sig);
                         return r;
                 }
@@ -2362,9 +2347,8 @@ static int event_source_enable(sd_event_source *s, int m) {
                 if (EVENT_SOURCE_WATCH_PIDFD(s)) {
                         /* yes, we have pidfd */
 
-                        r = source_child_pidfd_register(s, s->enabled);
+                        r = source_child_pidfd_register(s, enable);
                         if (r < 0) {
-                                s->enabled = SD_EVENT_OFF;
                                 s->event->n_enabled_child_sources--;
                                 return r;
                         }
@@ -2373,7 +2357,6 @@ static int event_source_enable(sd_event_source *s, int m) {
 
                         r = event_make_signal_data(s->event, SIGCHLD, NULL);
                         if (r < 0) {
-                                s->enabled = SD_EVENT_OFF;
                                 s->event->n_enabled_child_sources--;
                                 event_gc_signal_data(s->event, &s->priority, SIGCHLD);
                                 return r;
@@ -2382,10 +2365,12 @@ static int event_source_enable(sd_event_source *s, int m) {
 
                 break;
 
+        case SOURCE_TIME_REALTIME:
+        case SOURCE_TIME_BOOTTIME:
+        case SOURCE_TIME_MONOTONIC:
+        case SOURCE_TIME_REALTIME_ALARM:
+        case SOURCE_TIME_BOOTTIME_ALARM:
         case SOURCE_EXIT:
-                prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
-                break;
-
         case SOURCE_DEFER:
         case SOURCE_POST:
         case SOURCE_INOTIFY:
@@ -2395,6 +2380,26 @@ static int event_source_enable(sd_event_source *s, int m) {
                 assert_not_reached("Wut? I shouldn't exist.");
         }
 
+        s->enabled = enable;
+
+        /* Non-failing operations below */
+        switch (s->type) {
+        case SOURCE_TIME_REALTIME:
+        case SOURCE_TIME_BOOTTIME:
+        case SOURCE_TIME_MONOTONIC:
+        case SOURCE_TIME_REALTIME_ALARM:
+        case SOURCE_TIME_BOOTTIME_ALARM:
+                event_source_time_prioq_reshuffle(s);
+                break;
+
+        case SOURCE_EXIT:
+                prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
+                break;
+
+        default:
+                break;
+        }
+
         return 0;
 }