sd-event: make sure sd_event_now() cannot fail

Previously, if the event loop never ran before sd_event_now() would
fail. With this change it will instead fall back to invoking now(). This
way, the function cannot fail anymore, except for programming error when
invoking it with wrong parameters.

This takes into account the fact that many callers did not handle the
error condition correctly, and if the callers did, then they kept simply
invoking now() as fall back on their own. Hence let's shorten the code
using this call, and make things more robust, and let's just fall back
to now() internally.

Whether now() is used or the cache timestamp may still be detected via
the return value of sd_event_now(). If > 0 is returned, then the fall
back to now() was used, if == 0 is returned, then the cached value was
returned.

This patch also simplifies many of the invocations of sd_event_now():
the manual fall back to now() can be removed. Also, in cases where the
call is invoked withing void functions we can now protect the invocation
via assert_se(), acknowledging the fact that the call cannot fail
anymore except for programming errors with the parameters.

This change is inspired by #841.

diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c
index cc5e032344bd14ebb5230fcaa6395a99cc72ab1b..d2cc3194b30e16fbe0f796e130331ae19f805e6a 100644 (file)
--- a/src/libsystemd-network/sd-dhcp-server.c
+++ b/src/libsystemd-network/sd-dhcp-server.c
@@ -797,7 +797,7 @@ int dhcp_server_handle_message(sd_dhcp_server *server, DHCPMessage *message,
                                          clock_boottime_or_monotonic(),
                                          &time_now);
                         if (r < 0)
-                                time_now = now(clock_boottime_or_monotonic());
+                                return r;
                         lease->expiration = req->lifetime * USEC_PER_SEC + time_now;
 
                         r = server_send_ack(server, req, address);

diff --git a/src/libsystemd-network/sd-dhcp6-client.c b/src/libsystemd-network/sd-dhcp6-client.c
index 85162dc5555860e9b4ebe07d59c0c669fb9305f2..e2f58628514ceedfa2f9012bb598069b0ff25538 100644 (file)
--- a/src/libsystemd-network/sd-dhcp6-client.c
+++ b/src/libsystemd-network/sd-dhcp6-client.c
@@ -975,14 +975,9 @@ static int client_start(sd_dhcp6_client *client, enum DHCP6State state)
         client->retransmit_time = 0;
         client->retransmit_count = 0;
 
