From 38a03f06a7393d2721c23f23f0589d2f6d0904af Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Mon, 3 Aug 2015 17:29:09 +0200 Subject: [PATCH] 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. --- src/libsystemd-network/sd-dhcp-server.c | 2 +- src/libsystemd-network/sd-dhcp6-client.c | 11 +++------- src/libsystemd-network/sd-ipv4ll.c | 3 +-- src/libsystemd-network/sd-pppoe.c | 4 +--- src/libsystemd/sd-event/sd-event.c | 9 +++++--- src/resolve/resolved-dns-transaction.c | 8 ++----- src/udev/udevd.c | 28 +++++++++--------------- 7 files changed, 24 insertions(+), 41 deletions(-) diff --git a/src/libsystemd-network/sd-dhcp-server.c b/src/libsystemd-network/sd-dhcp-server.c index cc5e032344..d2cc3194b3 100644 --- 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 85162dc555..e2f5862851 100644 --- 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 9e04db96bb..0cb77ccf71 100644 --- 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 1de8a5e8bf..ff064f563f 100644 --- 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 76964aa0cc..754fb7614e 100644 --- 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 487b2c5162..53779f3372 100644 --- 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 d0b8bad48e..28ac44fb8e 100644 --- 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);