From: Lennart Poettering <lennart@poettering.net>
Date: Mon, 19 May 2025 08:15:25 +0000 (+0200)
Subject: logind: port logind state files to fopen_tmpfile_linkable()
X-Git-Tag: v258-rc1~529^2~4
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=019c8ea26a5b99584c60570caee8a573d4fafcfc;p=thirdparty%2Fsystemd.git

logind: port logind state files to fopen_tmpfile_linkable()

This replaces use of fopen_temporary() with fopen_tmpfile_linkable() +
flink_tmpfile(). This both shortens the code and means we use O_TMPFILE
for installing these files, which is always good.

No change in behaviour otherwise.
---

diff --git a/src/login/logind-inhibit.c b/src/login/logind-inhibit.c
index e5070ff5738..c8ba63ddfa6 100644
--- a/src/login/logind-inhibit.c
+++ b/src/login/logind-inhibit.c
@@ -89,21 +89,22 @@ Inhibitor* inhibitor_free(Inhibitor *i) {
 }
 
 static int inhibitor_save(Inhibitor *i) {
-        _cleanup_(unlink_and_freep) char *temp_path = NULL;
-        _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(i);
 
         r = mkdir_safe_label("/run/systemd/inhibit", 0755, 0, 0, MKDIR_WARN_MODE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create /run/systemd/inhibit/: %m");
 
-        r = fopen_temporary(i->state_file, &f, &temp_path);
+        _cleanup_(unlink_and_freep) char *temp_path = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        r = fopen_tmpfile_linkable(i->state_file, O_WRONLY|O_CLOEXEC, &temp_path, &f);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create state file '%s': %m", i->state_file);
 
-        (void) fchmod(fileno(f), 0644);
+        if (fchmod(fileno(f), 0644) < 0)
+                return log_error_errno(errno, "Failed to set access mode for state file '%s' to 0644: %m", i->state_file);
 
         fprintf(f,
                 "# This is private data. Do not parse.\n"
@@ -120,10 +121,8 @@ static int inhibitor_save(Inhibitor *i) {
                 _cleanup_free_ char *cc = NULL;
 
                 cc = cescape(i->who);
-                if (!cc) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (!cc)
+                        return log_oom();
 
                 fprintf(f, "WHO=%s\n", cc);
         }
@@ -132,10 +131,8 @@ static int inhibitor_save(Inhibitor *i) {
                 _cleanup_free_ char *cc = NULL;
 
                 cc = cescape(i->why);
-                if (!cc) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (!cc)
+                        return log_oom();
 
                 fprintf(f, "WHY=%s\n", cc);
         }
@@ -143,22 +140,12 @@ static int inhibitor_save(Inhibitor *i) {
         if (i->fifo_path)
                 fprintf(f, "FIFO=%s\n", i->fifo_path);
 
-        r = fflush_and_check(f);
+        r = flink_tmpfile(f, temp_path, i->state_file, LINK_TMPFILE_REPLACE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to move '%s' into place: %m", i->state_file);
 
-        if (rename(temp_path, i->state_file) < 0) {
-                r = -errno;
-                goto fail;
-        }
-
-        temp_path = mfree(temp_path);
+        temp_path = mfree(temp_path); /* disarm auto-destroy: temporary file does not exist anymore */
         return 0;
-
-fail:
-        (void) unlink(i->state_file);
-
-        return log_error_errno(r, "Failed to save inhibit data %s: %m", i->state_file);
 }
 
 static int bus_manager_send_inhibited_change(Inhibitor *i) {
diff --git a/src/login/logind-seat.c b/src/login/logind-seat.c
index fb52c5475e6..22690dcf66b 100644
--- a/src/login/logind-seat.c
+++ b/src/login/logind-seat.c
@@ -88,8 +88,6 @@ Seat* seat_free(Seat *s) {
 }
 
 int seat_save(Seat *s) {
-        _cleanup_(unlink_and_freep) char *temp_path = NULL;
-        _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(s);
@@ -99,13 +97,16 @@ int seat_save(Seat *s) {
 
         r = mkdir_safe_label("/run/systemd/seats", 0755, 0, 0, MKDIR_WARN_MODE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create /run/systemd/seats/: %m");
 
-        r = fopen_temporary(s->state_file, &f, &temp_path);
+        _cleanup_(unlink_and_freep) char *temp_path = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        r = fopen_tmpfile_linkable(s->state_file, O_WRONLY|O_CLOEXEC, &temp_path, &f);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create state file '%s': %m", s->state_file);
 
-        (void) fchmod(fileno(f), 0644);
+        if (fchmod(fileno(f), 0644) < 0)
+                return log_error_errno(errno, "Failed to set access mode for state file '%s' to 0644: %m", s->state_file);
 
         fprintf(f,
                 "# This is private data. Do not parse.\n"
@@ -144,21 +145,12 @@ int seat_save(Seat *s) {
                                 i->sessions_by_seat_next ? ' ' : '\n');
         }
 
-        r = fflush_and_check(f);
+        r = flink_tmpfile(f, temp_path, s->state_file, LINK_TMPFILE_REPLACE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to move '%s' into place: %m", s->state_file);
 
-        if (rename(temp_path, s->state_file) < 0) {
-                r = -errno;
-                goto fail;
-        }
-
-        temp_path = mfree(temp_path);
+        temp_path = mfree(temp_path); /* disarm auto-destroy: temporary file does not exist anymore */
         return 0;
-
-fail:
-        (void) unlink(s->state_file);
-        return log_error_errno(r, "Failed to save seat data %s: %m", s->state_file);
 }
 
 int seat_load(Seat *s) {
diff --git a/src/login/logind-session.c b/src/login/logind-session.c
index a540774ccaf..2ed8c428f57 100644
--- a/src/login/logind-session.c
+++ b/src/login/logind-session.c
@@ -276,8 +276,6 @@ static void session_save_devices(Session *s, FILE *f) {
 }
 
 int session_save(Session *s) {
-        _cleanup_(unlink_and_freep) char *temp_path = NULL;
-        _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(s);
@@ -290,13 +288,16 @@ int session_save(Session *s) {
 
         r = mkdir_safe_label("/run/systemd/sessions", 0755, 0, 0, MKDIR_WARN_MODE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create /run/systemd/sessions/: %m");
 
-        r = fopen_temporary(s->state_file, &f, &temp_path);
+        _cleanup_(unlink_and_freep) char *temp_path = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        r = fopen_tmpfile_linkable(s->state_file, O_WRONLY|O_CLOEXEC, &temp_path, &f);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create state file '%s': %m", s->state_file);
 
-        (void) fchmod(fileno(f), 0644);
+        if (fchmod(fileno(f), 0644) < 0)
+                return log_error_errno(errno, "Failed to set access mode for state file '%s' to 0644: %m", s->state_file);
 
         fprintf(f,
                 "# This is private data. Do not parse.\n"
@@ -345,10 +346,8 @@ int session_save(Session *s) {
                 _cleanup_free_ char *escaped = NULL;
 
                 escaped = cescape(s->remote_host);
-                if (!escaped) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (!escaped)
+                        return log_oom();
 
                 fprintf(f, "REMOTE_HOST=%s\n", escaped);
         }
@@ -357,10 +356,8 @@ int session_save(Session *s) {
                 _cleanup_free_ char *escaped = NULL;
 
                 escaped = cescape(s->remote_user);
-                if (!escaped) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (!escaped)
+                        return log_oom();
 
                 fprintf(f, "REMOTE_USER=%s\n", escaped);
         }
@@ -369,10 +366,8 @@ int session_save(Session *s) {
                 _cleanup_free_ char *escaped = NULL;
 
                 escaped = cescape(s->service);
-                if (!escaped) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (!escaped)
+                        return log_oom();
 
                 fprintf(f, "SERVICE=%s\n", escaped);
         }
@@ -381,10 +376,8 @@ int session_save(Session *s) {
                 _cleanup_free_ char *escaped = NULL;
 
                 escaped = cescape(s->desktop);
-                if (!escaped) {
-                        r = -ENOMEM;
-                        goto fail;
-                }
+                if (!escaped)
+                        return log_oom();
 
                 fprintf(f, "DESKTOP=%s\n", escaped);
         }
@@ -413,22 +406,12 @@ int session_save(Session *s) {
                 session_save_devices(s, f);
         }
 
-        r = fflush_and_check(f);
+        r = flink_tmpfile(f, temp_path, s->state_file, LINK_TMPFILE_REPLACE);
         if (r < 0)
-                goto fail;
-
-        if (rename(temp_path, s->state_file) < 0) {
-                r = -errno;
-                goto fail;
-        }
+                return log_error_errno(r, "Failed to move '%s' into place: %m", s->state_file);
 
-        temp_path = mfree(temp_path);
+        temp_path = mfree(temp_path); /* disarm auto-destroy: temporary file does not exist anymore */
         return 0;
-
-fail:
-        (void) unlink(s->state_file);
-
-        return log_error_errno(r, "Failed to save session data %s: %m", s->state_file);
 }
 
 static int session_load_devices(Session *s, const char *devices) {
diff --git a/src/login/logind-user.c b/src/login/logind-user.c
index d5795e2a7fc..c854b1e5fd5 100644
--- a/src/login/logind-user.c
+++ b/src/login/logind-user.c
@@ -144,8 +144,6 @@ User *user_free(User *u) {
 }
 
 static int user_save_internal(User *u) {
-        _cleanup_(unlink_and_freep) char *temp_path = NULL;
-        _cleanup_fclose_ FILE *f = NULL;
         int r;
 
         assert(u);
@@ -153,13 +151,16 @@ static int user_save_internal(User *u) {
 
         r = mkdir_safe_label("/run/systemd/users", 0755, 0, 0, MKDIR_WARN_MODE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create /run/systemd/users/: %m");
 
-        r = fopen_temporary(u->state_file, &f, &temp_path);
+        _cleanup_(unlink_and_freep) char *temp_path = NULL;
+        _cleanup_fclose_ FILE *f = NULL;
+        r = fopen_tmpfile_linkable(u->state_file, O_WRONLY|O_CLOEXEC, &temp_path, &f);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to create state file '%s': %m", u->state_file);
 
-        (void) fchmod(fileno(f), 0644);
+        if (fchmod(fileno(f), 0644) < 0)
+                return log_error_errno(errno, "Failed to set access mode for state file '%s' to 0644: %m", u->state_file);
 
         fprintf(f,
                 "# This is private data. Do not parse.\n"
@@ -282,22 +283,12 @@ static int user_save_internal(User *u) {
                 fputc('\n', f);
         }
 
-        r = fflush_and_check(f);
+        r = flink_tmpfile(f, temp_path, u->state_file, LINK_TMPFILE_REPLACE);
         if (r < 0)
-                goto fail;
+                return log_error_errno(r, "Failed to move '%s' into place: %m", u->state_file);
 
-        if (rename(temp_path, u->state_file) < 0) {
-                r = -errno;
-                goto fail;
-        }
-
-        temp_path = mfree(temp_path);
+        temp_path = mfree(temp_path); /* disarm auto-destroy: temporary file does not exist anymore */
         return 0;
-
-fail:
-        (void) unlink(u->state_file);
-
-        return log_error_errno(r, "Failed to save user data %s: %m", u->state_file);
 }
 
 int user_save(User *u) {