-        if (client->state == DHCP6_STATE_STOPPED) {
-                time_now = now(clock_boottime_or_monotonic());
-        } else {
-                r = sd_event_now(client->event, clock_boottime_or_monotonic(),
-                                 &time_now);
-                if (r < 0)
-                        return r;
-        }
+        r = sd_event_now(client->event, clock_boottime_or_monotonic(), &time_now);
+        if (r < 0)
+                return r;
 
         switch (state) {
         case DHCP6_STATE_STOPPED:

diff --git a/src/libsystemd-network/sd-ipv4ll.c b/src/libsystemd-network/sd-ipv4ll.c
index 9e04db96bb61fa179fe3f005bb7c473021bc08be..0cb77ccf713cbc80e2d1255293e6c6d432b60995 100644 (file)
--- a/src/libsystemd-network/sd-ipv4ll.c
+++ b/src/libsystemd-network/sd-ipv4ll.c
@@ -187,8 +187,7 @@ static void ipv4ll_set_next_wakeup(sd_ipv4ll *ll, int sec, int random_sec) {
         if (random_sec)
                 next_timeout += random_u32() % (random_sec * USEC_PER_SEC);
 
-        if (sd_event_now(ll->event, clock_boottime_or_monotonic(), &time_now) < 0)
-                time_now = now(clock_boottime_or_monotonic());
+        assert_se(sd_event_now(ll->event, clock_boottime_or_monotonic(), &time_now) >= 0);
 
         ll->next_wakeup = time_now + next_timeout;
         ll->next_wakeup_valid = 1;

diff --git a/src/libsystemd-network/sd-pppoe.c b/src/libsystemd-network/sd-pppoe.c
index 1de8a5e8bf1b9eac2cabf0bbcdf0d8e3968a5262..ff064f563f47d7ac589a44663d461900dd71f276 100644 (file)
--- a/src/libsystemd-network/sd-pppoe.c
+++ b/src/libsystemd-network/sd-pppoe.c
@@ -346,9 +346,7 @@ static int pppoe_arm_timeout(sd_pppoe *ppp) {
         assert(ppp);
 
         r = sd_event_now(ppp->event, clock_boottime_or_monotonic(), &next_timeout);
-        if (r == -ENODATA)
-                next_timeout = now(clock_boottime_or_monotonic());
-        else if (r < 0)
+        if (r < 0)
                 return r;
 
         next_timeout += 500 * USEC_PER_MSEC;

diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index 76964aa0ccd71ac0aecdd4d19d38d076e5a75f0c..754fb7614e3c65997e528ba2d4f4aa958a831d5b 100644 (file)
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -2572,9 +2572,12 @@ _public_ int sd_event_now(sd_event *e, clockid_t clock, uint64_t *usec) {
         assert_return(usec, -EINVAL);
         assert_return(!event_pid_changed(e), -ECHILD);
 
-        /* If we haven't run yet, just get the actual time */
-        if (!dual_timestamp_is_set(&e->timestamp))
-                return -ENODATA;
+        if (!dual_timestamp_is_set(&e->timestamp)) {
+                /* Implicitly fall back to now() if we never ran
+                 * before and thus have no cached time. */
+                *usec = now(clock);
+                return 1;
+        }
 
         switch (clock) {
 

diff --git a/src/resolve/resolved-dns-transaction.c b/src/resolve/resolved-dns-transaction.c
index 487b2c516210bb4073db555ae02b22b55d276487..53779f33721c65d16e5aeaffff193cfa961cf631 100644 (file)
--- a/src/resolve/resolved-dns-transaction.c
+++ b/src/resolve/resolved-dns-transaction.c
@@ -372,9 +372,7 @@ void dns_transaction_process_reply(DnsTransaction *t, DnsPacket *p) {
                 }
         }
 
-        r = sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts);
-        if (r < 0)
-                ts = now(clock_boottime_or_monotonic());
+        assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0);
 
         switch (t->scope->protocol) {
         case DNS_PROTOCOL_DNS:
@@ -602,9 +600,7 @@ int dns_transaction_go(DnsTransaction *t) {
                 return 0;
         }
 
-        r = sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts);
-        if (r < 0)
-                ts = now(clock_boottime_or_monotonic());
+        assert_se(sd_event_now(t->scope->manager->event, clock_boottime_or_monotonic(), &ts) >= 0);
 
         t->n_attempts++;
         t->start_usec = ts;

diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index d0b8bad48e680661676c648ba7b1746d49befb0d..28ac44fb8ebb6b1a278ce27d1cb2f27c070e9275 100644 (file)
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -261,7 +261,6 @@ static int on_event_timeout_warning(sd_event_source *s, uint64_t usec, void *use
 static void worker_attach_event(struct worker *worker, struct event *event) {
         sd_event *e;
         uint64_t usec;
-        int r;
 
         assert(worker);
         assert(worker->manager);
@@ -276,9 +275,7 @@ static void worker_attach_event(struct worker *worker, struct event *event) {
 
         e = worker->manager->event;
 
-        r = sd_event_now(e, clock_boottime_or_monotonic(), &usec);
-        if (r < 0)
-                return;
+        assert_se(sd_event_now(e, clock_boottime_or_monotonic(), &usec) >= 0);
 
         (void) sd_event_add_time(e, &event->timeout_warning, clock_boottime_or_monotonic(),
                                  usec + arg_event_timeout_warn_usec, USEC_PER_SEC, on_event_timeout_warning, event);
@@ -749,9 +746,7 @@ static void manager_exit(Manager *manager) {
         event_queue_cleanup(manager, EVENT_QUEUED);
         manager_kill_workers(manager);
 
-        r = sd_event_now(manager->event, clock_boottime_or_monotonic(), &usec);
-        if (r < 0)
-                return;
+        assert_se(sd_event_now(manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
 
         r = sd_event_add_time(manager->event, NULL, clock_boottime_or_monotonic(),
                               usec + 30 * USEC_PER_SEC, USEC_PER_SEC, on_exit_timeout, manager);
@@ -780,7 +775,6 @@ static void manager_reload(Manager *manager) {
 static void event_queue_start(Manager *manager) {
         struct udev_list_node *loop;
         usec_t usec;
-        int r;
 
         assert(manager);
 
@@ -788,17 +782,15 @@ static void event_queue_start(Manager *manager) {
             manager->exit || manager->stop_exec_queue)
                 return;
 
-        r = sd_event_now(manager->event, clock_boottime_or_monotonic(), &usec);
-        if (r >= 0) {
-                /* check for changed config, every 3 seconds at most */
-                if (manager->last_usec == 0 ||
-                    (usec - manager->last_usec) > 3 * USEC_PER_SEC) {
-                        if (udev_rules_check_timestamp(manager->rules) ||
-                            udev_builtin_validate(manager->udev))
-                                manager_reload(manager);
+        assert_se(sd_event_now(manager->event, clock_boottime_or_monotonic(), &usec) >= 0);
+        /* check for changed config, every 3 seconds at most */
+        if (manager->last_usec == 0 ||
+            (usec - manager->last_usec) > 3 * USEC_PER_SEC) {
+                if (udev_rules_check_timestamp(manager->rules) ||
+                    udev_builtin_validate(manager->udev))
+                        manager_reload(manager);
 
-                        manager->last_usec = usec;
-                }
+                manager->last_usec = usec;
         }
 
         udev_builtin_init(manager->udev);