util: replace close_nointr_nofail() by a more useful safe_close()

safe_close() automatically becomes a NOP when a negative fd is passed,
and returns -1 unconditionally. This makes it easy to write lines like
this:

        fd = safe_close(fd);

Which will close an fd if it is open, and reset the fd variable
correctly.

By making use of this new scheme we can drop a > 200 lines of code that
was required to test for non-negative fds or to reset the closed fd
variable afterwards.

17 files changed, 88 insertions, 190 deletions

diff --git a/src/core/audit-fd.c b/src/core/audit-fd.c
index 5955bd846..4326d1736 100644
--- a/src/core/audit-fd.c
+++ b/src/core/audit-fd.c
@@ -55,7 +55,7 @@ int get_audit_fd(void) {
 void close_audit_fd(void) {
 
         if (initialized && audit_fd >= 0)
-                close_nointr_nofail(audit_fd);
+                safe_close(audit_fd);
 
         initialized = true;
         audit_fd = -ECONNRESET;
diff --git a/src/core/automount.c b/src/core/automount.c
index 889b20e25..77d80b278 100644
--- a/src/core/automount.c
+++ b/src/core/automount.c
@@ -90,9 +90,7 @@ static void unmount_autofs(Automount *a) {
         automount_send_ready(a, -EHOSTDOWN);
 
         a->pipe_event_source = sd_event_source_unref(a->pipe_event_source);
-
-        close_nointr_nofail(a->pipe_fd);
-        a->pipe_fd = -1;
+        a->pipe_fd = safe_close(a->pipe_fd);
 
         /* If we reload/reexecute things we keep the mount point
          * around */
@@ -310,8 +308,7 @@ static int open_dev_autofs(Manager *m) {
 
         init_autofs_dev_ioctl(&param);
         if (ioctl(m->dev_autofs_fd, AUTOFS_DEV_IOCTL_VERSION, &param) < 0) {
-                close_nointr_nofail(m->dev_autofs_fd);
-                m->dev_autofs_fd = -1;
+                m->dev_autofs_fd = safe_close(m->dev_autofs_fd);
                 return -errno;
         }
 
@@ -411,8 +408,9 @@ static int autofs_send_ready(int dev_autofs_fd, int ioctl_fd, uint32_t token, in
 }
 
 int automount_send_ready(Automount *a, int status) {
-        int ioctl_fd, r;
+        _cleanup_close_ int ioctl_fd = -1;
         unsigned token;
+        int r;
 
         assert(a);
         assert(status <= 0);
@@ -421,10 +419,8 @@ int automount_send_ready(Automount *a, int status) {
                 return 0;
 
         ioctl_fd = open_ioctl_fd(UNIT(a)->manager->dev_autofs_fd, a->where, a->dev_id);
-        if (ioctl_fd < 0) {
-                r = ioctl_fd;
-                goto fail;
-        }
+        if (ioctl_fd < 0)
+                return ioctl_fd;
 
         if (status)
                 log_debug_unit(UNIT(a)->id, "Sending failure: %s", strerror(-status));
@@ -450,18 +446,15 @@ int automount_send_ready(Automount *a, int status) {
                         r = k;
         }
 
-fail:
-        if (ioctl_fd >= 0)
-                close_nointr_nofail(ioctl_fd);
-
         return r;
 }
 
 static void automount_enter_waiting(Automount *a) {
+        _cleanup_close_ int ioctl_fd = -1;
         int p[2] = { -1, -1 };
         char name[32], options[128];
         bool mounted = false;
-        int r, ioctl_fd = -1, dev_autofs_fd;
+        int r, dev_autofs_fd;
         struct stat st;
 
         assert(a);
@@ -500,8 +493,7 @@ static void automount_enter_waiting(Automount *a) {
 
         mounted = true;
 
-        close_nointr_nofail(p[1]);
-        p[1] = -1;
+        p[1] = safe_close(p[1]);
 
         if (stat(a->where, &st) < 0) {
                 r = -errno;
@@ -528,9 +520,6 @@ static void automount_enter_waiting(Automount *a) {
          * the direct mount will not receive events from the
          * kernel. */
 
-        close_nointr_nofail(ioctl_fd);
-        ioctl_fd = -1;
-
         r = sd_event_add_io(UNIT(a)->manager->event, &a->pipe_event_source, p[0], EPOLLIN, automount_dispatch_io, a);
         if (r < 0)
                 goto fail;
@@ -543,10 +532,7 @@ static void automount_enter_waiting(Automount *a) {
         return;
 
 fail:
-        assert_se(close_pipe(p) == 0);
-
-        if (ioctl_fd >= 0)
-                close_nointr_nofail(ioctl_fd);
+        close_pipe(p);
 
         if (mounted)
                 repeat_unmount(a->where);
@@ -713,9 +699,7 @@ static int automount_deserialize_item(Unit *u, const char *key, const char *valu
                 if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
                         log_debug_unit(u->id, "Failed to parse pipe-fd value %s", value);
                 else {
-                        if (a->pipe_fd >= 0)
-                                close_nointr_nofail(a->pipe_fd);
-
+                        safe_close(a->pipe_fd);
                         a->pipe_fd = fdset_remove(fds, fd);
                 }
         } else
@@ -809,8 +793,7 @@ fail:
 static void automount_shutdown(Manager *m) {
         assert(m);
 
-        if (m->dev_autofs_fd >= 0)
-                close_nointr_nofail(m->dev_autofs_fd);
+        m->dev_autofs_fd = safe_close(m->dev_autofs_fd);
 }
 
 static void automount_reset_failed(Unit *u) {
diff --git a/src/core/busname.c b/src/core/busname.c
index bd7d02d73..9d7a79653 100644
--- a/src/core/busname.c
+++ b/src/core/busname.c
@@ -56,11 +56,7 @@ static void busname_done(Unit *u) {
         unit_ref_unset(&n->service);
 
         n->event_source = sd_event_source_unref(n->event_source);
-
-        if (n->starter_fd >= 0) {
-                close_nointr_nofail(n->starter_fd);
-                n->starter_fd = -1;
-        }
+        n->starter_fd = safe_close(n->starter_fd);
 }
 
 static int busname_add_default_default_dependencies(BusName *n) {
@@ -122,8 +118,6 @@ static int busname_add_extras(BusName *n) {
         return 0;
 }
 
-
-
 static int busname_verify(BusName *n) {
         char *e;
 
@@ -202,8 +196,7 @@ static void busname_close_fd(BusName *n) {
         if (n->starter_fd <= 0)
                 return;
 
-        close_nointr_nofail(n->starter_fd);
-        n->starter_fd = -1;
+        n->starter_fd = safe_close(n->starter_fd);
 }
 
 static int busname_watch_fd(BusName *n) {
@@ -454,8 +447,7 @@ static int busname_deserialize_item(Unit *u, const char *key, const char *value,
                 if (safe_atoi(value, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
                         log_debug_unit(u->id, "Failed to parse starter fd value %s", value);
                 else {
-                        if (n->starter_fd >= 0)
-                                close_nointr_nofail(n->starter_fd);
+                        safe_close(n->starter_fd);
                         n->starter_fd = fdset_remove(fds, fd);
                 }
         } else
diff --git a/src/core/cgroup.c b/src/core/cgroup.c
index 18de888ba..9cc8544fc 100644
--- a/src/core/cgroup.c
+++ b/src/core/cgroup.c
@@ -862,8 +862,7 @@ int manager_setup_cgroup(Manager *m) {
         }
 
         /* 5. And pin it, so that it cannot be unmounted */
-        if (m->pin_cgroupfs_fd >= 0)
-                close_nointr_nofail(m->pin_cgroupfs_fd);
+        safe_close(m->pin_cgroupfs_fd);
 
         m->pin_cgroupfs_fd = open(path, O_RDONLY|O_CLOEXEC|O_DIRECTORY|O_NOCTTY|O_NONBLOCK);
         if (r < 0) {
@@ -888,10 +887,7 @@ void manager_shutdown_cgroup(Manager *m, bool delete) {
         if (delete && m->cgroup_root)
                 cg_trim(SYSTEMD_CGROUP_CONTROLLER, m->cgroup_root, false);
 
-        if (m->pin_cgroupfs_fd >= 0) {
-                close_nointr_nofail(m->pin_cgroupfs_fd);
-                m->pin_cgroupfs_fd = -1;
-        }
+        m->pin_cgroupfs_fd = safe_close(m->pin_cgroupfs_fd);
 
         free(m->cgroup_root);
         m->cgroup_root = NULL;
diff --git a/src/core/dbus.c b/src/core/dbus.c
index 72f36bdc1..d482e6c53 100644
--- a/src/core/dbus.c
+++ b/src/core/dbus.c
@@ -1086,10 +1086,7 @@ void bus_done(Manager *m) {
         if (m->private_listen_event_source)
                 m->private_listen_event_source = sd_event_source_unref(m->private_listen_event_source);
 
-        if (m->private_listen_fd >= 0) {
-                close_nointr_nofail(m->private_listen_fd);
-                m->private_listen_fd = -1;
-        }
+        m->private_listen_fd = safe_close(m->private_listen_fd);
 }
 
 int bus_fdset_add_all(Manager *m, FDSet *fds) {
diff --git a/src/core/execute.c b/src/core/execute.c
index ca807dc8c..a46f25de3 100644
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -123,7 +123,7 @@ static int shift_fds(int fds[], unsigned n_fds) {
                         if ((nfd = fcntl(fds[i], F_DUPFD, i+3)) < 0)
                                 return -errno;
 
-                        close_nointr_nofail(fds[i]);
+                        safe_close(fds[i]);
                         fds[i] = nfd;
 
                         /* Hmm, the fd we wanted isn't free? Then
@@ -209,7 +209,7 @@ static int open_null_as(int flags, int nfd) {
 
         if (fd != nfd) {
                 r = dup2(fd, nfd) < 0 ? -errno : nfd;
-                close_nointr_nofail(fd);
+                safe_close(fd);
         } else
                 r = nfd;
 
@@ -234,12 +234,12 @@ static int connect_logger_as(const ExecContext *context, ExecOutput output, cons
 
         r = connect(fd, &sa.sa, offsetof(struct sockaddr_un, sun_path) + strlen(sa.un.sun_path));
         if (r < 0) {
-                close_nointr_nofail(fd);
+                safe_close(fd);
                 return -errno;
         }
 
         if (shutdown(fd, SHUT_RD) < 0) {
-                close_nointr_nofail(fd);
+                safe_close(fd);
                 return -errno;
         }
 
@@ -263,7 +263,7 @@ static int connect_logger_as(const ExecContext *context, ExecOutput output, cons
 
         if (fd != nfd) {
                 r = dup2(fd, nfd) < 0 ? -errno : nfd;
-                close_nointr_nofail(fd);
+                safe_close(fd);
         } else
                 r = nfd;
 
@@ -280,7 +280,7 @@ static int open_terminal_as(const char *path, mode_t mode, int nfd) {
 
         if (fd != nfd) {
                 r = dup2(fd, nfd) < 0 ? -errno : nfd;
-                close_nointr_nofail(fd);
+                safe_close(fd);
         } else
                 r = nfd;
 
@@ -340,7 +340,7 @@ static int setup_input(const ExecContext *context, int socket_fd, bool apply_tty
 
                 if (fd != STDIN_FILENO) {
                         r = dup2(fd, STDIN_FILENO) < 0 ? -errno : STDIN_FILENO;
-                        close_nointr_nofail(fd);
+                        safe_close(fd);
                 } else
                         r = STDIN_FILENO;
 
@@ -504,7 +504,7 @@ static int setup_confirm_stdio(int *_saved_stdin,
         }
 
         if (fd >= 2)
-                close_nointr_nofail(fd);
+                safe_close(fd);
 
         *_saved_stdin = saved_stdin;
         *_saved_stdout = saved_stdout;
@@ -512,20 +512,15 @@ static int setup_confirm_stdio(int *_saved_stdin,
         return 0;
 
 fail:
-        if (saved_stdout >= 0)
-                close_nointr_nofail(saved_stdout);
-
-        if (saved_stdin >= 0)
-                close_nointr_nofail(saved_stdin);
-
-        if (fd >= 0)
-                close_nointr_nofail(fd);
+        safe_close(saved_stdout);
+        safe_close(saved_stdin);
+        safe_close(fd);
 
         return r;
 }
 
 _printf_(1, 2) static int write_confirm_message(const char *format, ...) {
-        int fd;
+        _cleanup_close_ int fd = -1;
         va_list ap;
 
         assert(format);
@@ -538,8 +533,6 @@ _printf_(1, 2) static int write_confirm_message(const char *format, ...) {
         vdprintf(fd, format, ap);
         va_end(ap);
 
-        close_nointr_nofail(fd);
-
         return 0;
 }
 
@@ -561,11 +554,8 @@ static int restore_confirm_stdio(int *saved_stdin,
                 if (dup2(*saved_stdout, STDOUT_FILENO) < 0)
                         r = -errno;
 
-        if (*saved_stdin >= 0)
-                close_nointr_nofail(*saved_stdin);
-
-        if (*saved_stdout >= 0)
-                close_nointr_nofail(*saved_stdout);
+        safe_close(*saved_stdin);
+        safe_close(*saved_stdout);
 
         return r;
 }
@@ -1125,10 +1115,9 @@ finish:
 static void do_idle_pipe_dance(int idle_pipe[4]) {
         assert(idle_pipe);
 
-        if (idle_pipe[1] >= 0)
-                close_nointr_nofail(idle_pipe[1]);
-        if (idle_pipe[2] >= 0)
-                close_nointr_nofail(idle_pipe[2]);
+
+        safe_close(idle_pipe[1]);
+        safe_close(idle_pipe[2]);
 
         if (idle_pipe[0] >= 0) {
                 int r;
@@ -1143,12 +1132,11 @@ static void do_idle_pipe_dance(int idle_pipe[4]) {
                         fd_wait_for_event(idle_pipe[0], POLLHUP, IDLE_TIMEOUT2_USEC);
                 }
 
-                close_nointr_nofail(idle_pipe[0]);
+                safe_close(idle_pipe[0]);
 
         }
 
-        if (idle_pipe[3] >= 0)
-                close_nointr_nofail(idle_pipe[3]);
+        safe_close(idle_pipe[3]);
 }
 
 static int build_environment(
@@ -2730,9 +2718,7 @@ int exec_runtime_deserialize_item(ExecRuntime **rt, Unit *u, const char *key, co
                 if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd))
                         log_debug_unit(u->id, "Failed to parse netns socket value %s", value);
                 else {
-                        if ((*rt)->netns_storage_socket[0] >= 0)
-                                close_nointr_nofail((*rt)->netns_storage_socket[0]);
-
+                        safe_close((*rt)->netns_storage_socket[0]);
                         (*rt)->netns_storage_socket[0] = fdset_remove(fds, fd);
                 }
         } else if (streq(key, "netns-socket-1")) {
@@ -2745,9 +2731,7 @@ int exec_runtime_deserialize_item(ExecRuntime **rt, Unit *u, const char *key, co
                 if (safe_atoi(value, &fd) < 0 || !fdset_contains(fds, fd))
                         log_debug_unit(u->id, "Failed to parse netns socket value %s", value);
                 else {
-                        if ((*rt)->netns_storage_socket[1] >= 0)
-                                close_nointr_nofail((*rt)->netns_storage_socket[1]);
-
+                        safe_close((*rt)->netns_storage_socket[1]);
                         (*rt)->netns_storage_socket[1] = fdset_remove(fds, fd);
                 }
         } else
diff --git a/src/core/ima-setup.c b/src/core/ima-setup.c
index 7f8ec23d5..ed65096f0 100644
--- a/src/core/ima-setup.c
+++ b/src/core/ima-setup.c
@@ -47,7 +47,7 @@ int ima_setup(void) {
        struct stat st;
        ssize_t policy_size = 0, written = 0;
        char *policy;
-       int policyfd = -1, imafd = -1;
+       _cleanup_close_ int policyfd = -1, imafd = -1;
        int result = 0;
 
        if (stat(IMA_POLICY_PATH, &st) < 0)
@@ -98,10 +98,6 @@ int ima_setup(void) {
 out_mmap:
        munmap(policy, policy_size);
 out:
-       if (policyfd >= 0)
-                close_nointr_nofail(policyfd);
-       if (imafd >= 0)
-                close_nointr_nofail(imafd);
        if (result)
                 return result;
 #endif /* HAVE_IMA */
diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c
index 06e3031d6..1c7ac75dd 100644
--- a/src/core/load-fragment.c
+++ b/src/core/load-fragment.c
@@ -3085,7 +3085,7 @@ static int open_follow(char **filename, FILE **_f, Set *names, char **_final) {
         f = fdopen(fd, "re");
         if (!f) {
                 r = -errno;
-                close_nointr_nofail(fd);
+                safe_close(fd);
                 return r;
         }
 
diff --git a/src/core/machine-id-setup.c b/src/core/machine-id-setup.c
index 43720d628..d459afe90 100644
--- a/src/core/machine-id-setup.c
+++ b/src/core/machine-id-setup.c
@@ -78,7 +78,7 @@ static int generate(char id[34], const char *root) {
         fd = open(dbus_machine_id, O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
         if (fd >= 0) {
                 k = loop_read(fd, id, 33, false);
-                close_nointr_nofail(fd);
+                safe_close(fd);
 
                 if (k == 33 && id[32] == '\n') {
 
@@ -104,7 +104,7 @@ static int generate(char id[34], const char *root) {
                 fd = open("/sys/class/dmi/id/product_uuid", O_RDONLY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
                 if (fd >= 0) {
                         k = loop_read(fd, uuid, 36, false);
-                        close_nointr_nofail(fd);
+                        safe_close(fd);
 
                         if (k >= 36) {
                                 r = shorten_uuid(id, uuid);
@@ -216,8 +216,7 @@ int machine_id_setup(const char *root) {
                         return 0;
         }
 
-        close_nointr_nofail(fd);
-        fd = -1;
+        fd = safe_close(fd);
 
         /* Hmm, we couldn't write it? So let's write it to
          * /run/machine-id as a replacement */
diff --git a/src/core/main.c b/src/core/main.c
index cc876efa9..e0fbb6e14 100644
--- a/src/core/main.c
+++ b/src/core/main.c
@@ -240,7 +240,7 @@ static int console_setup(bool do_reset) {
         if (r < 0)
                 log_error("Failed to reset /dev/console: %s", strerror(-r));
 
-        close_nointr_nofail(tty_fd);
+        safe_close(tty_fd);
         return r;
 }
 
diff --git a/src/core/manager.c b/src/core/manager.c
index ce5888e84..632ce74ac 100644
--- a/src/core/manager.c
+++ b/src/core/manager.c
@@ -250,8 +250,7 @@ static int manager_setup_time_change(Manager *m) {
 
         if (timerfd_settime(m->time_change_fd, TFD_TIMER_ABSTIME|TFD_TIMER_CANCEL_ON_SET, &its, NULL) < 0) {
                 log_debug("Failed to set up TFD_TIMER_CANCEL_ON_SET, ignoring: %m");
-                close_nointr_nofail(m->time_change_fd);
-                m->time_change_fd = -1;
+                m->time_change_fd = safe_close(m->time_change_fd);
                 return 0;
         }
 
@@ -793,14 +792,10 @@ void manager_free(Manager *m) {
         sd_event_source_unref(m->idle_pipe_event_source);
         sd_event_source_unref(m->run_queue_event_source);
 
-        if (m->signal_fd >= 0)
-                close_nointr_nofail(m->signal_fd);
-        if (m->notify_fd >= 0)
-                close_nointr_nofail(m->notify_fd);
-        if (m->time_change_fd >= 0)
-                close_nointr_nofail(m->time_change_fd);
-        if (m->kdbus_fd >= 0)
-                close_nointr_nofail(m->kdbus_fd);
+        safe_close(m->signal_fd);
+        safe_close(m->notify_fd);
+        safe_close(m->time_change_fd);
+        safe_close(m->kdbus_fd);
 
         manager_close_idle_pipe(m);
 
@@ -1756,9 +1751,7 @@ static int manager_dispatch_time_change_fd(sd_event_source *source, int fd, uint
 
         /* Restart the watch */
         m->time_change_event_source = sd_event_source_unref(m->time_change_event_source);
-
-        close_nointr_nofail(m->time_change_fd);
-        m->time_change_fd = -1;
+        m->time_change_fd = safe_close(m->time_change_fd);
 
         manager_setup_time_change(m);
 
@@ -2042,7 +2035,7 @@ int manager_open_serialization(Manager *m, FILE **_f) {
 
         f = fdopen(fd, "w+");
         if (!f) {
-                close_nointr_nofail(fd);
+                safe_close(fd);
                 return -errno;
         }
 
@@ -2263,11 +2256,8 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         if (safe_atoi(l + 10, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
                                 log_debug("Failed to parse notify fd: %s", l + 10);
                         else {
-                                if (m->notify_fd >= 0) {
-                                        m->notify_event_source = sd_event_source_unref(m->notify_event_source);
-                                        close_nointr_nofail(m->notify_fd);
-                                }
-
+                                m->notify_event_source = sd_event_source_unref(m->notify_event_source);
+                                safe_close(m->notify_fd);
                                 m->notify_fd = fdset_remove(fds, fd);
                         }
 
@@ -2289,9 +2279,7 @@ int manager_deserialize(Manager *m, FILE *f, FDSet *fds) {
                         if (safe_atoi(l + 9, &fd) < 0 || fd < 0 || !fdset_contains(fds, fd))
                                 log_debug("Failed to parse kdbus fd: %s", l + 9);
                         else {
-                                if (m->kdbus_fd >= 0)
-                                        close_nointr_nofail(m->kdbus_fd);
-
+                                safe_close(m->kdbus_fd);
                                 m->kdbus_fd = fdset_remove(fds, fd);
                         }
 
diff --git a/src/core/path.c b/src/core/path.c
index 93c51bb3b..20e454d96 100644
--- a/src/core/path.c
+++ b/src/core/path.c
@@ -152,11 +152,7 @@ void path_spec_unwatch(PathSpec *s) {
         assert(s);
 
         s->event_source = sd_event_source_unref(s->event_source);
-
-        if (s->inotify_fd >= 0) {
-                close_nointr_nofail(s->inotify_fd);
-                s->inotify_fd = -1;
-        }
+        s->inotify_fd = safe_close(s->inotify_fd);
 }
 
 int path_spec_fd_event(PathSpec *s, uint32_t revents) {
diff --git a/src/core/service.c b/src/core/service.c
index 386692a10..fe7ddd184 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -226,8 +226,7 @@ static void service_close_socket_fd(Service *s) {
         if (s->socket_fd < 0)
                 return;
 
-        close_nointr_nofail(s->socket_fd);
-        s->socket_fd = -1;
+        s->socket_fd = safe_close(s->socket_fd);
 }
 
 static void service_connection_unref(Service *s) {
@@ -2684,8 +2683,7 @@ static int service_deserialize_item(Unit *u, const char *key, const char *value,
                         log_debug_unit(u->id, "Failed to parse socket-fd value %s", value);
                 else {
 
-                        if (s->socket_fd >= 0)
-                                close_nointr_nofail(s->socket_fd);
+                        safe_close(s->socket_fd);
                         s->socket_fd = fdset_remove(fds, fd);
                 }
         } else if (streq(key, "main-exec-status-pid")) {
diff --git a/src/core/smack-setup.c b/src/core/smack-setup.c
index a68605c27..5f6dabf82 100644
--- a/src/core/smack-setup.c
+++ b/src/core/smack-setup.c
@@ -87,7 +87,7 @@ static int write_rules(const char* dstpath, const char* srcdir) {
                 if (!policy) {
                         if (r == 0)
                                 r = -errno;
-                        close_nointr_nofail(fd);
+                        safe_close(fd);
                         log_error("Failed to open %s: %m", entry->d_name);
                         continue;
                 }
diff --git a/src/core/socket.c b/src/core/socket.c
index b39bec2f2..b2a3e954c 100644
--- a/src/core/socket.c
+++ b/src/core/socket.c
@@ -121,9 +121,7 @@ void socket_free_ports(Socket *s) {
 
                 sd_event_source_unref(p->event_source);
 
-                if (p->fd >= 0)
-                        close_nointr_nofail(p->fd);
-
+                safe_close(p->fd);
                 free(p->path);
                 free(p);
         }
@@ -700,7 +698,7 @@ static void socket_close_fds(Socket *s) {
                 if (p->fd < 0)
                         continue;
 
-                close_nointr_nofail(p->fd);
+                p->fd = safe_close(p->fd);
 
                 /* One little note: we should never delete any sockets
                  * in the file system here! After all some other
@@ -709,8 +707,6 @@ static void socket_close_fds(Socket *s) {
                  * we delete sockets in the file system before we
                  * create a new one, not after we stopped using
                  * one! */
-
-                p->fd = -1;
         }
 }
 
@@ -880,9 +876,7 @@ static int fifo_address_create(
 
 fail:
         label_context_clear();
-
-        if (fd >= 0)
-                close_nointr_nofail(fd);
+        safe_close(fd);
 
         return r;
 }
@@ -917,8 +911,7 @@ static int special_address_create(
         return 0;
 
 fail:
-        if (fd >= 0)
-                close_nointr_nofail(fd);
+        safe_close(fd);
 
         return r;
 }
@@ -977,9 +970,7 @@ static int mq_address_create(
         return 0;
 
 fail:
-        if (fd >= 0)
-                close_nointr_nofail(fd);
-
+        safe_close(fd);
         return r;
 }
 
@@ -1472,7 +1463,7 @@ static void socket_enter_running(Socket *s, int cfd) {
                 log_debug_unit(UNIT(s)->id, "Suppressing connection request on %s since unit stop is scheduled.", UNIT(s)->id);
 
                 if (cfd >= 0)
-                        close_nointr_nofail(cfd);
+                        safe_close(cfd);
                 else  {
                         /* Flush all sockets by closing and reopening them */
                         socket_close_fds(s);
@@ -1520,7 +1511,7 @@ static void socket_enter_running(Socket *s, int cfd) {
 
                 if (s->n_connections >= s->max_connections) {
                         log_warning_unit(UNIT(s)->id, "%s: Too many incoming connections (%u)", UNIT(s)->id, s->n_connections);
-                        close_nointr_nofail(cfd);
+                        safe_close(cfd);
                         return;
                 }
 
@@ -1535,7 +1526,7 @@ static void socket_enter_running(Socket *s, int cfd) {
 
                         /* ENOTCONN is legitimate if TCP RST was received.
                          * This connection is over, but the socket unit lives on. */
-                        close_nointr_nofail(cfd);
+                        safe_close(cfd);
                         return;
                 }
 
@@ -1586,9 +1577,7 @@ fail:
                          bus_error_message(&error, r));
 
         socket_enter_stop_pre(s, SOCKET_FAILURE_RESOURCES);
-
-        if (cfd >= 0)
-                close_nointr_nofail(cfd);
+        safe_close(cfd);
 }
 
 static void socket_run_next(Socket *s) {
@@ -1819,8 +1808,7 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
                                         break;
 
                         if (p) {
-                                if (p->fd >= 0)
-                                        close_nointr_nofail(p->fd);
+                                safe_close(p->fd);
                                 p->fd = fdset_remove(fds, fd);
                         }
                 }
@@ -1839,8 +1827,7 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
                                         break;
 
                         if (p) {
-                                if (p->fd >= 0)
-                                        close_nointr_nofail(p->fd);
+                                safe_close(p->fd);
                                 p->fd = fdset_remove(fds, fd);
                         }
                 }
@@ -1859,8 +1846,7 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
                                         break;
 
                         if (p) {
-                                if (p->fd >= 0)
-                                        close_nointr_nofail(p->fd);
+                                safe_close(p->fd);
                                 p->fd = fdset_remove(fds, fd);
                         }
                 }
@@ -1878,8 +1864,7 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
                                         break;
 
                         if (p) {
-                                if (p->fd >= 0)
-                                        close_nointr_nofail(p->fd);
+                                safe_close(p->fd);
                                 p->fd = fdset_remove(fds, fd);
                         }
                 }
@@ -1897,8 +1882,7 @@ static int socket_deserialize_item(Unit *u, const char *key, const char *value,
                                         break;
 
                         if (p) {
-                                if (p->fd >= 0)
-                                        close_nointr_nofail(p->fd);
+                                safe_close(p->fd);
                                 p->fd = fdset_remove(fds, fd);
                         }
                 }
diff --git a/src/core/switch-root.c b/src/core/switch-root.c
index ce0e41d51..518ec1f0a 100644
--- a/src/core/switch-root.c
+++ b/src/core/switch-root.c
@@ -41,11 +41,10 @@ int switch_root(const char *new_root) {
                 "/sys\0"
                 "/run\0";
 
-        int r, old_root_fd = -1;
+        _cleanup_close_ int old_root_fd = -1;
         struct stat new_root_stat;
         bool old_root_remove;
-        const char *i;
-        _cleanup_free_ char *temporary_old_root = NULL;
+        const char *i, *temporary_old_root;
 
         if (path_equal(new_root, "/"))
                 return 0;
@@ -56,16 +55,13 @@ int switch_root(const char *new_root) {
          * directory we choose for this, but it should be more likely
          * than not that /mnt exists and is suitable as mount point
          * and is on the same fs as the old root dir */
-        temporary_old_root = strappend(new_root, "/mnt");
-        if (!temporary_old_root)
-                return -ENOMEM;
+        temporary_old_root = strappenda(new_root, "/mnt");
 
         old_root_remove = in_initrd();
 
         if (stat(new_root, &new_root_stat) < 0) {
-                r = -errno;
                 log_error("Failed to stat directory %s: %m", new_root);
-                goto fail;
+                return -errno;
         }
 
         /* Work-around for a kernel bug: for some reason the kernel
@@ -104,9 +100,8 @@ int switch_root(const char *new_root) {
         }
 
         if (chdir(new_root) < 0) {
-                r = -errno;
                 log_error("Failed to change directory to %s: %m", new_root);
-                goto fail;
+                return -errno;
         }
 
         if (old_root_remove) {
@@ -123,27 +118,23 @@ int switch_root(const char *new_root) {
                 /* Immediately get rid of the old root. Since we are
                  * running off it we need to do this lazily. */
                 if (umount2(temporary_old_root, MNT_DETACH) < 0) {
-                        r = -errno;
                         log_error("Failed to umount old root dir %s: %m", temporary_old_root);
-                        goto fail;
+                        return -errno;
                 }
 
         } else if (mount(new_root, "/", NULL, MS_MOVE, NULL) < 0) {
-                r = -errno;
                 log_error("Failed to mount moving %s to /: %m", new_root);
-                goto fail;
+                return -errno;
         }
 
         if (chroot(".") < 0) {
-                r = -errno;
                 log_error("Failed to change root: %m");
-                goto fail;
+                return -errno;
         }
 
         if (chdir("/") < 0) {
-                r = -errno;
                 log_error("Failed to change directory: %m");
-                goto fail;
+                return -errno;
         }
 
         if (old_root_fd >= 0) {
@@ -157,11 +148,5 @@ int switch_root(const char *new_root) {
                 }
         }
 
-        r = 0;
-
-fail:
-        if (old_root_fd >= 0)
-                close_nointr_nofail(old_root_fd);
-
-        return r;
+        return 0;
 }
diff --git a/src/core/umount.c b/src/core/umount.c
index 2d166c1c9..d1258f0f0 100644
--- a/src/core/umount.c
+++ b/src/core/umount.c
@@ -329,14 +329,14 @@ static int dm_list_get(MountPoint **head) {
 }
 
 static int delete_loopback(const char *device) {
-        int fd, r;
+        _cleanup_close_ int fd = -1;
+        int r;
 
-        if ((fd = open(device, O_RDONLY|O_CLOEXEC)) < 0)
+        fd = open(device, O_RDONLY|O_CLOEXEC);
+        if (fd < 0)
                 return errno == ENOENT ? 0 : -errno;
 
         r = ioctl(fd, LOOP_CLR_FD, 0);
-        close_nointr_nofail(fd);
-
         if (r >= 0)
                 return 1;