tmpfile-util: introduce new CLEANUP_TMPFILE_AT() API

This should allow us to get rid of a bunch of "fail:" labels, because we
can clean up tmpfiles relative to some atfd this way.

This only ports over a small number of potential users, but there's more
work to be done.

diff --git a/src/basic/env-file.c b/src/basic/env-file.c
index 968cd756bdce8e8022743d70f70b930fb142e041..fda52d08eccbc1f6a1784d81501d0b9da8bf186e 100644 (file)
--- a/src/basic/env-file.c
+++ b/src/basic/env-file.c
@@ -650,14 +650,16 @@ int write_env_file(int dir_fd, const char *fname, char **headers, char **l, Writ
         }
 
         r = fopen_tmpfile_linkable_at(dir_fd, fname, O_WRONLY|O_CLOEXEC, &p, &f);
-        if (call_label_ops_post)
-                RET_GATHER(r, label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fname, /* created= */ !!f));
+        int k = call_label_ops_post ? label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fname, /* created= */ !!f) : 0;
         if (r < 0)
                 return r;
+        CLEANUP_TMPFILE_AT(dir_fd, p);
+        if (k < 0)
+                return k;
 
         r = fchmod_umask(fileno(f), 0644);
         if (r < 0)
-                goto fail;
+                return r;
 
         STRV_FOREACH(i, headers) {
                 assert(isempty(*i) || startswith(*i, "#"));
@@ -670,15 +672,11 @@ int write_env_file(int dir_fd, const char *fname, char **headers, char **l, Writ
 
         r = flink_tmpfile_at(f, dir_fd, p, fname, LINK_TMPFILE_REPLACE|LINK_TMPFILE_SYNC);
         if (r < 0)
-                goto fail;
-
-        return 0;
+                return r;
 
-fail:
-        if (p)
-                (void) unlinkat(dir_fd, p, 0);
+        p = mfree(p); /* disarm CLEANUP_TMPFILE_AT() */
 
-        return r;
+        return 0;
 }
 
 int write_vconsole_conf(int dir_fd, const char *fname, char **l) {

diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 97f92213da57966583ca34903dc37e1dd2fa2fe2..a2fb5c7649c92de9567c728db3cf7c1d4542c84f 100644 (file)
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -240,24 +240,28 @@ static int write_string_file_atomic_at(
         }
 
         r = fopen_temporary_at(dir_fd, fn, &f, &p);
-        if (call_label_ops_post)
-                /* If fopen_temporary_at() failed in the above, propagate the error code, and ignore failures
-                 * in label_ops_post(). */
-                RET_GATHER(r, label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fn, /* created= */ !!f));
+        int k = call_label_ops_post ? label_ops_post(f ? fileno(f) : dir_fd, f ? NULL : fn, /* created= */ !!f) : 0;
+        /* If fopen_temporary_at() failed in the above, propagate the error code, and ignore failures in
+         * label_ops_post(). */
         if (r < 0)
-                goto fail;
+                return r;
+        CLEANUP_TMPFILE_AT(dir_fd, p);
+        if (k < 0)
+                return k;
 
         r = write_string_stream_full(f, line, flags, ts);
         if (r < 0)
-                goto fail;
+                return r;
 
         r = fchmod_umask(fileno(f), mode);
         if (r < 0)
-                goto fail;
+                return r;
 
         r = RET_NERRNO(renameat(dir_fd, p, dir_fd, fn));
         if (r < 0)
-                goto fail;
+                return r;
+
+        p = mfree(p); /* disarm CLEANUP_TMPFILE_AT() */
 
         if (FLAGS_SET(flags, WRITE_STRING_FILE_SYNC)) {
                 /* Sync the rename, too */
@@ -267,11 +271,6 @@ static int write_string_file_atomic_at(
         }
 
         return 0;
-
-fail:
-        if (f)
-                (void) unlinkat(dir_fd, p, 0);
-        return r;
 }
 
 int write_string_file_full(

diff --git a/src/basic/tmpfile-util.c b/src/basic/tmpfile-util.c
index 6e2069de7cbe06fc0088cc47d94b9666f3c6d33b..18a69367f1335ad70ddbd0ea3675bbec9e679a38 100644 (file)
--- a/src/basic/tmpfile-util.c
+++ b/src/basic/tmpfile-util.c
@@ -436,3 +436,17 @@ int mkdtemp_open(const char *template, int flags, char **ret) {
 
         return fd;
 }
+
+void cleanup_tmpfile_data_done(struct cleanup_tmpfile_data *d) {
+        assert(d);
+
+        if (!d->dir_fd ||
+            *d->dir_fd < 0 ||
+            !d->filename ||
+            !*d->filename)
+                return;
+
+        (void) unlinkat(*d->dir_fd, *d->filename, 0);
+        d->dir_fd = NULL;
+        d->filename = NULL;
+}

diff --git a/src/basic/tmpfile-util.h b/src/basic/tmpfile-util.h
index b39680f078bff50febcd23deb19925004e4c34e0..cbaae5dbb3d012b9df94de542c496237c054f0ec 100644 (file)
--- a/src/basic/tmpfile-util.h
+++ b/src/basic/tmpfile-util.h
@@ -48,3 +48,14 @@ static inline int flink_tmpfile(FILE *f, const char *path, const char *target, L
 
 int mkdtemp_malloc(const char *template, char **ret);
 int mkdtemp_open(const char *template, int flags, char **ret);
+
+/* A helper for removing link_tmpfile_at() via _cleanup_() */
+struct cleanup_tmpfile_data {
+        int *dir_fd;
+        char **filename;
+};
+void cleanup_tmpfile_data_done(struct cleanup_tmpfile_data *d);
+#define _CLEANUP_TMPFILE_AT(u, dir_fd, filename)                        \
+        _unused_ _cleanup_(cleanup_tmpfile_data_done) struct cleanup_tmpfile_data UNIQ_T(cta, u) = { &(dir_fd), &(filename) }
+#define CLEANUP_TMPFILE_AT(dir_fd, filename)                            \
+        _CLEANUP_TMPFILE_AT(UNIQ, dir_fd, filename)

diff --git a/src/shared/creds-util.c b/src/shared/creds-util.c
index c54349b9f7d22e87e7f7f030dd3fd5ff02e5907e..0d1a2da651137e96228a388f8057911e88b7c644 100644 (file)
--- a/src/shared/creds-util.c
+++ b/src/shared/creds-util.c
@@ -393,6 +393,8 @@ static int make_credential_host_secret(
         if (fd < 0)
                 return log_debug_errno(fd, "Failed to create temporary file for credential host secret: %m");
 
+        CLEANUP_TMPFILE_AT(dfd, t);
+
         r = chattr_secret(fd, 0);
         if (r < 0)
                 log_debug_errno(r, "Failed to set file attributes for secrets file, ignoring: %m");
@@ -405,29 +407,22 @@ static int make_credential_host_secret(
 
         r = crypto_random_bytes(buf.data, sizeof(buf.data));
         if (r < 0)
-                goto fail;
+                return r;
 
         r = loop_write(fd, &buf, sizeof(buf));
         if (r < 0)
-                goto fail;
-
-        if (fchmod(fd, 0400) < 0) {
-                r = -errno;
-                goto fail;
-        }
+                return r;
 
-        if (fsync(fd) < 0) {
-                r = -errno;
-                goto fail;
-        }
+        if (fchmod(fd, 0400) < 0)
+                return -errno;
 
         warn_not_encrypted(fd, flags, dirname, fn);
 
         r = link_tmpfile_at(fd, dfd, t, fn, LINK_TMPFILE_SYNC);
-        if (r < 0) {
-                log_debug_errno(r, "Failed to link host key into place: %m");
-                goto fail;
-        }
+        if (r < 0)
+                return log_debug_errno(r, "Failed to link host key into place: %m");
+
+        t = mfree(t); /* disarm CLEANUP_ERASE() */
 
         if (ret) {
                 void *copy;
@@ -440,12 +435,6 @@ static int make_credential_host_secret(
         }
 
         return 0;
-
-fail:
-        if (t && unlinkat(dfd, t, 0) < 0)
-                log_debug_errno(errno, "Failed to remove temporary credential key: %m");
-
-        return r;
 }
 
 int get_credential_host_secret(CredentialSecretFlags flags, struct iovec *ret) {