tree-wide: port to fd_wait_for_event()

Prompted by the discussion on #16110, let's migrate more code to
fd_wait_for_event().

This only leaves 7 places where we call into poll()/poll() directly in
our entire codebase. (one of which is fd_wait_for_event() itself)

TODO

@@ -19,6 +19,9 @@ Janitorial Clean-ups:
 
 Features:
 
+* add --copy-from and --copy-to command to systemd-dissect which copies stuff
+  in and out of a disk image
+
 * add systemd.random_seed= on the kernel cmdline, taking some hex or base64
   encoded data. During earliest boot, credit it to entropy. This is not useful
   for general purpose systems, but certainly for testing environments in VMs

docs/DESKTOP_ENVIRONMENTS.md

@@ -50,13 +50,22 @@ rather than the root slice?
 To ensure cross-desktop compatibility and encourage sharing of good practices,
 desktop environments should adhere to the following conventions:
 
- * Application units should follow the scheme `app-<launcher>-<ApplicationID>-<RANDOM>.service`,
-   e.g. `app-gnome-org.gnome.Evince-12345.service`,
-   `app-flatpak-org.telegram.desktop-12345.service` or `app-KDE-org.kde.okular-12345.service`.
+ * Application units should follow the scheme `app[-<launcher>]-<ApplicationID>[@<RANDOM>].service`
+ or `app[-<launcher>]-<ApplicationID>-<RANDOM>.scope`
+   e.g:
+    - `app-gnome-org.gnome.Evince@12345.service`
+    - `app-flatpak-org.telegram.desktop@12345.service`
+    - `app-KDE-org.kde.okular@12345.service`
+    - `app-org.kde.amarok.service`
+    - `app-org.gnome.Evince-12345.scope`
  * Using `.service` units instead of `.scope` units, i.e. allowing systemd to
    start the process on behalf of the caller,
    instead of the caller starting the process and letting systemd know about it,
    is encouraged.
+ * The RANDOM should be a string of random characters to ensure that multiple instances
+ of the application can be launched.
+ It can be ommitted in the case of a non-transient application services which can ensure
+ multiple instances are not spawned, such as a DBus activated application.
  * If no application ID is available, the launcher should generate a reasonable
    name when possible (e.g. using `basename(argv[0])`). This name must not
    contain a `-` character.

src/analyze/analyze-verify.c

