Skip to content

Commit aedec45

Browse files
poetteringbluca
poettering
authored and
bluca
committed
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.
1 parent 829b86b commit aedec45

File tree

11 files changed

+28
-25
lines changed

11 files changed

+28
-25
lines changed

‎src/core/execute.c‎

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -756,12 +756,16 @@ static int chown_terminal(int fd, uid_t uid) {
756756
return 1;
757757
}
758758

759-
static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_stdout) {
759+
static int setup_confirm_stdio(
760+
const char *vc,
761+
int *ret_saved_stdin,
762+
int *ret_saved_stdout) {
763+
760764
_cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1;
761765
int r;
762766

763-
assert(_saved_stdin);
764-
assert(_saved_stdout);
767+
assert(ret_saved_stdin);
768+
assert(ret_saved_stdout);
765769

766770
saved_stdin = fcntl(STDIN_FILENO, F_DUPFD, 3);
767771
if (saved_stdin < 0)
@@ -783,16 +787,13 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st
783787
if (r < 0)
784788
return r;
785789

786-
r = rearrange_stdio(fd, fd, STDERR_FILENO);
787-
fd = -1;
790+
r = rearrange_stdio(fd, fd, STDERR_FILENO); /* Invalidates 'fd' also on failure */
791+
TAKE_FD(fd);
788792
if (r < 0)
789793
return r;
790794

791-
*_saved_stdin = saved_stdin;
792-
*_saved_stdout = saved_stdout;
793-
794-
saved_stdin = saved_stdout = -1;
795-
795+
*ret_saved_stdin = TAKE_FD(saved_stdin);
796+
*ret_saved_stdout = TAKE_FD(saved_stdout);
796797
return 0;
797798
}
798799

‎src/home/homed-home.c‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1202,13 +1202,12 @@ static int home_start_work(Home *h, const char *verb, UserRecord *hr, UserRecord
12021202
if (r < 0)
12031203
log_warning_errno(r, "Failed to update $SYSTEMD_EXEC_PID, ignoring: %m");
12041204

1205-
r = rearrange_stdio(stdin_fd, stdout_fd, STDERR_FILENO);
1205+
r = rearrange_stdio(TAKE_FD(stdin_fd), TAKE_FD(stdout_fd), STDERR_FILENO); /* fds are invalidated by rearrange_stdio() even on failure */
12061206
if (r < 0) {
12071207
log_error_errno(r, "Failed to rearrange stdin/stdout/stderr: %m");
12081208
_exit(EXIT_FAILURE);
12091209
}
12101210

1211-
stdin_fd = stdout_fd = -1; /* have been invalidated by rearrange_stdio() */
12121211

12131212
/* Allow overriding the homework path via an environment variable, to make debugging
12141213
* easier. */

‎src/import/import-common.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ int import_fork_tar_x(const char *path, pid_t *ret) {
6565

6666
pipefd[1] = safe_close(pipefd[1]);
6767

68-
r = rearrange_stdio(pipefd[0], -1, STDERR_FILENO);
68+
r = rearrange_stdio(TAKE_FD(pipefd[0]), -1, STDERR_FILENO);
6969
if (r < 0) {
7070
log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
7171
_exit(EXIT_FAILURE);
@@ -131,7 +131,7 @@ int import_fork_tar_c(const char *path, pid_t *ret) {
131131

132132
pipefd[0] = safe_close(pipefd[0]);
133133

134-
r = rearrange_stdio(-1, pipefd[1], STDERR_FILENO);
134+
r = rearrange_stdio(-1, TAKE_FD(pipefd[1]), STDERR_FILENO);
135135
if (r < 0) {
136136
log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
137137
_exit(EXIT_FAILURE);

‎src/import/importd.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -389,9 +389,10 @@ static int transfer_start(Transfer *t) {
389389

390390
pipefd[0] = safe_close(pipefd[0]);
391391

392-
r = rearrange_stdio(t->stdin_fd,
393-
t->stdout_fd < 0 ? pipefd[1] : t->stdout_fd,
392+
r = rearrange_stdio(TAKE_FD(t->stdin_fd),
393+
t->stdout_fd < 0 ? pipefd[1] : TAKE_FD(t->stdout_fd),
394394
pipefd[1]);
395+
TAKE_FD(pipefd[1]);
395396
if (r < 0) {
396397
log_error_errno(r, "Failed to set stdin/stdout/stderr: %m");
397398
_exit(EXIT_FAILURE);

‎src/import/pull-common.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -442,7 +442,7 @@ static int verify_gpg(
442442

443443
gpg_pipe[1] = safe_close(gpg_pipe[1]);
444444

445-
r = rearrange_stdio(gpg_pipe[0], -1, STDERR_FILENO);
445+
r = rearrange_stdio(TAKE_FD(gpg_pipe[0]), -1, STDERR_FILENO);
446446
if (r < 0) {
447447
log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
448448
_exit(EXIT_FAILURE);

‎src/journal-remote/journal-remote-main.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,9 +83,9 @@ static int spawn_child(const char* child, char** argv) {
8383

8484
/* In the child */
8585
if (r == 0) {
86-
safe_close(fd[0]);
86+
fd[0] = safe_close(fd[0]);
8787

88-
r = rearrange_stdio(STDIN_FILENO, fd[1], STDERR_FILENO);
88+
r = rearrange_stdio(STDIN_FILENO, TAKE_FD(fd[1]), STDERR_FILENO);
8989
if (r < 0) {
9090
log_error_errno(r, "Failed to dup pipe to stdout: %m");
9191
_exit(EXIT_FAILURE);

‎src/libsystemd/sd-bus/bus-socket.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -988,7 +988,9 @@ int bus_socket_exec(sd_bus *b) {
988988
if (r == 0) {
989989
/* Child */
990990

991-
if (rearrange_stdio(s[1], s[1], STDERR_FILENO) < 0)
991+
r = rearrange_stdio(s[1], s[1], STDERR_FILENO);
992+
TAKE_FD(s[1]);
993+
if (r < 0)
992994
_exit(EXIT_FAILURE);
993995

994996
(void) rlimit_nofile_safe();

‎src/nspawn/nspawn-setuid.c‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -38,9 +38,9 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) {
3838
if (r == 0) {
3939
char *empty_env = NULL;
4040

41-
safe_close(pipe_fds[0]);
41+
pipe_fds[0] = safe_close(pipe_fds[0]);
4242

43-
if (rearrange_stdio(-1, pipe_fds[1], -1) < 0)
43+
if (rearrange_stdio(-1, TAKE_FD(pipe_fds[1]), -1) < 0)
4444
_exit(EXIT_FAILURE);
4545

4646
(void) close_all_fds(NULL, 0);

‎src/shared/exec-util.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid, b
5050
char *_argv[2];
5151

5252
if (stdout_fd >= 0) {
53-
r = rearrange_stdio(STDIN_FILENO, stdout_fd, STDERR_FILENO);
53+
r = rearrange_stdio(STDIN_FILENO, TAKE_FD(stdout_fd), STDERR_FILENO);
5454
if (r < 0)
5555
_exit(EXIT_FAILURE);
5656
}

‎src/test/test-execute.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,7 @@ static int find_libraries(const char *exec, char ***ret) {
579579
r = safe_fork("(spawn-ldd)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
580580
assert_se(r >= 0);
581581
if (r == 0) {
582-
if (rearrange_stdio(-1, outpipe[1], errpipe[1]) < 0)
582+
if (rearrange_stdio(-1, TAKE_FD(outpipe[1]), TAKE_FD(errpipe[1])) < 0)
583583
_exit(EXIT_FAILURE);
584584

585585
(void) close_all_fds(NULL, 0);

0 commit comments

Comments
 (0)