From 6bf0f408e4833152197fb38fb10a9989c89f3a59 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Feb 2016 18:38:03 +0100 Subject: core: make the StartLimitXYZ= settings generic and apply to any kind of unit, not just services This moves the StartLimitBurst=, StartLimitInterval=, StartLimitAction=, RebootArgument= from the [Service] section into the [Unit] section of unit files, and thus support it in all unit types, not just in services. This way we can enforce the start limit much earlier, in particular before testing the unit conditions, so that repeated start-up failure due to failed conditions is also considered for the start limit logic. For compatibility the four options may also be configured in the [Service] section still, but we only document them in their new section [Unit]. This also renamed the socket unit failure code "service-failed-permanent" into "service-start-limit-hit" to express more clearly what it is about, after all it's only triggered through the start limit being hit. Finally, the code in busname_trigger_notify() and socket_trigger_notify() is altered to become more alike. Fixes: #2467 --- src/core/busname.c | 24 ++++++++++-------- src/core/busname.h | 2 +- src/core/dbus-service.c | 9 ++++--- src/core/dbus-unit.c | 4 +++ src/core/load-fragment-gperf.gperf.m4 | 12 ++++++--- src/core/service.c | 27 +------------------- src/core/service.h | 4 --- src/core/socket.c | 47 ++++++++++------------------------- src/core/socket.h | 2 +- src/core/unit.c | 46 +++++++++++++++++++++++++++------- src/core/unit.h | 7 ++++++ 11 files changed, 90 insertions(+), 94 deletions(-) (limited to 'src') diff --git a/src/core/busname.c b/src/core/busname.c index 4b0cad8db4..a322252b96 100644 --- a/src/core/busname.c +++ b/src/core/busname.c @@ -945,7 +945,6 @@ static void busname_reset_failed(Unit *u) { static void busname_trigger_notify(Unit *u, Unit *other) { BusName *n = BUSNAME(u); - Service *s; assert(n); assert(other); @@ -953,19 +952,22 @@ static void busname_trigger_notify(Unit *u, Unit *other) { if (!IN_SET(n->state, BUSNAME_RUNNING, BUSNAME_LISTENING)) return; - if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE) + if (other->start_limit_hit) { + busname_enter_dead(n, BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT); return; + } - s = SERVICE(other); + if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE) + return; - if (s->state == SERVICE_FAILED && s->result == SERVICE_FAILURE_START_LIMIT) - busname_enter_dead(n, BUSNAME_FAILURE_SERVICE_FAILED_PERMANENT); - else if (IN_SET(s->state, - SERVICE_DEAD, SERVICE_FAILED, - SERVICE_STOP, SERVICE_STOP_SIGTERM, SERVICE_STOP_SIGKILL, - SERVICE_STOP_POST, SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, - SERVICE_AUTO_RESTART)) + if (IN_SET(SERVICE(other)->state, + SERVICE_DEAD, SERVICE_FAILED, + SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, + SERVICE_AUTO_RESTART)) busname_enter_listening(n); + + if (SERVICE(other)->state == SERVICE_RUNNING) + busname_set_state(n, BUSNAME_RUNNING); } static int busname_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { @@ -1006,7 +1008,7 @@ static const char* const busname_result_table[_BUSNAME_RESULT_MAX] = { [BUSNAME_FAILURE_EXIT_CODE] = "exit-code", [BUSNAME_FAILURE_SIGNAL] = "signal", [BUSNAME_FAILURE_CORE_DUMP] = "core-dump", - [BUSNAME_FAILURE_SERVICE_FAILED_PERMANENT] = "service-failed-permanent", + [BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit", }; DEFINE_STRING_TABLE_LOOKUP(busname_result, BusNameResult); diff --git a/src/core/busname.h b/src/core/busname.h index 46f7b6f097..bd37083695 100644 --- a/src/core/busname.h +++ b/src/core/busname.h @@ -33,7 +33,7 @@ typedef enum BusNameResult { BUSNAME_FAILURE_EXIT_CODE, BUSNAME_FAILURE_SIGNAL, BUSNAME_FAILURE_CORE_DUMP, - BUSNAME_FAILURE_SERVICE_FAILED_PERMANENT, + BUSNAME_FAILURE_SERVICE_START_LIMIT_HIT, _BUSNAME_RESULT_MAX, _BUSNAME_RESULT_INVALID = -1 } BusNameResult; diff --git a/src/core/dbus-service.c b/src/core/dbus-service.c index 16f50238a1..f36ce3f9fe 100644 --- a/src/core/dbus-service.c +++ b/src/core/dbus-service.c @@ -52,10 +52,11 @@ const sd_bus_vtable bus_service_vtable[] = { SD_BUS_PROPERTY("RuntimeMaxUSec", "t", bus_property_get_usec, offsetof(Service, runtime_max_usec), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("WatchdogUSec", "t", bus_property_get_usec, offsetof(Service, watchdog_usec), SD_BUS_VTABLE_PROPERTY_CONST), BUS_PROPERTY_DUAL_TIMESTAMP("WatchdogTimestamp", offsetof(Service, watchdog_timestamp), 0), - SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Service, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Service, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Service, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST), - SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Service, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST), + /* The following four are obsolete, and thus marked hidden here. They moved into the Unit interface */ + SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), + SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Unit, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), + SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Unit, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), + SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Unit, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST|SD_BUS_VTABLE_HIDDEN), SD_BUS_PROPERTY("FailureAction", "s", property_get_failure_action, offsetof(Service, failure_action), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("PermissionsStartOnly", "b", bus_property_get_bool, offsetof(Service, permissions_start_only), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("RootDirectoryStartOnly", "b", bus_property_get_bool, offsetof(Service, root_directory_start_only), SD_BUS_VTABLE_PROPERTY_CONST), diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index d7929e5566..47f8ba5362 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -704,6 +704,10 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_PROPERTY("LoadError", "(ss)", property_get_load_error, 0, SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("Transient", "b", bus_property_get_bool, offsetof(Unit, transient), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("NetClass", "u", NULL, offsetof(Unit, cgroup_netclass_id), 0), + SD_BUS_PROPERTY("StartLimitInterval", "t", bus_property_get_usec, offsetof(Unit, start_limit.interval), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StartLimitBurst", "u", bus_property_get_unsigned, offsetof(Unit, start_limit.burst), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("StartLimitAction", "s", property_get_failure_action, offsetof(Unit, start_limit_action), SD_BUS_VTABLE_PROPERTY_CONST), + SD_BUS_PROPERTY("RebootArgument", "s", NULL, offsetof(Unit, reboot_arg), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_METHOD("Start", "s", "o", method_start, SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("Stop", "s", "o", method_stop, SD_BUS_VTABLE_UNPRIVILEGED), diff --git a/src/core/load-fragment-gperf.gperf.m4 b/src/core/load-fragment-gperf.gperf.m4 index 2507db1932..c2a47c7b11 100644 --- a/src/core/load-fragment-gperf.gperf.m4 +++ b/src/core/load-fragment-gperf.gperf.m4 @@ -164,6 +164,10 @@ Unit.IgnoreOnSnapshot, config_parse_warn_compat, DISABLED_LE Unit.JobTimeoutSec, config_parse_sec, 0, offsetof(Unit, job_timeout) Unit.JobTimeoutAction, config_parse_failure_action, 0, offsetof(Unit, job_timeout_action) Unit.JobTimeoutRebootArgument, config_parse_string, 0, offsetof(Unit, job_timeout_reboot_arg) +Unit.StartLimitInterval, config_parse_sec, 0, offsetof(Unit, start_limit.interval) +Unit.StartLimitBurst, config_parse_unsigned, 0, offsetof(Unit, start_limit.burst) +Unit.StartLimitAction, config_parse_failure_action, 0, offsetof(Unit, start_limit_action) +Unit.RebootArgument, config_parse_string, 0, offsetof(Unit, reboot_arg) Unit.ConditionPathExists, config_parse_unit_condition_path, CONDITION_PATH_EXISTS, offsetof(Unit, conditions) Unit.ConditionPathExistsGlob, config_parse_unit_condition_path, CONDITION_PATH_EXISTS_GLOB, offsetof(Unit, conditions) Unit.ConditionPathIsDirectory, config_parse_unit_condition_path, CONDITION_PATH_IS_DIRECTORY, offsetof(Unit, conditions) @@ -216,10 +220,10 @@ Service.TimeoutStartSec, config_parse_service_timeout, 0, Service.TimeoutStopSec, config_parse_service_timeout, 0, offsetof(Service, timeout_stop_usec) Service.RuntimeMaxSec, config_parse_sec, 0, offsetof(Service, runtime_max_usec) Service.WatchdogSec, config_parse_sec, 0, offsetof(Service, watchdog_usec) -Service.StartLimitInterval, config_parse_sec, 0, offsetof(Service, start_limit.interval) -Service.StartLimitBurst, config_parse_unsigned, 0, offsetof(Service, start_limit.burst) -Service.StartLimitAction, config_parse_failure_action, 0, offsetof(Service, start_limit_action) -Service.RebootArgument, config_parse_string, 0, offsetof(Service, reboot_arg) +Service.StartLimitInterval, config_parse_sec, 0, offsetof(Unit, start_limit.interval) +Service.StartLimitBurst, config_parse_unsigned, 0, offsetof(Unit, start_limit.burst) +Service.StartLimitAction, config_parse_failure_action, 0, offsetof(Unit, start_limit_action) +Service.RebootArgument, config_parse_string, 0, offsetof(Unit, reboot_arg) Service.FailureAction, config_parse_failure_action, 0, offsetof(Service, failure_action) Service.Type, config_parse_service_type, 0, offsetof(Service, type) Service.Restart, config_parse_service_restart, 0, offsetof(Service, restart) diff --git a/src/core/service.c b/src/core/service.c index 02ce1a566a..3bb3cc8b18 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -119,8 +119,6 @@ static void service_init(Unit *u) { s->stdin_fd = s->stdout_fd = s->stderr_fd = -1; s->guess_main_pid = true; - RATELIMIT_INIT(s->start_limit, u->manager->default_start_limit_interval, u->manager->default_start_limit_burst); - s->control_command_id = _SERVICE_EXEC_COMMAND_INVALID; } @@ -302,7 +300,6 @@ static void service_done(Unit *u) { s->pid_file = mfree(s->pid_file); s->status_text = mfree(s->status_text); - s->reboot_arg = mfree(s->reboot_arg); s->exec_runtime = exec_runtime_unref(s->exec_runtime); exec_command_free_array(s->exec_command, _SERVICE_EXEC_COMMAND_MAX); @@ -1422,7 +1419,7 @@ static void service_enter_dead(Service *s, ServiceResult f, bool allow_restart) if (s->result != SERVICE_SUCCESS) { log_unit_warning(UNIT(s), "Failed with result '%s'.", service_result_to_string(s->result)); - failure_action(UNIT(s)->manager, s->failure_action, s->reboot_arg); + failure_action(UNIT(s)->manager, s->failure_action, UNIT(s)->reboot_arg); } if (allow_restart && service_shall_restart(s)) { @@ -1989,20 +1986,8 @@ fail: service_enter_stop(s, SERVICE_FAILURE_RESOURCES); } -static int service_start_limit_test(Service *s) { - assert(s); - - if (ratelimit_test(&s->start_limit)) - return 0; - - log_unit_warning(UNIT(s), "Start request repeated too quickly."); - - return failure_action(UNIT(s)->manager, s->start_limit_action, s->reboot_arg); -} - static int service_start(Unit *u) { Service *s = SERVICE(u); - int r; assert(s); @@ -2029,13 +2014,6 @@ static int service_start(Unit *u) { assert(IN_SET(s->state, SERVICE_DEAD, SERVICE_FAILED)); - /* Make sure we don't enter a busy loop of some kind. */ - r = service_start_limit_test(s); - if (r < 0) { - service_enter_dead(s, SERVICE_FAILURE_START_LIMIT, false); - return r; - } - s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; s->main_pid_known = false; @@ -3248,8 +3226,6 @@ static void service_reset_failed(Unit *u) { s->result = SERVICE_SUCCESS; s->reload_result = SERVICE_SUCCESS; - - RATELIMIT_RESET(s->start_limit); } static int service_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { @@ -3317,7 +3293,6 @@ static const char* const service_result_table[_SERVICE_RESULT_MAX] = { [SERVICE_FAILURE_SIGNAL] = "signal", [SERVICE_FAILURE_CORE_DUMP] = "core-dump", [SERVICE_FAILURE_WATCHDOG] = "watchdog", - [SERVICE_FAILURE_START_LIMIT] = "start-limit" }; DEFINE_STRING_TABLE_LOOKUP(service_result, ServiceResult); diff --git a/src/core/service.h b/src/core/service.h index 24408940d4..55fcec48c7 100644 --- a/src/core/service.h +++ b/src/core/service.h @@ -88,7 +88,6 @@ typedef enum ServiceResult { SERVICE_FAILURE_SIGNAL, SERVICE_FAILURE_CORE_DUMP, SERVICE_FAILURE_WATCHDOG, - SERVICE_FAILURE_START_LIMIT, _SERVICE_RESULT_MAX, _SERVICE_RESULT_INVALID = -1 } ServiceResult; @@ -178,10 +177,7 @@ struct Service { char *status_text; int status_errno; - RateLimit start_limit; - FailureAction start_limit_action; FailureAction failure_action; - char *reboot_arg; UnitRef accept_socket; diff --git a/src/core/socket.c b/src/core/socket.c index f0d65577e3..1f24f289e0 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -2702,23 +2702,6 @@ static void socket_reset_failed(Unit *u) { s->result = SOCKET_SUCCESS; } -static void socket_notify_service_dead(Socket *s, bool failed_permanent) { - assert(s); - - /* The service is dead. Dang! - * - * This is strictly for one-instance-for-all-connections - * services. */ - - if (s->state == SOCKET_RUNNING) { - log_unit_debug(UNIT(s), "Got notified about service death (failed permanently: %s)", yes_no(failed_permanent)); - if (failed_permanent) - socket_enter_stop_pre(s, SOCKET_FAILURE_SERVICE_FAILED_PERMANENT); - else - socket_enter_listening(s); - } -} - void socket_connection_unref(Socket *s) { assert(s); @@ -2735,34 +2718,30 @@ void socket_connection_unref(Socket *s) { static void socket_trigger_notify(Unit *u, Unit *other) { Socket *s = SOCKET(u); - Service *se; assert(u); assert(other); /* Don't propagate state changes from the service if we are already down or accepting connections */ - if ((s->state != SOCKET_RUNNING && - s->state != SOCKET_LISTENING) || - s->accept) + if (!IN_SET(s->state, SOCKET_RUNNING, SOCKET_LISTENING) || s->accept) return; - if (other->load_state != UNIT_LOADED || - other->type != UNIT_SERVICE) + if (other->start_limit_hit) { + socket_enter_stop_pre(s, SOCKET_FAILURE_SERVICE_START_LIMIT_HIT); return; + } - se = SERVICE(other); - - if (se->state == SERVICE_FAILED) - socket_notify_service_dead(s, se->result == SERVICE_FAILURE_START_LIMIT); + if (other->load_state != UNIT_LOADED || other->type != UNIT_SERVICE) + return; - if (se->state == SERVICE_DEAD || - se->state == SERVICE_FINAL_SIGTERM || - se->state == SERVICE_FINAL_SIGKILL || - se->state == SERVICE_AUTO_RESTART) - socket_notify_service_dead(s, false); + if (IN_SET(SERVICE(other)->state, + SERVICE_DEAD, SERVICE_FAILED, + SERVICE_FINAL_SIGTERM, SERVICE_FINAL_SIGKILL, + SERVICE_AUTO_RESTART)) + socket_enter_listening(s); - if (se->state == SERVICE_RUNNING) + if (SERVICE(other)->state == SERVICE_RUNNING) socket_set_state(s, SOCKET_RUNNING); } @@ -2818,7 +2797,7 @@ static const char* const socket_result_table[_SOCKET_RESULT_MAX] = { [SOCKET_FAILURE_EXIT_CODE] = "exit-code", [SOCKET_FAILURE_SIGNAL] = "signal", [SOCKET_FAILURE_CORE_DUMP] = "core-dump", - [SOCKET_FAILURE_SERVICE_FAILED_PERMANENT] = "service-failed-permanent" + [SOCKET_FAILURE_SERVICE_START_LIMIT_HIT] = "service-start-limit-hit" }; DEFINE_STRING_TABLE_LOOKUP(socket_result, SocketResult); diff --git a/src/core/socket.h b/src/core/socket.h index 08033287a6..ca3f815a47 100644 --- a/src/core/socket.h +++ b/src/core/socket.h @@ -54,7 +54,7 @@ typedef enum SocketResult { SOCKET_FAILURE_EXIT_CODE, SOCKET_FAILURE_SIGNAL, SOCKET_FAILURE_CORE_DUMP, - SOCKET_FAILURE_SERVICE_FAILED_PERMANENT, + SOCKET_FAILURE_SERVICE_START_LIMIT_HIT, _SOCKET_RESULT_MAX, _SOCKET_RESULT_INVALID = -1 } SocketResult; diff --git a/src/core/unit.c b/src/core/unit.c index be4dac8fea..9df951badb 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -101,6 +101,7 @@ Unit *unit_new(Manager *m, size_t size) { u->cgroup_inotify_wd = -1; u->job_timeout = USEC_INFINITY; + RATELIMIT_INIT(u->start_limit, m->default_start_limit_interval, m->default_start_limit_burst); RATELIMIT_INIT(u->auto_stop_ratelimit, 10 * USEC_PER_SEC, 16); return u; @@ -559,6 +560,8 @@ void unit_free(Unit *u) { condition_free_list(u->conditions); condition_free_list(u->asserts); + free(u->reboot_arg); + unit_ref_unset(&u->slice); while (u->refs) @@ -1446,23 +1449,36 @@ void unit_status_emit_starting_stopping_reloading(Unit *u, JobType t) { unit_status_print_starting_stopping(u, t); } +static int unit_start_limit_test(Unit *u) { + assert(u); + + if (ratelimit_test(&u->start_limit)) { + u->start_limit_hit = false; + return 0; + } + + log_unit_warning(u, "Start request repeated too quickly."); + u->start_limit_hit = true; + + return failure_action(u->manager, u->start_limit_action, u->reboot_arg); +} + /* Errors: - * -EBADR: This unit type does not support starting. - * -EALREADY: Unit is already started. - * -EAGAIN: An operation is already in progress. Retry later. - * -ECANCELED: Too many requests for now. - * -EPROTO: Assert failed + * -EBADR: This unit type does not support starting. + * -EALREADY: Unit is already started. + * -EAGAIN: An operation is already in progress. Retry later. + * -ECANCELED: Too many requests for now. + * -EPROTO: Assert failed + * -EINVAL: Unit not loaded + * -EOPNOTSUPP: Unit type not supported */ int unit_start(Unit *u) { UnitActiveState state; Unit *following; + int r; assert(u); - /* Units that aren't loaded cannot be started */ - if (u->load_state != UNIT_LOADED) - return -EINVAL; - /* If this is already started, then this will succeed. Note * that this will even succeed if this unit is not startable * by the user. This is relied on to detect when we need to @@ -1471,6 +1487,15 @@ int unit_start(Unit *u) { if (UNIT_IS_ACTIVE_OR_RELOADING(state)) return -EALREADY; + /* Make sure we don't enter a busy loop of some kind. */ + r = unit_start_limit_test(u); + if (r < 0) + return r; + + /* Units that aren't loaded cannot be started */ + if (u->load_state != UNIT_LOADED) + return -EINVAL; + /* If the conditions failed, don't do anything at all. If we * already are activating this call might still be useful to * speed up activation in case there is some hold-off time, @@ -2988,6 +3013,9 @@ void unit_reset_failed(Unit *u) { if (UNIT_VTABLE(u)->reset_failed) UNIT_VTABLE(u)->reset_failed(u); + + RATELIMIT_RESET(u->start_limit); + u->start_limit_hit = false; } Unit *unit_following(Unit *u) { diff --git a/src/core/unit.h b/src/core/unit.h index 8712e03133..a87ef74fb3 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -167,6 +167,11 @@ struct Unit { /* Error code when we didn't manage to load the unit (negative) */ int load_error; + /* Put a ratelimit on unit starting */ + RateLimit start_limit; + FailureAction start_limit_action; + char *reboot_arg; + /* Make sure we never enter endless loops with the check unneeded logic, or the BindsTo= logic */ RateLimit auto_stop_ratelimit; @@ -230,6 +235,8 @@ struct Unit { bool cgroup_members_mask_valid:1; bool cgroup_subtree_mask_valid:1; + bool start_limit_hit:1; + /* Did we already invoke unit_coldplug() for this unit? */ bool coldplugged:1; }; -- cgit v1.2.3-54-g00ecf From 04a1d84cefe4dbb5bfee86190489c3c07a8c238c Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 9 Feb 2016 19:55:04 +0100 Subject: util-lib: make sure usec_sub() doesn't degrade USEC_INFINITY As suggested in: https://github.com/systemd/systemd/pull/2542#issuecomment-181877820 --- src/basic/time-util.h | 14 ++++++++------ src/test/test-time.c | 13 +++++++++++++ 2 files changed, 21 insertions(+), 6 deletions(-) (limited to 'src') diff --git a/src/basic/time-util.h b/src/basic/time-util.h index 87440faeee..080c1110f5 100644 --- a/src/basic/time-util.h +++ b/src/basic/time-util.h @@ -143,11 +143,13 @@ static inline usec_t usec_add(usec_t a, usec_t b) { static inline usec_t usec_sub(usec_t timestamp, int64_t delta) { if (delta < 0) - timestamp = usec_add(timestamp, (usec_t) (-delta)); - else if (timestamp > (usec_t) delta) - timestamp -= delta; - else - timestamp = 0; + return usec_add(timestamp, (usec_t) (-delta)); - return timestamp; + if (timestamp == USEC_INFINITY) /* Make sure infinity doesn't degrade */ + return USEC_INFINITY; + + if (timestamp < (usec_t) delta) + return 0; + + return timestamp - delta; } diff --git a/src/test/test-time.c b/src/test/test-time.c index 254a8d0e52..fdbf9fa881 100644 --- a/src/test/test-time.c +++ b/src/test/test-time.c @@ -191,6 +191,18 @@ static void test_usec_add(void) { assert_se(usec_add(USEC_INFINITY, 2) == USEC_INFINITY); } +static void test_usec_sub(void) { + assert_se(usec_sub(0, 0) == 0); + assert_se(usec_sub(4, 1) == 3); + assert_se(usec_sub(4, 4) == 0); + assert_se(usec_sub(4, 5) == 0); + assert_se(usec_sub(USEC_INFINITY-3, -3) == USEC_INFINITY); + assert_se(usec_sub(USEC_INFINITY-3, -3) == USEC_INFINITY); + assert_se(usec_sub(USEC_INFINITY-3, -4) == USEC_INFINITY); + assert_se(usec_sub(USEC_INFINITY-3, -5) == USEC_INFINITY); + assert_se(usec_sub(USEC_INFINITY, 5) == USEC_INFINITY); +} + int main(int argc, char *argv[]) { uintmax_t x; @@ -203,6 +215,7 @@ int main(int argc, char *argv[]) { test_timezone_is_valid(); test_get_timezones(); test_usec_add(); + test_usec_sub(); /* Ensure time_t is signed */ assert_cc((time_t) -1 < (time_t) 1); -- cgit v1.2.3-54-g00ecf