@@ -94,6 +94,7 @@ static int generate_path(char **var, char **filenames) {
 }
 
 static int verify_socket(Unit *u) {
+        Unit *service;
         int r;
 
         assert(u);
@@ -101,26 +102,15 @@ static int verify_socket(Unit *u) {
         if (u->type != UNIT_SOCKET)
                 return 0;
 
-        /* Cannot run this without the service being around */
-
-        /* This makes sure instance is created if necessary. */
-        r = socket_instantiate_service(SOCKET(u));
+        r = socket_load_service_unit(SOCKET(u), -1, &service);
         if (r < 0)
-                return log_unit_error_errno(u, r, "Socket cannot be started, failed to create instance: %m");
+                return log_unit_error_errno(u, r, "service unit for the socket cannot be loaded: %m");
 
-        /* This checks both type of sockets */
-        if (UNIT_ISSET(SOCKET(u)->service)) {
-                Service *service;
-
-                service = SERVICE(UNIT_DEREF(SOCKET(u)->service));
-                log_unit_debug(u, "Using %s", UNIT(service)->id);
-
-                if (UNIT(service)->load_state != UNIT_LOADED) {
-                        log_unit_error(u, "Service %s not loaded, %s cannot be started.", UNIT(service)->id, u->id);
-                        return -ENOENT;
-                }
-        }
+        if (service->load_state != UNIT_LOADED)
+                return log_unit_error_errno(u, SYNTHETIC_ERRNO(ENOENT),
+                                            "service %s not loaded, socket cannot be started.", service->id);
 
+        log_unit_debug(u, "using service unit %s.", service->id);
         return 0;
 }
 

src/basic/hashmap.c

@@ -833,7 +833,7 @@ static int hashmap_base_ensure_allocated(HashmapBase **h, const struct hash_ops
                 return -ENOMEM;
 
         *h = q;
-        return 0;
+        return 1;
 }
 
 int _hashmap_ensure_allocated(Hashmap **h, const struct hash_ops *hash_ops  HASHMAP_DEBUG_PARAMS) {

src/basic/io-util.c

@@ -11,10 +11,6 @@
 #include "time-util.h"
 
 int flush_fd(int fd) {
-        struct pollfd pollfd = {
-                .fd = fd,
-                .events = POLLIN,
-        };
         int count = 0;
 
         /* Read from the specified file descriptor, until POLLIN is not set anymore, throwing away everything
@@ -27,22 +23,18 @@ int flush_fd(int fd) {
                 ssize_t l;
                 int r;
 
-                r = poll(&pollfd, 1, 0);
+                r = fd_wait_for_event(fd, POLLIN, 0);
                 if (r < 0) {
-                        if (errno == EINTR)
+                        if (r == -EINTR)
                                 continue;
 
-                        return -errno;
+                        return r;
                 }
                 if (r == 0)
                         return count;
 
-                if (pollfd.revents & POLLNVAL)
-                        return -EBADF;
-
                 l = read(fd, buf, sizeof(buf));
                 if (l < 0) {
-
                         if (errno == EINTR)
                                 continue;
 
@@ -158,24 +150,15 @@ int loop_write(int fd, const void *buf, size_t nbytes, bool do_poll) {
 }
 
 int pipe_eof(int fd) {
-        struct pollfd pollfd = {
-                .fd = fd,
-                .events = POLLIN|POLLHUP,
-        };
-
         int r;
 
-        r = poll(&pollfd, 1, 0);
+        r = fd_wait_for_event(fd, POLLIN, 0);
         if (r < 0)
-                return -errno;
-
+                return r;
         if (r == 0)
                 return 0;
 
-        if (pollfd.revents & POLLNVAL)
-                return -EBADF;
-
-        return pollfd.revents & POLLHUP;
+        return !!(r & POLLHUP);
 }
 
 int fd_wait_for_event(int fd, int event, usec_t t) {

src/basic/socket-util.c

@@ -21,6 +21,7 @@
 #include "fd-util.h"
 #include "fileio.h"
 #include "format-util.h"
+#include "io-util.h"
 #include "log.h"
 #include "macro.h"
 #include "memory-util.h"
@@ -968,10 +969,6 @@ fallback:
 
 int flush_accept(int fd) {
 
-        struct pollfd pollfd = {
-                .fd = fd,
-                .events = POLLIN,
-        };
         int r, b;
         socklen_t l = sizeof(b);
 
@@ -992,19 +989,16 @@ int flush_accept(int fd) {
         for (unsigned iteration = 0;; iteration++) {
                 int cfd;
 
-                r = poll(&pollfd, 1, 0);
+                r = fd_wait_for_event(fd, POLLIN, 0);
                 if (r < 0) {
-                        if (errno == EINTR)
+                        if (r == -EINTR)
                                 continue;
 
-                        return -errno;
+                        return r;
                 }
                 if (r == 0)
                         return 0;
 
-                if (pollfd.revents & POLLNVAL)
-                        return -EBADF;
-
                 if (iteration >= MAX_FLUSH_ITERATIONS)
                         return log_debug_errno(SYNTHETIC_ERRNO(EBUSY),
                                                "Failed to flush connections within " STRINGIFY(MAX_FLUSH_ITERATIONS) " iterations.");

src/core/dbus-unit.c

@@ -102,20 +102,24 @@ static int property_get_names(
                 void *userdata,
                 sd_bus_error *error) {
 
-        Set **s = userdata;
+        Unit *u = userdata;
         Iterator i;
         const char *t;
         int r;
 
         assert(bus);
         assert(reply);
-        assert(s);
+        assert(u);
 
         r = sd_bus_message_open_container(reply, 'a', "s");
         if (r < 0)
                 return r;
 
-        SET_FOREACH(t, *s, i) {
+        r = sd_bus_message_append(reply, "s", u->id);
+        if (r < 0)
+                return r;
+
+        SET_FOREACH(t, u->aliases, i) {
                 r = sd_bus_message_append(reply, "s", t);
                 if (r < 0)
                         return r;
@@ -841,7 +845,7 @@ const sd_bus_vtable bus_unit_vtable[] = {
         SD_BUS_VTABLE_START(0),
 
         SD_BUS_PROPERTY("Id", "s", NULL, offsetof(Unit, id), SD_BUS_VTABLE_PROPERTY_CONST),
-        SD_BUS_PROPERTY("Names", "as", property_get_names, offsetof(Unit, names), SD_BUS_VTABLE_PROPERTY_CONST),
+        SD_BUS_PROPERTY("Names", "as", property_get_names, 0, SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Following", "s", property_get_following, 0, 0),
         SD_BUS_PROPERTY("Requires", "as", property_get_dependencies, offsetof(Unit, dependencies[UNIT_REQUIRES]), SD_BUS_VTABLE_PROPERTY_CONST),
         SD_BUS_PROPERTY("Requisite", "as", property_get_dependencies, offsetof(Unit, dependencies[UNIT_REQUISITE]), SD_BUS_VTABLE_PROPERTY_CONST),

src/core/load-dropin.c

@@ -19,9 +19,8 @@ static int process_deps(Unit *u, UnitDependency dependency, const char *dir_suff
         r = unit_file_find_dropin_paths(NULL,
                                         u->manager->lookup_paths.search_path,
                                         u->manager->unit_path_cache,
-                                        dir_suffix,
-                                        NULL,
-                                        u->names,
+                                        dir_suffix, NULL,
+                                        u->id, u->aliases,
                                         &paths);
         if (r < 0)
                 return r;

src/core/load-dropin.h

@@ -13,7 +13,7 @@ static inline int unit_find_dropin_paths(Unit *u, char ***paths) {
                                            u->manager->lookup_paths.search_path,
                                            u->manager->unit_path_cache,
                                            ".d", ".conf",
-                                           u->names,
+                                           u->id, u->aliases,
                                            paths);
 }
 

src/core/service.c

@@ -1703,10 +1703,10 @@ static bool service_shall_restart(Service *s, const char **reason) {
                 return s->result == SERVICE_SUCCESS;
 
         case SERVICE_RESTART_ON_FAILURE:
-                return s->result != SERVICE_SUCCESS;
+                return !IN_SET(s->result, SERVICE_SUCCESS, SERVICE_SKIP_CONDITION);
 
         case SERVICE_RESTART_ON_ABNORMAL:
-                return !IN_SET(s->result, SERVICE_SUCCESS, SERVICE_FAILURE_EXIT_CODE);
+                return !IN_SET(s->result, SERVICE_SUCCESS, SERVICE_FAILURE_EXIT_CODE, SERVICE_SKIP_CONDITION);
 
         case SERVICE_RESTART_ON_WATCHDOG:
                 return s->result == SERVICE_FAILURE_WATCHDOG;

src/core/socket.c

@@ -205,38 +205,25 @@ static int socket_arm_timer(Socket *s, usec_t usec) {
         return 0;
 }
 
-int socket_instantiate_service(Socket *s) {
-        _cleanup_free_ char *prefix = NULL, *name = NULL;
+static int socket_instantiate_service(Socket *s, int cfd) {
+        Unit *service;
         int r;
-        Unit *u;
 
         assert(s);
+        assert(cfd >= 0);
 
-        /* This fills in s->service if it isn't filled in yet. For
-         * Accept=yes sockets we create the next connection service
-         * here. For Accept=no this is mostly a NOP since the service
-         * is figured out at load time anyway. */
+        /* This fills in s->service if it isn't filled in yet. For Accept=yes sockets we create the next
+         * connection service here. For Accept=no this is mostly a NOP since the service is figured out at
+         * load time anyway. */
 
-        if (UNIT_DEREF(s->service))
-                return 0;
-
-        if (!s->accept)
-                return 0;
-
-        r = unit_name_to_prefix(UNIT(s)->id, &prefix);
+        r = socket_load_service_unit(s, cfd, &service);
         if (r < 0)
                 return r;
 
-        if (asprintf(&name, "%s@%u.service", prefix, s->n_accepted) < 0)
-                return -ENOMEM;
+        unit_ref_set(&s->service, UNIT(s), service);
 
-        r = manager_load_unit(UNIT(s)->manager, name, NULL, NULL, &u);
-        if (r < 0)
-                return r;
-
-        unit_ref_set(&s->service, UNIT(s), u);
-
-        return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, u, false, UNIT_DEPENDENCY_IMPLICIT);
+        return unit_add_two_dependencies(UNIT(s), UNIT_BEFORE, UNIT_TRIGGERS, service,
+                                         false, UNIT_DEPENDENCY_IMPLICIT);
 }
 
 static bool have_non_accept_socket(Socket *s) {
@@ -1406,37 +1393,81 @@ clear:
         return r;
 }
 
+int socket_load_service_unit(Socket *s, int cfd, Unit **ret) {
+        /* Figure out what the unit that will be used to handle the connections on the socket looks like.
+         *
+         * If cfd < 0, then we don't have a connection yet. In case of Accept=yes sockets, use a fake
+         * instance name.
+         */
+
+        if (UNIT_ISSET(s->service)) {
+                *ret = UNIT_DEREF(s->service);
+                return 0;
+        }
+
+        if (!s->accept)
+                return -ENODATA;
+
+        /* Build the instance name and load the unit */
+        _cleanup_free_ char *prefix = NULL, *instance = NULL, *name = NULL;
+        int r;
+
+        r = unit_name_to_prefix(UNIT(s)->id, &prefix);
+        if (r < 0)
+                return r;
+
+        if (cfd >= 0) {
+                r = instance_from_socket(cfd, s->n_accepted, &instance);
+                if (r == -ENOTCONN)
+                        /* ENOTCONN is legitimate if TCP RST was received.
+                         * This connection is over, but the socket unit lives on. */
+                        return log_unit_debug_errno(UNIT(s), r,
+                                                    "Got ENOTCONN on incoming socket, assuming aborted connection attempt, ignoring.");
+                if (r < 0)
+                        return r;
+        }
+
+        /* For accepting sockets, we don't know how the instance will be called until we get a connection and
+         * can figure out what the peer name is. So let's use "internal" as the instance to make it clear
+         * that this is not an actual peer name. We use "unknown" when we cannot figure out the peer. */
+        r = unit_name_build(prefix, instance ?: "internal", ".service", &name);
+        if (r < 0)
+                return r;
+
+        return manager_load_unit(UNIT(s)->manager, name, NULL, NULL, ret);
+}
+
 static int socket_determine_selinux_label(Socket *s, char **ret) {
-        Service *service;
-        ExecCommand *c;
-        _cleanup_free_ char *path = NULL;
         int r;
 
         assert(s);
         assert(ret);
 
         if (s->selinux_context_from_net) {
-                /* If this is requested, get label from the network label */
+                /* If this is requested, get the label from the network label */
 
                 r = mac_selinux_get_our_label(ret);
                 if (r == -EOPNOTSUPP)
                         goto no_label;
 
         } else {
-                /* Otherwise, get it from the executable we are about to start */
-                r = socket_instantiate_service(s);
+                /* Otherwise, get it from the executable we are about to start. */
+
+                Unit *service;
+                ExecCommand *c;
+                _cleanup_free_ char *path = NULL;
+
+                r = socket_load_service_unit(s, -1, &service);
+                if (r == -ENODATA)
+                        goto no_label;
                 if (r < 0)
                         return r;
 
-                if (!UNIT_ISSET(s->service))
-                        goto no_label;
-
-                service = SERVICE(UNIT_DEREF(s->service));
-                c = service->exec_command[SERVICE_EXEC_START];
+                c = SERVICE(service)->exec_command[SERVICE_EXEC_START];
                 if (!c)
                         goto no_label;
 
-                r = chase_symlinks(c->path, service->exec_context.root_directory, CHASE_PREFIX_ROOT, &path, NULL);
+                r = chase_symlinks(c->path, SERVICE(service)->exec_context.root_directory, CHASE_PREFIX_ROOT, &path, NULL);
                 if (r < 0)
                         goto no_label;
 
@@ -1622,8 +1653,8 @@ static int socket_open_fds(Socket *_s) {
                 case SOCKET_SOCKET:
 
                         if (!know_label) {
-                                /* Figure out label, if we don't it know yet. We do it once, for the first socket where
-                                 * we need this and remember it for the rest. */
+                                /* Figure out the label, if we don't it know yet. We do it once for the first
+                                 * socket where we need this and remember it for the rest. */
 
                                 r = socket_determine_selinux_label(s, &label);
                                 if (r < 0)
@@ -2340,7 +2371,6 @@ static void socket_enter_running(Socket *s, int cfd) {
 
                 socket_set_state(s, SOCKET_RUNNING);
         } else {
-                _cleanup_free_ char *prefix = NULL, *instance = NULL, *name = NULL;
                 _cleanup_(socket_peer_unrefp) SocketPeer *p = NULL;
                 Service *service;
 
@@ -2352,9 +2382,9 @@ static void socket_enter_running(Socket *s, int cfd) {
 
                 if (s->max_connections_per_source > 0) {
                         r = socket_acquire_peer(s, cfd, &p);
-                        if (r < 0) {
+                        if (r < 0)
                                 goto refuse;
-                        } else if (r > 0 && p->n_ref > s->max_connections_per_source) {
+                        if (r > 0 && p->n_ref > s->max_connections_per_source) {
                                 _cleanup_free_ char *t = NULL;
 
                                 (void) sockaddr_pretty(&p->peer.sa, p->peer_salen, true, false, &t);
@@ -2366,30 +2396,7 @@ static void socket_enter_running(Socket *s, int cfd) {
                         }
                 }
 
-                r = socket_instantiate_service(s);
-                if (r < 0)
-                        goto fail;
-
-                r = instance_from_socket(cfd, s->n_accepted, &instance);
-                if (r < 0) {
-                        if (r != -ENOTCONN)
-                                goto fail;
-
-                        /* ENOTCONN is legitimate if TCP RST was received.
-                         * This connection is over, but the socket unit lives on. */
-                        log_unit_debug(UNIT(s), "Got ENOTCONN on incoming socket, assuming aborted connection attempt, ignoring.");
-                        goto refuse;
-                }
-
-                r = unit_name_to_prefix(UNIT(s)->id, &prefix);
-                if (r < 0)
-                        goto fail;
-
-                r = unit_name_build(prefix, instance, ".service", &name);
-                if (r < 0)
-                        goto fail;
-
-                r = unit_add_name(UNIT_DEREF(s->service), name);
+                r = socket_instantiate_service(s, cfd);
                 if (r < 0)
                         goto fail;
 
@@ -2397,21 +2404,20 @@ static void socket_enter_running(Socket *s, int cfd) {
                 unit_ref_unset(&s->service);
 
                 s->n_accepted++;
-                unit_choose_id(UNIT(service), name);
 
                 r = service_set_socket_fd(service, cfd, s, s->selinux_context_from_net);
                 if (r < 0)
                         goto fail;
 
-                cfd = -1; /* We passed ownership of the fd to the service now. Forget it here. */
+                TAKE_FD(cfd); /* We passed ownership of the fd to the service now. Forget it here. */
                 s->n_connections++;
 
                 service->peer = TAKE_PTR(p); /* Pass ownership of the peer reference */
 
                 r = manager_add_job(UNIT(s)->manager, JOB_START, UNIT(service), JOB_REPLACE, NULL, &error, NULL);
                 if (r < 0) {
-                        /* We failed to activate the new service, but it still exists. Let's make sure the service
-                         * closes and forgets the connection fd again, immediately. */
+                        /* We failed to activate the new service, but it still exists. Let's make sure the
+                         * service closes and forgets the connection fd again, immediately. */
                         service_close_socket_fd(service);
                         goto fail;
                 }

src/core/socket.h

@@ -166,7 +166,7 @@ void socket_connection_unref(Socket *s);
 
 void socket_free_ports(Socket *s);
 
-int socket_instantiate_service(Socket *s);
+int socket_load_service_unit(Socket *s, int cfd, Unit **ret);
 
 char *socket_fdname(Socket *s);
 

src/core/unit.c

@@ -93,10 +93,6 @@ Unit *unit_new(Manager *m, size_t size) {
         if (!u)
                 return NULL;
 
-        u->names = set_new(&string_hash_ops);
-        if (!u->names)
-                return mfree(u);
-
         u->manager = m;
         u->type = _UNIT_TYPE_INVALID;
         u->default_dependencies = true;
@@ -152,7 +148,8 @@ bool unit_has_name(const Unit *u, const char *name) {
         assert(u);
         assert(name);
 
-        return set_contains(u->names, (char*) name);
+        return streq_ptr(name, u->id) ||
+               set_contains(u->aliases, name);
 }
 
 static void unit_init(Unit *u) {
@@ -207,8 +204,25 @@ static void unit_init(Unit *u) {
                 UNIT_VTABLE(u)->init(u);
 }
 
+static int unit_add_alias(Unit *u, char *donated_name) {
+        int r;
+
+        /* Make sure that u->names is allocated. We may leave u->names
+         * empty if we fail later, but this is not a problem. */
+        r = set_ensure_allocated(&u->aliases, &string_hash_ops);
+        if (r < 0)
+                return r;
+
+        r = set_put(u->aliases, donated_name);
+        if (r < 0)
+                return r;
+        assert(r > 0);
+
+        return 0;
+}
+
 int unit_add_name(Unit *u, const char *text) {
-        _cleanup_free_ char *s = NULL, *i = NULL;
+        _cleanup_free_ char *name = NULL, *instance = NULL;
         UnitType t;
         int r;
 
@@ -216,99 +230,101 @@ int unit_add_name(Unit *u, const char *text) {
         assert(text);
 
         if (unit_name_is_valid(text, UNIT_NAME_TEMPLATE)) {
-
                 if (!u->instance)
                         return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL),
                                                     "instance is not set when adding name '%s': %m", text);
 
-                r = unit_name_replace_instance(text, u->instance, &s);
+                r = unit_name_replace_instance(text, u->instance, &name);
                 if (r < 0)
                         return log_unit_debug_errno(u, r,
                                                     "failed to build instance name from '%s': %m", text);
         } else {
-                s = strdup(text);
-                if (!s)
+                name = strdup(text);
+                if (!name)
                         return -ENOMEM;
         }
 
-        if (set_contains(u->names, s))
+        if (unit_has_name(u, name))
                 return 0;
-        if (hashmap_contains(u->manager->units, s))
+
+        if (hashmap_contains(u->manager->units, name))
                 return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EEXIST),
-                                            "unit already exist when adding name '%s': %m", text);
+                                            "unit already exist when adding name '%s': %m", name);
 
-        if (!unit_name_is_valid(s, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE))
+        if (!unit_name_is_valid(name, UNIT_NAME_PLAIN|UNIT_NAME_INSTANCE))
                 return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL),
-                                            "name '%s' is invalid: %m", text);
+                                            "name '%s' is invalid: %m", name);
 
-        t = unit_name_to_type(s);
+        t = unit_name_to_type(name);
         if (t < 0)
                 return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL),
-                                            "failed to to derive unit type from name '%s': %m", text);
+                                            "failed to to derive unit type from name '%s': %m", name);
 
         if (u->type != _UNIT_TYPE_INVALID && t != u->type)
                 return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL),
                                             "unit type is illegal: u->type(%d) and t(%d) for name '%s': %m",
-                                            u->type, t, text);
+                                            u->type, t, name);
 
