Merge pull request #15278 from vcaputo/more-trivial-cleanups

Expand use of _cleanup_close_ where trivial

src/basic/copy.c

@@ -15,6 +15,7 @@
 #include "copy.h"
 #include "dirent-util.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "fs-util.h"
 #include "io-util.h"
 #include "macro.h"
@@ -569,10 +570,9 @@ static int fd_copy_directory(
         if (fdf < 0)
                 return -errno;
 
-        d = fdopendir(fdf);
+        d = take_fdopendir(&fdf);
         if (!d)
                 return -errno;
-        fdf = -1;
 
         exists = false;
         if (copy_flags & COPY_MERGE_EMPTY) {

src/basic/fileio.c

@@ -54,6 +54,44 @@ int fdopen_unlocked(int fd, const char *options, FILE **ret) {
         return 0;
 }
 
+int take_fdopen_unlocked(int *fd, const char *options, FILE **ret) {
+        int     r;
+
+        assert(fd);
+
+        r = fdopen_unlocked(*fd, options, ret);
+        if (r < 0)
+                return r;
+
+        *fd = -1;
+
+        return 0;
+}
+
+FILE* take_fdopen(int *fd, const char *options) {
+        assert(fd);
+
+        FILE *f = fdopen(*fd, options);
+        if (!f)
+                return NULL;
+
+        *fd = -1;
+
+        return f;
+}
+
+DIR* take_fdopendir(int *dfd) {
+        assert(dfd);
+
+        DIR *d = fdopendir(*dfd);
+        if (!d)
+                return NULL;
+
+        *dfd = -1;
+
+        return d;
+}
+
 FILE* open_memstream_unlocked(char **ptr, size_t *sizeloc) {
         FILE *f = open_memstream(ptr, sizeloc);
         if (!f)

src/basic/fileio.h

@@ -39,6 +39,9 @@ typedef enum {
 
 int fopen_unlocked(const char *path, const char *options, FILE **ret);
 int fdopen_unlocked(int fd, const char *options, FILE **ret);
+int take_fdopen_unlocked(int *fd, const char *options, FILE **ret);
+FILE* take_fdopen(int *fd, const char *options);
+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);
 

src/basic/stat-util.c

@@ -10,6 +10,7 @@
 #include "alloc-util.h"
 #include "dirent-util.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "fs-util.h"
 #include "macro.h"
 #include "missing_fs.h"
@@ -77,10 +78,9 @@ int dir_is_empty_at(int dir_fd, const char *path) {
         if (fd < 0)
                 return -errno;
 
-        d = fdopendir(fd);
+        d = take_fdopendir(&fd);
         if (!d)
                 return -errno;
-        fd = -1;
 
         FOREACH_DIRENT(de, d, return -errno)
                 return 0;

src/basic/tmpfile-util.c

@@ -48,14 +48,12 @@ int fopen_temporary(const char *path, FILE **ret_f, char **ret_temp_path) {
         /* This assumes that returned FILE object is short-lived and used within the same single-threaded
          * context and never shared externally, hence locking is not necessary. */
 
-        r = fdopen_unlocked(fd, "w", &f);
+        r = take_fdopen_unlocked(&fd, "w", &f);
         if (r < 0) {
                 (void) unlink(t);
                 return r;
         }
 
-        TAKE_FD(fd);
-
         if (ret_f)
                 *ret_f = TAKE_PTR(f);
 
@@ -80,18 +78,16 @@ int mkostemp_safe(char *pattern) {
 }
 
 int fmkostemp_safe(char *pattern, const char *mode, FILE **ret_f) {
-        int fd;
+        _cleanup_close_ int fd = -1;
         FILE *f;
 
         fd = mkostemp_safe(pattern);
         if (fd < 0)
                 return fd;
 
-        f = fdopen(fd, mode);
-        if (!f) {
-                safe_close(fd);
+        f = take_fdopen(&fd, mode);
+        if (!f)
                 return -errno;
-        }
 
         *ret_f = f;
         return 0;

src/boot/bootctl.c

@@ -982,8 +982,9 @@ static int install_loader_config(const char *esp_path, sd_id128_t machine_id) {
         char machine_string[SD_ID128_STRING_MAX];
         _cleanup_(unlink_and_freep) char *t = NULL;
         _cleanup_fclose_ FILE *f = NULL;
+        _cleanup_close_ int fd = -1;
         const char *p;
-        int r, fd;
+        int r;
 
         p = prefix_roota(esp_path, "/loader/loader.conf");
         if (access(p, F_OK) >= 0) /* Silently skip creation if the file already exists (early check) */
@@ -993,11 +994,9 @@ static int install_loader_config(const char *esp_path, sd_id128_t machine_id) {
         if (fd < 0)
                 return log_error_errno(fd, "Failed to open \"%s\" for writing: %m", p);
 
-        f = fdopen(fd, "w");
-        if (!f) {
-                safe_close(fd);
+        f = take_fdopen(&fd, "w");
+        if (!f)
                 return log_oom();
-        }
 
         fprintf(f, "#timeout 3\n"
                    "#console-mode keep\n"

src/core/manager.c

@@ -3107,7 +3107,7 @@ void manager_send_unit_plymouth(Manager *m, Unit *u) {
 }
 
 int manager_open_serialization(Manager *m, FILE **_f) {
-        int fd;
+        _cleanup_close_ int fd = -1;
         FILE *f;
 
         assert(_f);
@@ -3116,11 +3116,9 @@ int manager_open_serialization(Manager *m, FILE **_f) {
         if (fd < 0)
                 return fd;
 
-        f = fdopen(fd, "w+");
-        if (!f) {
-                safe_close(fd);
+        f = take_fdopen(&fd, "w+");
+        if (!f)
                 return -errno;
-        }
 
         *_f = f;
         return 0;

src/coredump/coredump.c

@@ -560,7 +560,7 @@ static int compose_open_fds(pid_t pid, char **open_fds) {
         FOREACH_DIRENT(dent, proc_fd_dir, return -errno) {
                 _cleanup_fclose_ FILE *fdinfo = NULL;
                 _cleanup_free_ char *fdname = NULL;
-                int fd;
+                _cleanup_close_ int fd = -1;
 
                 r = readlinkat_malloc(dirfd(proc_fd_dir), dent->d_name, &fdname);
                 if (r < 0)
@@ -574,11 +574,9 @@ static int compose_open_fds(pid_t pid, char **open_fds) {
                 if (fd < 0)
                         continue;
 
-                fdinfo = fdopen(fd, "r");
-                if (!fdinfo) {
-                        safe_close(fd);
+                fdinfo = take_fdopen(&fd, "r");
+                if (!fdinfo)
                         continue;
-                }
 
                 for (;;) {
                         _cleanup_free_ char *line = NULL;

src/home/homed-home.c

@@ -360,12 +360,10 @@ static int home_parse_worker_stdout(int _fd, UserRecord **ret) {
         if (lseek(fd, SEEK_SET, 0) == (off_t) -1)
                 return log_error_errno(errno, "Failed to seek to beginning of memfd: %m");
 
-        f = fdopen(fd, "r");
+        f = take_fdopen(&fd, "r");
         if (!f)
                 return log_error_errno(errno, "Failed to reopen memfd: %m");
 
-        TAKE_FD(fd);
-
         if (DEBUG_LOGGING) {
                 _cleanup_free_ char *text = NULL;
 

src/home/homework-cifs.c

@@ -142,7 +142,8 @@ int home_create_cifs(UserRecord *h, UserRecord **ret_home) {
         _cleanup_(home_setup_undo) HomeSetup setup = HOME_SETUP_INIT;
         _cleanup_(user_record_unrefp) UserRecord *new_home = NULL;
         _cleanup_(closedirp) DIR *d = NULL;
-        int r, copy;
+        _cleanup_close_ int copy = -1;
+        int r;
 
         assert(h);
         assert(user_record_storage(h) == USER_CIFS);
@@ -166,11 +167,9 @@ int home_create_cifs(UserRecord *h, UserRecord **ret_home) {
         if (copy < 0)
                 return -errno;
 
-        d = fdopendir(copy);
-        if (!d) {
-                safe_close(copy);
+        d = take_fdopendir(&copy);
+        if (!d)
                 return -errno;
-        }
 
         errno = 0;
         if (readdir_no_dot(d))

src/home/homework.c

@@ -38,7 +38,6 @@ int user_record_authenticate(
 
         bool need_password = false, need_token = false, need_pin = false, need_protected_authentication_path_permitted = false,
                 pin_locked = false, pin_incorrect = false, pin_incorrect_few_tries_left = false, pin_incorrect_one_try_left = false;
-        size_t n;
         int r;
 
         assert(h);
@@ -70,7 +69,7 @@ int user_record_authenticate(
         }
 
         /* Second, let's see if any of the PKCS#11 security tokens are plugged in and help us */
-        for (n = 0; n < h->n_pkcs11_encrypted_key; n++) {
+        for (size_t n = 0; n < h->n_pkcs11_encrypted_key; n++) {
 #if HAVE_P11KIT
                 _cleanup_(pkcs11_callback_data_release) struct pkcs11_callback_data data = {
                         .user_record = h,
@@ -280,12 +279,10 @@ static int read_identity_file(int root_fd, JsonVariant **ret) {
         if (r < 0)
                 return log_error_errno(r, "Embedded identity file is not a regular file, refusing: %m");
 
-        identity_file = fdopen(identity_fd, "r");
+        identity_file = take_fdopen(&identity_fd, "r");
         if (!identity_file)
                 return log_oom();
 
-        identity_fd = -1;
-
         r = json_parse_file(identity_file, ".identity", JSON_PARSE_SENSITIVE, ret, &line, &column);
         if (r < 0)
                 return log_error_errno(r, "[.identity:%u:%u] Failed to parse JSON data: %m", line, column);
@@ -319,14 +316,12 @@ static int write_identity_file(int root_fd, JsonVariant *v, uid_t uid) {
         if (identity_fd < 0)
                 return log_error_errno(errno, "Failed to create .identity file in home directory: %m");
 
-        identity_file = fdopen(identity_fd, "w");
+        identity_file = take_fdopen(&identity_fd, "w");
         if (!identity_file) {
                 r = log_oom();
                 goto fail;
         }
 
-        identity_fd = -1;
-
         json_variant_dump(normalized, JSON_FORMAT_PRETTY, identity_file, NULL);
 
         r = fflush_and_check(identity_file);

src/journal-remote/journal-gatewayd.c

@@ -123,17 +123,15 @@ static int request_meta_ensure_tmp(RequestMeta *m) {
         if (m->tmp)
                 rewind(m->tmp);
         else {
-                int fd;
+                _cleanup_close_ int fd = -1;
 
                 fd = open_tmpfile_unlinkable("/tmp", O_RDWR|O_CLOEXEC);
                 if (fd < 0)
                         return fd;
 
-                m->tmp = fdopen(fd, "w+");
-                if (!m->tmp) {
-                        safe_close(fd);
+                m->tmp = take_fdopen(&fd, "w+");
+                if (!m->tmp)
                         return -errno;
-                }
         }
 
         return 0;

src/journal/sd-journal.c

@@ -1718,7 +1718,7 @@ static int add_root_directory(sd_journal *j, const char *p, bool missing_ok) {
                         goto fail;
                 }
         } else {
-                int dfd;
+                _cleanup_close_ int dfd = -1;
 
                 /* If there's no path specified, then we use the top-level fd itself. We duplicate the fd here, since
                  * opendir() will take possession of the fd, and close it, which we don't want. */
@@ -1731,10 +1731,9 @@ static int add_root_directory(sd_journal *j, const char *p, bool missing_ok) {
                         goto fail;
                 }
 
-                d = fdopendir(dfd);
+                d = take_fdopendir(&dfd);
                 if (!d) {
                         r = -errno;
-                        safe_close(dfd);
                         goto fail;
                 }
 

src/machine/machine-dbus.c

@@ -20,6 +20,7 @@
 #include "env-file.h"
 #include "env-util.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "format-util.h"
 #include "fs-util.h"
 #include "in-addr-util.h"
@@ -399,12 +400,10 @@ int bus_machine_method_get_os_release(sd_bus_message *message, void *userdata, s
 
                 pair[1] = safe_close(pair[1]);
 
-                f = fdopen(pair[0], "r");
+                f = take_fdopen(&pair[0], "r");
                 if (!f)
                         return -errno;
 
-                pair[0] = -1;
-
                 r = load_env_file_pairs(f, "/etc/os-release", &l);
                 if (r < 0)
                         return r;

src/machine/machined-dbus.c

@@ -620,12 +620,10 @@ static int clean_pool_done(Operation *operation, int ret, sd_bus_error *error) {
         if (lseek(operation->extra_fd, 0, SEEK_SET) == (off_t) -1)
                 return -errno;
 
-        f = fdopen(operation->extra_fd, "r");
+        f = take_fdopen(&operation->extra_fd, "r");
         if (!f)
                 return -errno;
 
-        operation->extra_fd = -1;
-
         /* The resulting temporary file starts with a boolean value that indicates success or not. */
         errno = 0;
         n = fread(&success, 1, sizeof(success), f);

src/nspawn/nspawn-patch-uid.c

@@ -8,6 +8,7 @@
 #include "acl-util.h"
 #include "dirent-util.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "fs-util.h"
 #include "missing_magic.h"
 #include "nspawn-def.h"
@@ -335,12 +336,11 @@ static int recurse_fd(int fd, bool donate_fd, const struct stat *st, uid_t shift
                         donate_fd = true;
                 }
 
-                d = fdopendir(fd);
+                d = take_fdopendir(&fd);
                 if (!d) {
                         r = -errno;
                         goto finish;
                 }
-                fd = -1;
 
                 FOREACH_DIRENT_ALL(de, d, r = -errno; goto finish) {
                         struct stat fst;

src/nspawn/nspawn-setuid.c

@@ -118,10 +118,9 @@ int change_uid_gid(const char *user, char **_home) {
         if (fd < 0)
                 return fd;
 
-        f = fdopen(fd, "r");
+        f = take_fdopen(&fd, "r");
         if (!f)
                 return log_oom();
-        fd = -1;
 
         r = read_line(f, LONG_LINE_MAX, &line);
         if (r == 0)
@@ -191,10 +190,9 @@ int change_uid_gid(const char *user, char **_home) {
         if (fd < 0)
                 return fd;
 
-        f = fdopen(fd, "r");
+        f = take_fdopen(&fd, "r");
         if (!f)
                 return log_oom();
-        fd = -1;
 
         r = read_line(f, LONG_LINE_MAX, &line);
         if (r == 0)

src/portable/portable.c

@@ -1092,10 +1092,9 @@ static int test_chroot_dropin(
                 return log_debug_errno(errno, "Failed to open %s/%s: %m", where, p);
         }
 
-        r = fdopen_unlocked(fd, "r", &f);
+        r = take_fdopen_unlocked(&fd, "r", &f);
         if (r < 0)
                 return log_debug_errno(r, "Failed to convert file handle: %m");
-        TAKE_FD(fd);
 
         r = read_line(f, LONG_LINE_MAX, &line);
         if (r < 0)

src/portable/portabled-image-bus.c

@@ -79,12 +79,10 @@ static int append_fd(sd_bus_message *m, PortableMetadata *d) {
         assert(d);
         assert(d->fd >= 0);
 
-        f = fdopen(d->fd, "r");
+        f = take_fdopen(&d->fd, "r");
         if (!f)
                 return -errno;
 
-        d->fd = -1;
-
         r = read_full_stream(f, &buf, &n);
         if (r < 0)
                 return r;

src/shared/ask-password-api.c

@@ -791,14 +791,12 @@ int ask_password_agent(
 
         (void) fchmod(fd, 0644);
 
-        f = fdopen(fd, "w");
+        f = take_fdopen(&fd, "w");
         if (!f) {
                 r = -errno;
                 goto finish;
         }
 
-        fd = -1;
-
         signal_fd = signalfd(-1, &mask, SFD_NONBLOCK|SFD_CLOEXEC);
         if (signal_fd < 0) {
                 r = -errno;

src/shared/dissect-image.c

@@ -1541,14 +1541,12 @@ int dissected_image_acquire_metadata(DissectedImage *m) {
 
                 fds[2*k+1] = safe_close(fds[2*k+1]);
 
-                f = fdopen(fds[2*k], "r");
+                f = take_fdopen(&fds[2*k], "r");
                 if (!f) {
                         r = -errno;
                         goto finish;
                 }
 
-                fds[2*k] = -1;
-
                 switch (k) {
 
                 case META_HOSTNAME:

src/shared/install.c

@@ -1280,10 +1280,9 @@ static int unit_file_load(
         if (r < 0)
                 return r;
 
-        f = fdopen(fd, "r");
+        f = take_fdopen(&fd, "r");
         if (!f)
                 return -errno;
-        fd = -1;
 
         /* c is only needed if we actually load the file (it's referenced from items[] btw, in case you wonder.) */
         assert(c);

src/shared/os-util.c

@@ -3,6 +3,7 @@
 #include "alloc-util.h"
 #include "env-file.h"
 #include "fd-util.h"
+#include "fileio.h"
 #include "fs-util.h"
 #include "macro.h"
 #include "os-util.h"
@@ -76,10 +77,9 @@ int fopen_os_release(const char *root, char **ret_path, FILE **ret_file) {
         if (r < 0)
                 return r;
 
-        f = fdopen(fd, "r");
+        f = take_fdopen(&fd, "r");
         if (!f)
                 return -errno;
-        fd = -1;
 
         *ret_file = f;