Merge pull request #17567 from keszybz/various-small-cleanups

Various small cleanups

17 files changed, 96 insertions, 115 deletions

diff --git a/src/basic/env-util.c b/src/basic/env-util.c
index bf191044c03..a84863ff225 100644
--- a/src/basic/env-util.c
+++ b/src/basic/env-util.c
@@ -747,3 +747,15 @@ int getenv_bool_secure(const char *p) {
 
         return parse_boolean(e);
 }
+
+int set_unset_env(const char *name, const char *value, bool overwrite) {
+        int r;
+
+        if (value)
+                r = setenv(name, value, overwrite);
+        else
+                r = unsetenv(name);
+        if (r < 0)
+                return -errno;
+        return 0;
+}
diff --git a/src/basic/env-util.h b/src/basic/env-util.h
index a37603dbd8d..6684b3350f0 100644
--- a/src/basic/env-util.h
+++ b/src/basic/env-util.h
@@ -52,3 +52,6 @@ char *strv_env_get(char **x, const char *n) _pure_;
 
 int getenv_bool(const char *p);
 int getenv_bool_secure(const char *p);
+
+/* Like setenv, but calls unsetenv if value == NULL. */
+int set_unset_env(const char *name, const char *value, bool overwrite);
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 71a2654507f..973756c891f 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -117,7 +117,7 @@ int write_string_stream_ts(
                 FILE *f,
                 const char *line,
                 WriteStringFileFlags flags,
-                struct timespec *ts) {
+                const struct timespec *ts) {
 
         bool needs_nl;
         int r, fd;
@@ -161,7 +161,7 @@ int write_string_stream_ts(
                 return r;
 
         if (ts) {
-                struct timespec twice[2] = {*ts, *ts};
+                const struct timespec twice[2] = {*ts, *ts};
 
                 if (futimens(fd, twice) < 0)
                         return -errno;
@@ -174,7 +174,7 @@ static int write_string_file_atomic(
                 const char *fn,
                 const char *line,
                 WriteStringFileFlags flags,
-                struct timespec *ts) {
+                const struct timespec *ts) {
 
         _cleanup_fclose_ FILE *f = NULL;
         _cleanup_free_ char *p = NULL;
@@ -221,7 +221,7 @@ int write_string_file_ts(
                 const char *fn,
                 const char *line,
                 WriteStringFileFlags flags,
-                struct timespec *ts) {
+                const struct timespec *ts) {
 
         _cleanup_fclose_ FILE *f = NULL;
         int q, r, fd;
diff --git a/src/basic/fileio.h b/src/basic/fileio.h
index b34ed5e88b6..0886354cbcf 100644
--- a/src/basic/fileio.h
+++ b/src/basic/fileio.h
@@ -48,11 +48,11 @@ DIR* take_fdopendir(int *dfd);
 FILE* open_memstream_unlocked(char **ptr, size_t *sizeloc);
 FILE* fmemopen_unlocked(void *buf, size_t size, const char *mode);
 
-int write_string_stream_ts(FILE *f, const char *line, WriteStringFileFlags flags, struct timespec *ts);
+int write_string_stream_ts(FILE *f, const char *line, WriteStringFileFlags flags, const struct timespec *ts);
 static inline int write_string_stream(FILE *f, const char *line, WriteStringFileFlags flags) {
         return write_string_stream_ts(f, line, flags, NULL);
 }
-int write_string_file_ts(const char *fn, const char *line, WriteStringFileFlags flags, struct timespec *ts);
+int write_string_file_ts(const char *fn, const char *line, WriteStringFileFlags flags, const struct timespec *ts);
 static inline int write_string_file(const char *fn, const char *line, WriteStringFileFlags flags) {
         return write_string_file_ts(fn, line, flags, NULL);
 }
diff --git a/src/core/main.c b/src/core/main.c
index c08f541bc13..a280b756ffd 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -1418,9 +1418,8 @@ static int fixup_environment(void) {
                 return -errno;
 
         /* The kernels sets HOME=/ for init. Let's undo this. */
-        if (path_equal_ptr(getenv("HOME"), "/") &&
-            unsetenv("HOME") < 0)
-                log_warning_errno(errno, "Failed to unset $HOME: %m");
+        if (path_equal_ptr(getenv("HOME"), "/"))
+                assert_se(unsetenv("HOME") == 0);
 
         return 0;
 }
diff --git a/src/home/homectl.c b/src/home/homectl.c
index 470e05b24ea..7cfda7ed2fd 100644
--- a/src/home/homectl.c
+++ b/src/home/homectl.c
@@ -215,9 +215,7 @@ static int acquire_existing_password(const char *user_name, UserRecord *hr, bool
                         return log_error_errno(r, "Failed to store password: %m");
 
                 string_erase(e);
-
-                if (unsetenv("PASSWORD") < 0)
-                        return log_error_errno(errno, "Failed to unset $PASSWORD: %m");
+                assert_se(unsetenv("PASSWORD") == 0);
 
                 return 0;
         }
@@ -255,9 +253,7 @@ static int acquire_token_pin(const char *user_name, UserRecord *hr) {
                         return log_error_errno(r, "Failed to store token PIN: %m");
 
                 string_erase(e);
-
-                if (unsetenv("PIN") < 0)
-                        return log_error_errno(errno, "Failed to unset $PIN: %m");
+                assert_se(unsetenv("PIN") == 0);
 
                 return 0;
         }
@@ -997,9 +993,7 @@ static int acquire_new_password(
                         return log_error_errno(r, "Failed to store password: %m");
 
                 string_erase(e);
-
-                if (unsetenv("NEWPASSWORD") < 0)
-                        return log_error_errno(errno, "Failed to unset $NEWPASSWORD: %m");
+                assert_se(unsetenv("NEWPASSWORD") == 0);
 
                 if (ret)
                         *ret = TAKE_PTR(copy);
diff --git a/src/libsystemd-network/dhcp-network.c b/src/libsystemd-network/dhcp-network.c
index 6c82d5508ec..656482bf838 100644
--- a/src/libsystemd-network/dhcp-network.c
+++ b/src/libsystemd-network/dhcp-network.c
@@ -106,7 +106,7 @@ static int _bind_raw_socket(int ifindex, union sockaddr_union *link,
                 .sll_hatype = htobe16(arp_type),
                 .sll_halen = bcast_addr_len,
         };
-        memcpy(link->ll.sll_addr, bcast_addr, bcast_addr_len);
+        memcpy(link->ll.sll_addr, bcast_addr, bcast_addr_len); /* We may overflow link->ll. link->ll_buffer ensures we have enough space. */
 
         r = bind(s, &link->sa, SOCKADDR_LL_LEN(link->ll));
         if (r < 0)
diff --git a/src/libsystemd/sd-daemon/sd-daemon.c b/src/libsystemd/sd-daemon/sd-daemon.c
index 6336a0cce47..6f0b975627e 100644
--- a/src/libsystemd/sd-daemon/sd-daemon.c
+++ b/src/libsystemd/sd-daemon/sd-daemon.c
@@ -30,13 +30,12 @@
 #define SNDBUF_SIZE (8*1024*1024)
 
 static void unsetenv_all(bool unset_environment) {
-
         if (!unset_environment)
                 return;
 
-        unsetenv("LISTEN_PID");
-        unsetenv("LISTEN_FDS");
-        unsetenv("LISTEN_FDNAMES");
+        assert_se(unsetenv("LISTEN_PID") == 0);
+        assert_se(unsetenv("LISTEN_FDS") == 0);
+        assert_se(unsetenv("LISTEN_FDNAMES") == 0);
 }
 
 _public_ int sd_listen_fds(int unset_environment) {
@@ -548,7 +547,7 @@ _public_ int sd_pid_notify_with_fds(
 
 finish:
         if (unset_environment)
-                unsetenv("NOTIFY_SOCKET");
+                assert_se(unsetenv("NOTIFY_SOCKET") == 0);
 
         return r;
 }
@@ -672,9 +671,9 @@ _public_ int sd_watchdog_enabled(int unset_environment, uint64_t *usec) {
 
 finish:
         if (unset_environment && s)
-                unsetenv("WATCHDOG_USEC");
+                assert_se(unsetenv("WATCHDOG_USEC") == 0);
         if (unset_environment && p)
-                unsetenv("WATCHDOG_PID");
+                assert_se(unsetenv("WATCHDOG_PID") == 0);
 
         return r;
 }
diff --git a/src/libsystemd/sd-event/sd-event.c b/src/libsystemd/sd-event/sd-event.c
index bbb2b6bfd74..7a3f89ed37e 100644
--- a/src/libsystemd/sd-event/sd-event.c
+++ b/src/libsystemd/sd-event/sd-event.c
@@ -402,8 +402,7 @@ static int source_io_register(
 
         if (epoll_ctl(s->event->epoll_fd,
                       s->io.registered ? EPOLL_CTL_MOD : EPOLL_CTL_ADD,
-                      s->io.fd,
-                      &ev) < 0)
+                      s->io.fd, &ev) < 0)
                 return -errno;
 
         s->io.registered = true;
@@ -430,8 +429,6 @@ static void source_child_pidfd_unregister(sd_event_source *s) {
 }
 
 static int source_child_pidfd_register(sd_event_source *s, int enabled) {
-        int r;
-
         assert(s);
         assert(s->type == SOURCE_CHILD);
         assert(enabled != SD_EVENT_OFF);
@@ -442,11 +439,9 @@ static int source_child_pidfd_register(sd_event_source *s, int enabled) {
                         .data.ptr = s,
                 };
 
-                if (s->child.registered)
-                        r = epoll_ctl(s->event->epoll_fd, EPOLL_CTL_MOD, s->child.pidfd, &ev);
-                else
-                        r = epoll_ctl(s->event->epoll_fd, EPOLL_CTL_ADD, s->child.pidfd, &ev);
-                if (r < 0)
+                if (epoll_ctl(s->event->epoll_fd,
+                              s->child.registered ? EPOLL_CTL_MOD : EPOLL_CTL_ADD,
+                              s->child.pidfd, &ev) < 0)
                         return -errno;
         }
 
@@ -1340,31 +1335,25 @@ _public_ int sd_event_add_child(
         if (r < 0)
                 return r;
 
-        e->n_enabled_child_sources++;
-
         if (EVENT_SOURCE_WATCH_PIDFD(s)) {
                 /* We have a pidfd and we only want to watch for exit */
-
                 r = source_child_pidfd_register(s, s->enabled);
-                if (r < 0) {
-                        e->n_enabled_child_sources--;
+                if (r < 0)
                         return r;
-                }
+
         } else {
                 /* We have no pidfd or we shall wait for some other event than WEXITED */
-
                 r = event_make_signal_data(e, SIGCHLD, NULL);
-                if (r < 0) {
-                        e->n_enabled_child_sources--;
+                if (r < 0)
                         return r;
-                }
 
                 e->need_process_child = true;
         }
 
+        e->n_enabled_child_sources++;
+
         if (ret)
                 *ret = s;
-
         TAKE_PTR(s);
         return 0;
 }
@@ -1429,31 +1418,24 @@ _public_ int sd_event_add_child_pidfd(
         if (r < 0)
                 return r;
 
-        e->n_enabled_child_sources++;
-
         if (EVENT_SOURCE_WATCH_PIDFD(s)) {
                 /* We only want to watch for WEXITED */
-
                 r = source_child_pidfd_register(s, s->enabled);
-                if (r < 0) {
-                        e->n_enabled_child_sources--;
+                if (r < 0)
                         return r;
-                }
         } else {
                 /* We shall wait for some other event than WEXITED */
-
                 r = event_make_signal_data(e, SIGCHLD, NULL);
-                if (r < 0) {
-                        e->n_enabled_child_sources--;
+                if (r < 0)
                         return r;
-                }
 
                 e->need_process_child = true;
         }
 
+        e->n_enabled_child_sources++;
+
         if (ret)
                 *ret = s;
-
         TAKE_PTR(s);
         return 0;
 }
@@ -2311,11 +2293,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 +2307,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;
                 }
@@ -2357,35 +2324,32 @@ static int event_source_enable(sd_event_source *s, int m) {
                 break;
 
         case SOURCE_CHILD:
-                s->event->n_enabled_child_sources++;
-
                 if (EVENT_SOURCE_WATCH_PIDFD(s)) {
                         /* yes, we have pidfd */
 
-                        r = source_child_pidfd_register(s, s->enabled);
-                        if (r < 0) {
-                                s->enabled = SD_EVENT_OFF;
-                                s->event->n_enabled_child_sources--;
+                        r = source_child_pidfd_register(s, enable);
+                        if (r < 0)
                                 return r;
-                        }
                 } else {
                         /* no pidfd, or something other to watch for than WEXITED */
 
                         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;
                         }
                 }
 
-                break;
+                s->event->n_enabled_child_sources++;
 
-        case SOURCE_EXIT:
-                prioq_reshuffle(s->event->exit, s, &s->exit.prioq_index);
                 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:
         case SOURCE_DEFER:
         case SOURCE_POST:
         case SOURCE_INOTIFY:
@@ -2395,6 +2359,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;
 }
 
diff --git a/src/shared/pager.c b/src/shared/pager.c
index cd8a840e578..f689d9f28f0 100644
--- a/src/shared/pager.c
+++ b/src/shared/pager.c
@@ -189,12 +189,9 @@ int pager_open(PagerFlags flags) {
 
                 /* We generally always set variables used by less, even if we end up using a different pager.
                  * They shouldn't hurt in any case, and ideally other pagers would look at them too. */
-                if (use_secure_mode)
-                        r = setenv("LESSSECURE", "1", 1);
-                else
-                        r = unsetenv("LESSSECURE");
+                r = set_unset_env("LESSSECURE", use_secure_mode ? "1" : NULL, true);
                 if (r < 0) {
-                        log_error_errno(errno, "Failed to adjust environment variable LESSSECURE: %m");
+                        log_error_errno(r, "Failed to adjust environment variable LESSSECURE: %m");
                         _exit(EXIT_FAILURE);
                 }
 
diff --git a/src/test/test-exec-util.c b/src/test/test-exec-util.c
index 5da69f7321e..e9e8e210405 100644
--- a/src/test/test-exec-util.c
+++ b/src/test/test-exec-util.c
@@ -372,10 +372,7 @@ static void test_environment_gathering(void) {
         assert_se(streq(strv_env_get(env, "PATH"), DEFAULT_PATH ":/no/such/file"));
 
         /* reset environ PATH */
-        if (old)
-                (void) setenv("PATH", old, 1);
-        else
-                (void) unsetenv("PATH");
+        assert_se(set_unset_env("PATH", old, true) == 0);
 }
 
 static void test_error_catching(void) {
diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index e15b7a55fa0..3b6a4be2602 100644
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -898,11 +898,11 @@ int main(int argc, char *argv[]) {
         }
 #endif
 
-        (void) unsetenv("USER");
-        (void) unsetenv("LOGNAME");
-        (void) unsetenv("SHELL");
-        (void) unsetenv("HOME");
-        (void) unsetenv("TMPDIR");
+        assert_se(unsetenv("USER") == 0);
+        assert_se(unsetenv("LOGNAME") == 0);
+        assert_se(unsetenv("SHELL") == 0);
+        assert_se(unsetenv("HOME") == 0);
+        assert_se(unsetenv("TMPDIR") == 0);
 
         can_unshare = have_namespaces();
 
diff --git a/src/test/test-path-util.c b/src/test/test-path-util.c
index 36108f548b7..f4f8d0550b7 100644
--- a/src/test/test-path-util.c
+++ b/src/test/test-path-util.c
@@ -184,7 +184,7 @@ static void test_find_executable_full(void) {
         if (p)
                 assert_se(oldpath = strdup(p));
 
-        assert_se(unsetenv("PATH") >= 0);
+        assert_se(unsetenv("PATH") == 0);
 
         assert_se(find_executable_full("sh", true, &p) == 0);
         puts(p);
@@ -347,7 +347,7 @@ static void test_fsck_exists(void) {
         log_info("/* %s */", __func__);
 
         /* Ensure we use a sane default for PATH. */
-        unsetenv("PATH");
+        assert_se(unsetenv("PATH") == 0);
 
         /* fsck.minix is provided by util-linux and will probably exist. */
         assert_se(fsck_exists("minix") == 1);
diff --git a/src/test/test-time-util.c b/src/test/test-time-util.c
index cfe8753441c..cc391e81a05 100644
--- a/src/test/test-time-util.c
+++ b/src/test/test-time-util.c
@@ -480,7 +480,7 @@ static void test_in_utc_timezone(void) {
         assert_se(streq(tzname[0], "CET"));
         assert_se(streq(tzname[1], "CEST"));
 
-        assert_se(unsetenv("TZ") >= 0);
+        assert_se(unsetenv("TZ") == 0);
 }
 
 static void test_map_clock_usec(void) {
diff --git a/src/timedate/timedatectl.c b/src/timedate/timedatectl.c
index 455c602c30f..abc792a4525 100644
--- a/src/timedate/timedatectl.c
+++ b/src/timedate/timedatectl.c
@@ -12,6 +12,7 @@
 #include "bus-locator.h"
 #include "bus-map-properties.h"
 #include "bus-print-properties.h"
+#include "env-util.h"
 #include "format-table.h"
 #include "in-addr-util.h"
 #include "main-func.h"
@@ -139,12 +140,9 @@ static int print_status_info(const StatusInfo *i) {
 
 
         /* Restore the $TZ */
-        if (old_tz)
-                r = setenv("TZ", old_tz, true);
-        else
-                r = unsetenv("TZ");
+        r = set_unset_env("TZ", old_tz, true);
         if (r < 0)
-                log_warning_errno(errno, "Failed to set TZ environment variable, ignoring: %m");
+                log_warning_errno(r, "Failed to set TZ environment variable, ignoring: %m");
         else
                 tzset();
 
diff --git a/src/udev/udevd.c b/src/udev/udevd.c
index b6544988c64..d24b8d43985 100644
--- a/src/udev/udevd.c
+++ b/src/udev/udevd.c
@@ -565,7 +565,7 @@ static int worker_main(Manager *_manager, sd_device_monitor *monitor, sd_device
         assert(monitor);
         assert(dev);
 
-        unsetenv("NOTIFY_SOCKET");
+        assert_se(unsetenv("NOTIFY_SOCKET") == 0);
 
         assert_se(sigprocmask_many(SIG_BLOCK, NULL, SIGTERM, -1) >= 0);
 
diff --git a/src/userdb/userdbctl.c b/src/userdb/userdbctl.c
index 0d0b2870abc..a0e22dff559 100644
--- a/src/userdb/userdbctl.c
+++ b/src/userdb/userdbctl.c
@@ -780,10 +780,8 @@ static int run(int argc, char *argv[]) {
                         return log_error_errno(r, "Failed to set $SYSTEMD_ONLY_USERDB: %m");
 
                 log_info("Enabled services: %s", e);
-        } else {
-                if (unsetenv("SYSTEMD_ONLY_USERDB") < 0)
-                        return log_error_errno(r, "Failed to unset $SYSTEMD_ONLY_USERDB: %m");
-        }
+        } else
+                assert_se(unsetenv("SYSTEMD_ONLY_USERDB") == 0);
 
         return dispatch_verb(argc, argv, verbs, NULL);
 }