-        r = unit_name_to_instance(s, &i);
+        r = unit_name_to_instance(name, &instance);
         if (r < 0)
-                return log_unit_debug_errno(u, r, "failed to extract instance from name '%s': %m", text);
+                return log_unit_debug_errno(u, r, "failed to extract instance from name '%s': %m", name);
 
-        if (i && !unit_type_may_template(t))
-                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL), "templates are not allowed for name '%s': %m", text);
+        if (instance && !unit_type_may_template(t))
+                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL), "templates are not allowed for name '%s': %m", name);
 
-        /* Ensure that this unit is either instanced or not instanced,
-         * but not both. Note that we do allow names with different
-         * instance names however! */
-        if (u->type != _UNIT_TYPE_INVALID && !u->instance != !i)
+        /* Ensure that this unit either has no instance, or that the instance matches. */
+        if (u->type != _UNIT_TYPE_INVALID && !streq_ptr(u->instance, instance))
                 return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EINVAL),
-                                            "instance is illegal: u->type(%d), u->instance(%s) and i(%s) for name '%s': %m",
-                                            u->type, u->instance, i, text);
+                                            "cannot add name %s, the instances don't match (\"%s\" != \"%s\").",
+                                            name, instance, u->instance);
 
-        if (!unit_type_may_alias(t) && !set_isempty(u->names))
-                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EEXIST), "symlinks are not allowed for name '%s': %m", text);
+        if (u->id && !unit_type_may_alias(t))
+                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(EEXIST),
+                                            "cannot add name %s, aliases are not allowed for %s units.",
+                                            name, unit_type_to_string(t));
 
         if (hashmap_size(u->manager->units) >= MANAGER_MAX_NAMES)
