tree-wide: always use TAKE_FD() when calling rearrange_stdio()

rearrange_stdio() invalidates specified fds even on failure, which means
we should always invalidate the fds we pass in no matter what. Let's
make this explicit by using TAKE_FD() for that everywhere.

Note that in many places we such invalidation doesnt get us much
behaviour-wise, since we don't use the variables anymore later. But
TAKE_FD() in a way is also documentation, it encodes explicitly that the
fds are invalidated here, so I think it's a good thing to always make
this explicit here.

diff --git a/src/core/execute.c b/src/core/execute.c
index d8bbc694b4672a0eee6a372beab7fefa27cb2d9b..26f847c1b0131c976f7cff11d27b26e8f7bfb1f4 100644 (file)
--- a/src/core/execute.c
+++ b/src/core/execute.c
@@ -756,12 +756,16 @@ static int chown_terminal(int fd, uid_t uid) {
         return 1;
 }
 
-static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_stdout) {
+static int setup_confirm_stdio(
+                const char *vc,
+                int *ret_saved_stdin,
+                int *ret_saved_stdout) {
+
         _cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1;
         int r;
 
-        assert(_saved_stdin);
-        assert(_saved_stdout);
+        assert(ret_saved_stdin);
+        assert(ret_saved_stdout);
 
         saved_stdin = fcntl(STDIN_FILENO, F_DUPFD, 3);
         if (saved_stdin < 0)
@@ -783,16 +787,13 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st
         if (r < 0)
                 return r;
 
-        r = rearrange_stdio(fd, fd, STDERR_FILENO);
-        fd = -1;
+        r = rearrange_stdio(fd, fd, STDERR_FILENO); /* Invalidates 'fd' also on failure */
+        TAKE_FD(fd);
         if (r < 0)
                 return r;
 
-        *_saved_stdin = saved_stdin;
-        *_saved_stdout = saved_stdout;
-
-        saved_stdin = saved_stdout = -1;
-
+        *ret_saved_stdin = TAKE_FD(saved_stdin);
+        *ret_saved_stdout = TAKE_FD(saved_stdout);
         return 0;
 }
 

diff --git a/src/home/homed-home.c b/src/home/homed-home.c
index ccd99e3c9d4b5c2baabb4f7529536ff247389cb9..c111bfa782550e59208b55ff963d3216093e8d73 100644 (file)
--- a/src/home/homed-home.c
+++ b/src/home/homed-home.c
@@ -1202,13 +1202,12 @@ static int home_start_work(Home *h, const char *verb, UserRecord *hr, UserRecord
                 if (r < 0)
                         log_warning_errno(r, "Failed to update $SYSTEMD_EXEC_PID, ignoring: %m");
 
-                r = rearrange_stdio(stdin_fd, stdout_fd, STDERR_FILENO);
+                r = rearrange_stdio(TAKE_FD(stdin_fd), TAKE_FD(stdout_fd), STDERR_FILENO); /* fds are invalidated by rearrange_stdio() even on failure */
                 if (r < 0) {
                         log_error_errno(r, "Failed to rearrange stdin/stdout/stderr: %m");
                         _exit(EXIT_FAILURE);
                 }
 
-                stdin_fd = stdout_fd = -1; /* have been invalidated by rearrange_stdio() */
 
                 /* Allow overriding the homework path via an environment variable, to make debugging
                  * easier. */

diff --git a/src/import/import-common.c b/src/import/import-common.c
index 247f49a855aec19b4b939d83e1ff0394a85582a4..c36105221f6cf0a39c33d63f0dcf6451b845af16 100644 (file)
--- a/src/import/import-common.c
+++ b/src/import/import-common.c
@@ -65,7 +65,7 @@ int import_fork_tar_x(const char *path, pid_t *ret) {
 
                 pipefd[1] = safe_close(pipefd[1]);
 
-                r = rearrange_stdio(pipefd[0], -1, STDERR_FILENO);
+                r = rearrange_stdio(TAKE_FD(pipefd[0]), -1, STDERR_FILENO);
                 if (r < 0) {
                         log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
                         _exit(EXIT_FAILURE);
@@ -131,7 +131,7 @@ int import_fork_tar_c(const char *path, pid_t *ret) {
 
                 pipefd[0] = safe_close(pipefd[0]);
 
-                r = rearrange_stdio(-1, pipefd[1], STDERR_FILENO);
+                r = rearrange_stdio(-1, TAKE_FD(pipefd[1]), STDERR_FILENO);
                 if (r < 0) {
                         log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/import/importd.c b/src/import/importd.c
index 3e2d8427cb036080ab38d25f6545b4ec0484b8bd..0400d41b14727d4d31f820402b2e3b3c71341c65 100644 (file)
--- a/src/import/importd.c
+++ b/src/import/importd.c
@@ -389,9 +389,10 @@ static int transfer_start(Transfer *t) {
 
                 pipefd[0] = safe_close(pipefd[0]);
 
-                r = rearrange_stdio(t->stdin_fd,
-                                    t->stdout_fd < 0 ? pipefd[1] : t->stdout_fd,
+                r = rearrange_stdio(TAKE_FD(t->stdin_fd),
+                                    t->stdout_fd < 0 ? pipefd[1] : TAKE_FD(t->stdout_fd),
                                     pipefd[1]);
+                TAKE_FD(pipefd[1]);
                 if (r < 0) {
                         log_error_errno(r, "Failed to set stdin/stdout/stderr: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/import/pull-common.c b/src/import/pull-common.c
index adb366222dae8528218e24765f368edf4096417a..d0d0c85adc5ed17e2800a5d7ea4c1dbdc97449fb 100644 (file)
--- a/src/import/pull-common.c
+++ b/src/import/pull-common.c
@@ -442,7 +442,7 @@ static int verify_gpg(
 
                 gpg_pipe[1] = safe_close(gpg_pipe[1]);
 
-                r = rearrange_stdio(gpg_pipe[0], -1, STDERR_FILENO);
+                r = rearrange_stdio(TAKE_FD(gpg_pipe[0]), -1, STDERR_FILENO);
                 if (r < 0) {
                         log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/journal-remote/journal-remote-main.c b/src/journal-remote/journal-remote-main.c
index 91b28d0410ba005314deae1d4b1755c6edba3350..67f1267d0a6e27d881c1423ad7b3d0762b30d84c 100644 (file)
--- a/src/journal-remote/journal-remote-main.c
+++ b/src/journal-remote/journal-remote-main.c
@@ -83,9 +83,9 @@ static int spawn_child(const char* child, char** argv) {
 
         /* In the child */
         if (r == 0) {
-                safe_close(fd[0]);
+                fd[0] = safe_close(fd[0]);
 
-                r = rearrange_stdio(STDIN_FILENO, fd[1], STDERR_FILENO);
+                r = rearrange_stdio(STDIN_FILENO, TAKE_FD(fd[1]), STDERR_FILENO);
                 if (r < 0) {
                         log_error_errno(r, "Failed to dup pipe to stdout: %m");
                         _exit(EXIT_FAILURE);

diff --git a/src/libsystemd/sd-bus/bus-socket.c b/src/libsystemd/sd-bus/bus-socket.c
index 05c89f61bf091e70e7873b4bcd3ddcc43bba4492..9c448e36394536adc914eba5de7662f3fcb07097 100644 (file)
--- a/src/libsystemd/sd-bus/bus-socket.c
+++ b/src/libsystemd/sd-bus/bus-socket.c
@@ -988,7 +988,9 @@ int bus_socket_exec(sd_bus *b) {
         if (r == 0) {
                 /* Child */
 
-                if (rearrange_stdio(s[1], s[1], STDERR_FILENO) < 0)
+                r = rearrange_stdio(s[1], s[1], STDERR_FILENO);
+                TAKE_FD(s[1]);
+                if (r < 0)
                         _exit(EXIT_FAILURE);
 
                 (void) rlimit_nofile_safe();

diff --git a/src/nspawn/nspawn-setuid.c b/src/nspawn/nspawn-setuid.c
index 2639b70934c8a9659cf60c343a0438097410d442..34758d8b0fa9324c48b638c41ffcacf67c120bbc 100644 (file)
--- a/src/nspawn/nspawn-setuid.c
+++ b/src/nspawn/nspawn-setuid.c
@@ -38,9 +38,9 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) {
         if (r == 0) {
                 char *empty_env = NULL;
 
-                safe_close(pipe_fds[0]);
+                pipe_fds[0] = safe_close(pipe_fds[0]);
 
-                if (rearrange_stdio(-1, pipe_fds[1], -1) < 0)
+                if (rearrange_stdio(-1, TAKE_FD(pipe_fds[1]), -1) < 0)
                         _exit(EXIT_FAILURE);
 
                 (void) close_all_fds(NULL, 0);

diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c
index f0a9ea9e9fae5cbe5601edf9ef73ff7a3bbac4aa..b93de9c9227d04b7b7674adb2e2f1cd38baff47e 100644 (file)
--- a/src/shared/exec-util.c
+++ b/src/shared/exec-util.c
@@ -50,7 +50,7 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid, b
                 char *_argv[2];
 
                 if (stdout_fd >= 0) {
-                        r = rearrange_stdio(STDIN_FILENO, stdout_fd, STDERR_FILENO);
+                        r = rearrange_stdio(STDIN_FILENO, TAKE_FD(stdout_fd), STDERR_FILENO);
                         if (r < 0)
                                 _exit(EXIT_FAILURE);
                 }

diff --git a/src/test/test-execute.c b/src/test/test-execute.c
index b9c1dc72c2393c1298f56b9c597f769dd306769f..c81842a0d0187db638a69c3d177db0658885440a 100644 (file)
--- a/src/test/test-execute.c
+++ b/src/test/test-execute.c
@@ -579,7 +579,7 @@ static int find_libraries(const char *exec, char ***ret) {
         r = safe_fork("(spawn-ldd)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
         assert_se(r >= 0);
         if (r == 0) {
-                if (rearrange_stdio(-1, outpipe[1], errpipe[1]) < 0)
+                if (rearrange_stdio(-1, TAKE_FD(outpipe[1]), TAKE_FD(errpipe[1])) < 0)
                         _exit(EXIT_FAILURE);
 
                 (void) close_all_fds(NULL, 0);

diff --git a/src/udev/udev-event.c b/src/udev/udev-event.c
index 1c666d01231632fcc029299a424e3eca9c2ea028..837cef083875b2222914ed3a4bcd3a276abca6fd 100644 (file)
--- a/src/udev/udev-event.c
+++ b/src/udev/udev-event.c
@@ -783,7 +783,7 @@ int udev_event_spawn(UdevEvent *event,
                 return log_device_error_errno(event->dev, r,
                                               "Failed to fork() to execute command '%s': %m", cmd);
         if (r == 0) {
-                if (rearrange_stdio(-1, outpipe[WRITE_END], errpipe[WRITE_END]) < 0)
+                if (rearrange_stdio(-1, TAKE_FD(outpipe[WRITE_END]), TAKE_FD(errpipe[WRITE_END])) < 0)
                         _exit(EXIT_FAILURE);
 
                 (void) close_all_fds(NULL, 0);