-                return log_unit_debug_errno(u, SYNTHETIC_ERRNO(E2BIG), "too many units: %m");
+                return log_unit_warning_errno(u, SYNTHETIC_ERRNO(E2BIG), "cannot add name, manager has too many units: %m");
 
-        r = set_put(u->names, s);
+        /* Add name to the global hashmap first, because that's easier to undo */
+        r = hashmap_put(u->manager->units, name, u);
         if (r < 0)
-                return r;
-        assert(r > 0);
-
-        r = hashmap_put(u->manager->units, s, u);
-        if (r < 0) {
-                (void) set_remove(u->names, s);
                 return log_unit_debug_errno(u, r, "add unit to hashmap failed for name '%s': %m", text);
-        }
 
-        if (u->type == _UNIT_TYPE_INVALID) {
+        if (u->id) {
+                r = unit_add_alias(u, name); /* unit_add_alias() takes ownership of the name on success */
+                if (r < 0) {
+                        hashmap_remove(u->manager->units, name);
+                        return r;
+                }
+                TAKE_PTR(name);
+
+        } else {
+                /* A new name, we don't need the set yet. */
+                assert(u->type == _UNIT_TYPE_INVALID);
+                assert(!u->instance);
+
                 u->type = t;
-                u->id = s;
-                u->instance = TAKE_PTR(i);
+                u->id = TAKE_PTR(name);
+                u->instance = TAKE_PTR(instance);
 
                 LIST_PREPEND(units_by_type, u->manager->units_by_type[t], u);
-
                 unit_init(u);
         }
 
-        s = NULL;
-
         unit_add_to_dbus_queue(u);
         return 0;
 }
 
 int unit_choose_id(Unit *u, const char *name) {
         _cleanup_free_ char *t = NULL;
-        char *s, *i;
+        char *s;
         int r;
 
         assert(u);
         assert(name);
 
         if (unit_name_is_valid(name, UNIT_NAME_TEMPLATE)) {
-
                 if (!u->instance)
                         return -EINVAL;
 
@@ -319,21 +335,22 @@ int unit_choose_id(Unit *u, const char *name) {
                 name = t;
         }
 
-        /* Selects one of the names of this unit as the id */
-        s = set_get(u->names, (char*) name);
+        if (streq_ptr(u->id, name))
+                return 0; /* Nothing to do. */
+
+        /* Selects one of the aliases of this unit as the id */
+        s = set_get(u->aliases, (char*) name);
         if (!s)
                 return -ENOENT;
 
-        /* Determine the new instance from the new id */
-        r = unit_name_to_instance(s, &i);
+        if (u->id) {
+                r = set_remove_and_put(u->aliases, name, u->id);
                 if (r < 0)
                         return r;
+        } else
+                assert_se(set_remove(u->aliases, name)); /* see set_get() above… */
 
-        u->id = s;
-
-        free(u->instance);
-        u->instance = i;
-
+        u->id = s; /* Old u->id is now stored in the set, and s is not stored anywhere */
         unit_add_to_dbus_queue(u);
 
         return 0;
@@ -629,8 +646,10 @@ void unit_free(Unit *u) {
 
         unit_free_requires_mounts_for(u);
 
-        SET_FOREACH(t, u->names, i)
+        SET_FOREACH(t, u->aliases, i)
                 hashmap_remove_value(u->manager->units, t, u);
+        if (u->id)
+                hashmap_remove_value(u->manager->units, u->id, u);
 
         if (!sd_id128_is_null(u->invocation_id))
                 hashmap_remove_value(u->manager->units_by_invocation_id, &u->invocation_id, u);
@@ -727,11 +746,11 @@ void unit_free(Unit *u) {
         free(u->instance);
 
         free(u->job_timeout_reboot_arg);
-
-        set_free_free(u->names);
-
         free(u->reboot_arg);
 
+        set_free_free(u->aliases);
+        free(u->id);
+
         free(u);
 }
 
@@ -786,21 +805,6 @@ const char* unit_sub_state_to_string(Unit *u) {
         return UNIT_VTABLE(u)->sub_state_to_string(u);
 }
 
-static int set_complete_move(Set **s, Set **other) {
-        assert(s);
-        assert(other);
-
-        if (!other)
-                return 0;
-
-        if (*s)
-                return set_move(*s, *other);
-        else
-                *s = TAKE_PTR(*other);
-
-        return 0;
-}
-
 static int hashmap_complete_move(Hashmap **s, Hashmap **other) {
         assert(s);
         assert(other);
@@ -817,23 +821,28 @@ static int hashmap_complete_move(Hashmap **s, Hashmap **other) {
 }
 
 static int merge_names(Unit *u, Unit *other) {
-        char *t;
+        char *name;
         Iterator i;
         int r;
 
         assert(u);
         assert(other);
 
-        r = set_complete_move(&u->names, &other->names);
+        r = unit_add_alias(u, other->id);
         if (r < 0)
                 return r;
 
-        set_free_free(other->names);
-        other->names = NULL;
-        other->id = NULL;
+        r = set_move(u->aliases, other->aliases);
+        if (r < 0) {
+                set_remove(u->aliases, other->id);
+                return r;
+        }
 
-        SET_FOREACH(t, u->names, i)
-                assert_se(hashmap_replace(u->manager->units, t, u) == 0);
+        TAKE_PTR(other->id);
+        other->aliases = set_free_free(other->aliases);
+
+        SET_FOREACH(name, u->aliases, i)
+                assert_se(hashmap_replace(u->manager->units, name, u) == 0);
 
         return 0;
 }
@@ -935,15 +944,15 @@ int unit_merge(Unit *u, Unit *other) {
         if (u->type != other->type)
                 return -EINVAL;
 
-        if (!u->instance != !other->instance)
-                return -EINVAL;
-
         if (!unit_type_may_alias(u->type)) /* Merging only applies to unit names that support aliases */
                 return -EEXIST;
 
         if (!IN_SET(other->load_state, UNIT_STUB, UNIT_NOT_FOUND))
                 return -EEXIST;
 
+        if (!streq_ptr(u->instance, other->instance))
+                return -EINVAL;
+
         if (other->job)
                 return -EEXIST;
 
@@ -1231,8 +1240,7 @@ void unit_dump(Unit *u, FILE *f, const char *prefix) {
                 "%s-> Unit %s:\n",
                 prefix, u->id);
 
-        SET_FOREACH(t, u->names, i)
-                if (!streq(t, u->id))
+        SET_FOREACH(t, u->aliases, i)
                 fprintf(f, "%s\tAlias: %s\n", prefix, t);
 
         fprintf(f,

src/core/unit.h

@@ -117,10 +117,10 @@ typedef struct Unit {
         FreezerState freezer_state;
         sd_bus_message *pending_freezer_message;
 
-        char *id; /* One name is special because we use it for identification. Points to an entry in the names set */
+        char *id;   /* The one special name that we use for identification */
         char *instance;
 
-        Set *names;
+        Set *aliases; /* All the other names. */
 
         /* For each dependency type we maintain a Hashmap whose key is the Unit* object, and the value encodes why the
          * dependency exists, using the UnitDependencyInfo type */

src/libsystemd/sd-bus/bus-socket.c

@@ -1262,23 +1262,15 @@ int bus_socket_read_message(sd_bus *bus) {
 }
 
 int bus_socket_process_opening(sd_bus *b) {
-        int error = 0;
+        int error = 0, events, r;
         socklen_t slen = sizeof(error);
-        struct pollfd p = {
-                .fd = b->output_fd,
-                .events = POLLOUT,
-        };
-        int r;
 
         assert(b->state == BUS_OPENING);
 
-        r = poll(&p, 1, 0);
-        if (r < 0)
-                return -errno;
-        if (p.revents & POLLNVAL)
-                return -EBADF;
-
-        if (!(p.revents & (POLLOUT|POLLERR|POLLHUP)))
+        events = fd_wait_for_event(b->output_fd, POLLOUT, 0);
+        if (events < 0)
+                return events;
+        if (!(events & (POLLOUT|POLLERR|POLLHUP)))
                 return 0;
 
         r = getsockopt(b->output_fd, SOL_SOCKET, SO_ERROR, &error, &slen);
@@ -1286,7 +1278,7 @@ int bus_socket_process_opening(sd_bus *b) {
                 b->last_connect_error = errno;
         else if (error != 0)
                 b->last_connect_error = error;
-        else if (p.revents & (POLLERR|POLLHUP))
+        else if (events & (POLLERR|POLLHUP))
                 b->last_connect_error = ECONNREFUSED;
         else
                 return bus_socket_start_auth(b);

src/libsystemd/sd-daemon/sd-daemon.c

@@ -555,7 +555,6 @@ finish:
 
 _public_ int sd_notify_barrier(int unset_environment, uint64_t timeout) {
         _cleanup_close_pair_ int pipe_fd[2] = { -1, -1 };
-        struct timespec ts;
         int r;
 
         if (pipe2(pipe_fd, O_CLOEXEC) < 0)
@@ -567,18 +566,11 @@ _public_ int sd_notify_barrier(int unset_environment, uint64_t timeout) {
 
         pipe_fd[1] = safe_close(pipe_fd[1]);
 
-        struct pollfd pfd = {
-                .fd = pipe_fd[0],
-                /* POLLHUP is implicit */
-                .events = 0,
-        };
-        r = ppoll(&pfd, 1, timeout == UINT64_MAX ? NULL : timespec_store(&ts, timeout), NULL);
+        r = fd_wait_for_event(pipe_fd[0], 0 /* POLLHUP is implicit */, timeout);
         if (r < 0)
-                return -errno;
+                return r;
         if (r == 0)
                 return -ETIMEDOUT;
-        if (pfd.revents & POLLNVAL)
-                return -EBADF;
 
         return 1;
 }

src/libsystemd/sd-netlink/sd-netlink.c

@@ -7,6 +7,7 @@
 #include "alloc-util.h"
 #include "fd-util.h"
 #include "hashmap.h"
+#include "io-util.h"
 #include "macro.h"
 #include "netlink-internal.h"
 #include "netlink-slot.h"
@@ -462,7 +463,6 @@ static usec_t calc_elapse(uint64_t usec) {
 }
 
 static int rtnl_poll(sd_netlink *rtnl, bool need_more, uint64_t timeout_usec) {
-        struct timespec ts;
         usec_t m = USEC_INFINITY;
         int r, e;
 
@@ -491,23 +491,15 @@ static int rtnl_poll(sd_netlink *rtnl, bool need_more, uint64_t timeout_usec) {
                 }
         }
 
-        if (timeout_usec != (uint64_t) -1 && (m == (uint64_t) -1 || timeout_usec < m))
+        if (timeout_usec != (uint64_t) -1 && (m == USEC_INFINITY || timeout_usec < m))
                 m = timeout_usec;
 
-        struct pollfd p = {
-                .fd = rtnl->fd,
-                .events = e,
-        };
-
-        r = ppoll(&p, 1, m == (uint64_t) -1 ? NULL : timespec_store(&ts, m), NULL);
+        r = fd_wait_for_event(rtnl->fd, e, m);
         if (r < 0)
-                return -errno;
+                return r;
         if (r == 0)
                 return 0;
 
-        if (p.revents & POLLNVAL)
-                return -EBADF;
-
         return 1;
 }
 

src/libudev/libudev-monitor.c

@@ -9,6 +9,7 @@
 #include "device-monitor-private.h"
 #include "device-private.h"
 #include "device-util.h"
+#include "io-util.h"
 #include "libudev-device-internal.h"
 #include "string-util.h"
 
@@ -191,17 +192,11 @@ _public_ int udev_monitor_get_fd(struct udev_monitor *udev_monitor) {
 }
 
 static int udev_monitor_receive_sd_device(struct udev_monitor *udev_monitor, sd_device **ret) {
-        struct pollfd pfd;
         int r;
 
         assert(udev_monitor);
         assert(ret);
 
-        pfd = (struct pollfd) {
-                .fd = device_monitor_get_fd(udev_monitor->monitor),
-                .events = POLLIN,
-        };
-
         for (;;) {
                 /* r == 0 means a device is received but it does not pass the current filter. */
                 r = device_monitor_receive_device(udev_monitor->monitor, ret);
@@ -209,20 +204,18 @@ static int udev_monitor_receive_sd_device(struct udev_monitor *udev_monitor, sd_
                         return r;
 
                 for (;;) {
-                        /* wait next message */
-                        r = poll(&pfd, 1, 0);
+                        /* Wait for next message */
+                        r = fd_wait_for_event(device_monitor_get_fd(udev_monitor->monitor), POLLIN, 0);
                         if (r < 0) {
-                                if (IN_SET(errno, EINTR, EAGAIN))
+                                if (IN_SET(r, -EINTR, -EAGAIN))
                                         continue;
 
-                                return -errno;
+                                return r;
                         }
                         if (r == 0)
                                 return -EAGAIN;
-                        if (pfd.revents & POLLNVAL)
-                                return -EBADF;
 
-                        /* receive next message */
+                        /* Receive next message */
                         break;
                 }
         }

src/shared/dropin.c

@@ -226,30 +226,35 @@ int unit_file_find_dropin_paths(
                 Set *unit_path_cache,
                 const char *dir_suffix,
                 const char *file_suffix,
-                const Set *names,
+                const char *name,
+                const Set *aliases,
                 char ***ret) {
 
         _cleanup_strv_free_ char **dirs = NULL;
-        char *name, **p;
+        const char *n;
+        char **p;
         Iterator i;
         int r;
 
         assert(ret);
 
-        SET_FOREACH(name, names, i)
+        if (name)
                 STRV_FOREACH(p, lookup_path)
                         (void) unit_file_find_dirs(original_root, unit_path_cache, *p, name, dir_suffix, &dirs);
 
+        SET_FOREACH(n, aliases, i)
+                STRV_FOREACH(p, lookup_path)
+                        (void) unit_file_find_dirs(original_root, unit_path_cache, *p, n, dir_suffix, &dirs);
+
         /* All the names in the unit are of the same type so just grab one. */
-        name = (char*) set_first(names);
-        if (name) {
+        n = name ?: (const char*) set_first(aliases);
+        if (n) {
                 UnitType type = _UNIT_TYPE_INVALID;
 
-                type = unit_name_to_type(name);
+                type = unit_name_to_type(n);
                 if (type < 0)
                         return log_error_errno(SYNTHETIC_ERRNO(EINVAL),
-                                               "Failed to to derive unit type from unit name: %s",
-                                               name);
+                                               "Failed to to derive unit type from unit name: %s", n);
 
                 /* Special top level drop in for "<unit type>.<suffix>". Add this last as it's the most generic
                  * and should be able to be overridden by more specific drop-ins. */

src/shared/dropin.h

@@ -21,5 +21,6 @@ int unit_file_find_dropin_paths(
                 Set *unit_path_cache,
                 const char *dir_suffix,
                 const char *file_suffix,
-                const Set *names,
+                const char *name,
+                const Set *aliases,
                 char ***paths);

src/shared/utmp-wtmp.c

@@ -14,6 +14,7 @@
 #include "alloc-util.h"
 #include "fd-util.h"
 #include "hostname-util.h"
+#include "io-util.h"
 #include "macro.h"
 #include "memory-util.h"
 #include "path-util.h"
@@ -297,7 +298,7 @@ int utmp_put_runlevel(int runlevel, int previous) {
         return write_entry_both(&store);
 }
 
-#define TIMEOUT_MSEC 50
+#define TIMEOUT_USEC (50 * USEC_PER_MSEC)
 
 static int write_to_terminal(const char *tty, const char *message) {
         _cleanup_close_ int fd = -1;
@@ -315,14 +316,10 @@ static int write_to_terminal(const char *tty, const char *message) {
         p = message;
         left = strlen(message);
 
-        end = now(CLOCK_MONOTONIC) + TIMEOUT_MSEC*USEC_PER_MSEC;
+        end = now(CLOCK_MONOTONIC) + TIMEOUT_USEC;
 
         while (left > 0) {
                 ssize_t n;
-                struct pollfd pollfd = {
-                        .fd = fd,
-                        .events = POLLOUT,
-                };
                 usec_t t;
                 int k;
 
@@ -331,13 +328,11 @@ static int write_to_terminal(const char *tty, const char *message) {
                 if (t >= end)
                         return -ETIME;
 
-                k = poll(&pollfd, 1, (end - t) / USEC_PER_MSEC);
+                k = fd_wait_for_event(fd, POLLOUT, end - t);
                 if (k < 0)
-                        return -errno;
+                        return k;
                 if (k == 0)
                         return -ETIME;
-                if (pollfd.revents & POLLNVAL)
-                        return -EBADF;
 
                 n = write(fd, p, left);
                 if (n < 0) {

src/shared/varlink.c

@@ -6,6 +6,7 @@
 #include "errno-util.h"
 #include "fd-util.h"
 #include "hashmap.h"
+#include "io-util.h"
 #include "list.h"
 #include "process-util.h"
 #include "set.h"
@@ -1003,8 +1004,6 @@ static void handle_revents(Varlink *v, int revents) {
 }
 
 int varlink_wait(Varlink *v, usec_t timeout) {
-        struct timespec ts;
-        struct pollfd pfd;
         int r, fd, events;
         usec_t t;
 
@@ -1038,23 +1037,13 @@ int varlink_wait(Varlink *v, usec_t timeout) {
         if (events < 0)
                 return events;
 
-        pfd = (struct pollfd) {
-                .fd = fd,
-                .events = events,
-        };
-
-        r = ppoll(&pfd, 1,
-                  t == USEC_INFINITY ? NULL : timespec_store(&ts, t),
-                  NULL);
+        r = fd_wait_for_event(fd, events, t);
         if (r < 0)
-                return -errno;
+                return r;
         if (r == 0)
                 return 0;
 
-        if (pfd.revents & POLLNVAL)
-                return -EBADF;
-
-        handle_revents(v, pfd.revents);
+        handle_revents(v, r);
         return 1;
 }
 
@@ -1123,8 +1112,6 @@ int varlink_flush(Varlink *v) {
                 return -ENOTCONN;
 
         for (;;) {
-                struct pollfd pfd;
-
                 if (v->output_buffer_size == 0)
                         break;
                 if (v->write_disconnected)
@@ -1138,20 +1125,13 @@ int varlink_flush(Varlink *v) {
                         continue;
                 }
 
-                pfd = (struct pollfd) {
-                        .fd = v->fd,
-                        .events = POLLOUT,
-                };
-
-                r = poll(&pfd, 1, -1);
+                r = fd_wait_for_event(v->fd, POLLOUT, USEC_INFINITY);
                 if (r < 0)
-                        return -errno;
+                        return r;
 
-                assert(r > 0);
-                if (pfd.revents & POLLNVAL)
-                        return -EBADF;
+                assert(r != 0);
 
-                handle_revents(v, pfd.revents);
+                handle_revents(v, r);
         }
 
         return ret;

src/sleep/sleep.c

@@ -23,6 +23,7 @@
 #include "fd-util.h"
 #include "fileio.h"
 #include "format-util.h"
+#include "io-util.h"
 #include "log.h"
 #include "main-func.h"
 #include "parse-util.h"
@@ -239,7 +240,6 @@ static int execute_s2h(const SleepConfig *sleep_config) {
         _cleanup_close_ int tfd = -1;
         char buf[FORMAT_TIMESPAN_MAX];
         struct itimerspec ts = {};
-        struct pollfd fds;
         int r;
 
         assert(sleep_config);
@@ -261,34 +261,25 @@ static int execute_s2h(const SleepConfig *sleep_config) {
         if (r < 0)
                 return r;
 
-        fds = (struct pollfd) {
-                .fd = tfd,
-                .events = POLLIN,
-        };
-        r = poll(&fds, 1, 0);
+        r = fd_wait_for_event(tfd, POLLIN, 0);
         if (r < 0)
-                return log_error_errno(errno, "Error polling timerfd: %m");
+                return log_error_errno(r, "Error polling timerfd: %m");
+        if (!FLAGS_SET(r, POLLIN)) /* We woke up before the alarm time, we are done. */
+                return 0;
 
         tfd = safe_close(tfd);
 
-        if (FLAGS_SET(fds.revents, POLLNVAL))
-                return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Invalid timer fd to sleep on?");
-
-        if (!FLAGS_SET(fds.revents, POLLIN)) /* We woke up before the alarm time, we are done. */
-                return 0;
-
         /* If woken up after alarm time, hibernate */
         log_debug("Attempting to hibernate after waking from %s timer",
                   format_timespan(buf, sizeof(buf), sleep_config->hibernate_delay_sec, USEC_PER_SEC));
 
         r = execute(sleep_config->hibernate_modes, sleep_config->hibernate_states);
         if (r < 0) {
-                log_notice("Couldn't hibernate, will try to suspend again.");
+                log_notice_errno(r, "Couldn't hibernate, will try to suspend again.");
+
                 r = execute(sleep_config->suspend_modes, sleep_config->suspend_states);
-                if (r < 0) {
-                        log_notice("Could neither hibernate nor suspend again, giving up.");
-                        return r;
-                }
+                if (r < 0)
+                        return log_notice_errno(r, "Could neither hibernate nor suspend again, giving up.");
         }
 
         return 0;

src/systemctl/systemctl.c

@@ -2650,7 +2650,7 @@ static int unit_find_paths(
                 if (ret_dropin_paths) {
                         r = unit_file_find_dropin_paths(arg_root, lp->search_path, NULL,
                                                         ".d", ".conf",
-                                                        names, &dropins);
+                                                        NULL, names, &dropins);
                         if (r < 0)
                                 return r;
                 }

src/test/test-hashmap-plain.c

@@ -251,7 +251,7 @@ static void test_hashmap_put(void) {
 
         log_info("/* %s */", __func__);
 
-        assert_se(hashmap_ensure_allocated(&m, &string_hash_ops) >= 0);
+        assert_se(hashmap_ensure_allocated(&m, &string_hash_ops) == 1);
         assert_se(m);
 
         valid_hashmap_put = hashmap_put(m, "key 1", val1);
@@ -451,18 +451,20 @@ static void test_hashmap_remove_and_replace(void) {
 }
 
 static void test_hashmap_ensure_allocated(void) {
-        Hashmap *m;
-        int valid_hashmap;
+        _cleanup_hashmap_free_ Hashmap *m = NULL;
+        int r;
 
         log_info("/* %s */", __func__);
 
-        m = hashmap_new(&string_hash_ops);
+        r = hashmap_ensure_allocated(&m, &string_hash_ops);
+        assert_se(r == 1);
 
-        valid_hashmap = hashmap_ensure_allocated(&m, &string_hash_ops);
-        assert_se(valid_hashmap == 0);
+        r = hashmap_ensure_allocated(&m, &string_hash_ops);
+        assert_se(r == 0);
 
-        assert_se(m);
-        hashmap_free(m);
+        /* different hash ops shouldn't matter at this point */
+        r = hashmap_ensure_allocated(&m, &trivial_hash_ops);
+        assert_se(r == 0);
 }
 
 static void test_hashmap_foreach_key(void) {
@@ -557,8 +559,7 @@ static void test_hashmap_foreach(void) {
 }
 
 static void test_hashmap_merge(void) {
-        Hashmap *m;
-        Hashmap *n;
+        Hashmap *m, *n;
         char *val1, *val2, *val3, *val4, *r;
 
         log_info("/* %s */", __func__);
@@ -572,8 +573,8 @@ static void test_hashmap_merge(void) {
         val4 = strdup("my val4");
         assert_se(val4);
 
-        n = hashmap_new(&string_hash_ops);
         m = hashmap_new(&string_hash_ops);
+        n = hashmap_new(&string_hash_ops);
 
         hashmap_put(m, "Key 1", val1);
         hashmap_put(m, "Key 2", val2);
@@ -586,8 +587,8 @@ static void test_hashmap_merge(void) {
         r = hashmap_get(m, "Key 4");
         assert_se(r && streq(r, "my val4"));
 
-        assert_se(n);
         assert_se(m);
+        assert_se(n);
         hashmap_free(n);
         hashmap_free_free(m);
 }

src/udev/udevadm-settle.c

@@ -15,6 +15,7 @@
 #include "sd-bus.h"
 #include "sd-login.h"
 
+#include "io-util.h"
 #include "libudev-util.h"
 #include "string-util.h"
 #include "strv.h"
@@ -141,9 +142,8 @@ static int emit_deprecation_warning(void) {
 
 int settle_main(int argc, char *argv[], void *userdata) {
         _cleanup_(udev_queue_unrefp) struct udev_queue *queue = NULL;
-        struct pollfd pfd;
         usec_t deadline;
-        int r;
+        int r, fd;
 
         r = parse_argv(argc, argv);
         if (r <= 0)
@@ -177,17 +177,12 @@ int settle_main(int argc, char *argv[], void *userdata) {
         if (!queue)
                 return log_error_errno(errno, "Failed to get udev queue: %m");
 
-        r = udev_queue_get_fd(queue);
-        if (r < 0) {
-                log_debug_errno(r, "Queue is empty, nothing to watch.");
+        fd = udev_queue_get_fd(queue);
+        if (fd < 0) {
+                log_debug_errno(fd, "Queue is empty, nothing to watch: %m");
                 return 0;
         }
 
-        pfd = (struct pollfd) {
-                .events = POLLIN,
-                .fd = r,
-        };
-
         (void) emit_deprecation_warning();
 
         for (;;) {
@@ -202,12 +197,10 @@ int settle_main(int argc, char *argv[], void *userdata) {
                         return -ETIMEDOUT;
 
                 /* wake up when queue becomes empty */
-                r = poll(&pfd, 1, MSEC_PER_SEC);
+                r = fd_wait_for_event(fd, POLLIN, MSEC_PER_SEC);
                 if (r < 0)
-                        return -errno;
-                if (pfd.revents & POLLNVAL)
-                        return -EBADF;
-                if (r > 0 && pfd.revents & POLLIN) {
+                        return r;
+                if (r & POLLIN) {
                         r = udev_queue_flush(queue);
                         if (r < 0)
                                 return log_error_errno(r, "Failed to flush queue: %m");

src/userdb/userwork.c

@@ -9,6 +9,7 @@
 #include "fd-util.h"
 #include "group-record-nss.h"
 #include "group-record.h"
+#include "io-util.h"
 #include "main-func.h"
 #include "process-util.h"
 #include "strv.h"
@@ -747,20 +748,14 @@ static int run(int argc, char *argv[]) {
                         return log_error_errno(fd, "Failed to accept() from listening socket: %m");
 
                 if (now(CLOCK_MONOTONIC) <= usec_add(n, PRESSURE_SLEEP_TIME_USEC)) {
-                        struct pollfd pfd = {
-                                .fd = listen_fd,
-                                .events = POLLIN,
-                        };
-
                         /* We only slept a very short time? If so, let's see if there are more sockets
                          * pending, and if so, let's ask our parent for more workers */
 
-                        if (poll(&pfd, 1, 0) < 0)
-                                return log_error_errno(errno, "Failed to test for POLLIN on listening socket: %m");
-                        if (FLAGS_SET(pfd.revents, POLLNVAL))
-                                return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Listening socket dead?");
+                        r = fd_wait_for_event(listen_fd, POLLIN, 0);
+                        if (r < 0)
+                                return log_error_errno(r, "Failed to test for POLLIN on listening socket: %m");
 
-                        if (FLAGS_SET(pfd.revents, POLLIN)) {
+                        if (FLAGS_SET(r, POLLIN)) {
                                 pid_t parent;
 
                                 parent = getppid();

test/TEST-02-CRYPTSETUP/test.sh

@@ -52,6 +52,10 @@ EOF
         cat >>$initdir/etc/fstab <<EOF
 /dev/mapper/varcrypt    /var    ext4    defaults 0 1
 EOF
+
+        # Forward journal messages to the console, so we have something
+        # to investigate even if we fail to mount the encrypted /var
+        echo ForwardToConsole=yes >> $initdir/etc/systemd/journald.conf
     )
 }
 

test/TEST-51-ISSUE-16115/Makefile

@@ -0,0 +1 @@
+../TEST-01-BASIC/Makefile

test/TEST-51-ISSUE-16115/test.sh

@@ -0,0 +1,6 @@
+#!/usr/bin/env bash
+set -e
+TEST_DESCRIPTION="Test ExecCondition= does not restart on abnormal or failure"
+. $TEST_BASE_DIR/test-functions
+
+do_test "$@" 51

test/test-functions

@@ -80,6 +80,7 @@ BASICTOOLS=(
     mktemp
     modprobe
     mount
+    mountpoint
     mv
     nc
     nproc

test/units/testsuite-02.service

@@ -4,5 +4,5 @@ After=multi-user.target
 
 [Service]
 ExecStartPre=rm -f /failed /testok
-ExecStart=sh -x -c 'systemctl --state=failed --no-legend --no-pager >/failed ; echo OK > /testok'
+ExecStart=sh -x -e -c 'mountpoint /var; systemctl --state=failed --no-legend --no-pager >/failed; echo OK >/testok'
 Type=oneshot

test/units/testsuite-51-repro-1.service

@@ -0,0 +1,9 @@
+[Unit]
+Description=Issue 16115 Repro with on-abnormal
+
+[Service]
+Type=simple
+Restart=on-abnormal
+ExecCondition=/bin/false
+ExecStart=sleep 100
+RestartSec=1

test/units/testsuite-51-repro-2.service

@@ -0,0 +1,9 @@
+[Unit]
+Description=Issue 16115 Repro with on-failure
+
+[Service]
+Type=simple
+Restart=on-failure
+ExecCondition=/bin/false
+ExecStart=sleep 100
+RestartSec=1

test/units/testsuite-51.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=TEST-51-ISSUE-16115
+
+[Service]
+ExecStartPre=rm -f /failed /testok
+ExecStart=/usr/lib/systemd/tests/testdata/units/%N.sh
+Type=oneshot

test/units/testsuite-51.sh

@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+set -ex
+set -o pipefail
+
+systemctl start testsuite-51-repro-1
+systemctl start testsuite-51-repro-2
+sleep 5 # wait a bit in case there are restarts so we can count them below
+
+[[ "$(systemctl show testsuite-51-repro-1 -P NRestarts)" == "0" ]]
+[[ "$(systemctl show testsuite-51-repro-2 -P NRestarts)" == "0" ]]
+
+touch /testok