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.

src/basic/fd-util.h

@@ -91,9 +91,10 @@ static inline int make_null_stdio(void) {
 /* Like TAKE_PTR() but for file descriptors, resetting them to -1 */
 #define TAKE_FD(fd)                             \
         ({                                      \
-                int _fd_ = (fd);                \
+                int *_fd_ = &(fd);              \
-                (fd) = -1;                      \
+                int _ret_ = *_fd_;              \
-                _fd_;                           \
+                *_fd_ = -1;                     \
+                _ret_;                          \
         })
 
 /* Like free_and_replace(), but for file descriptors */

src/basic/limits-util.c

@@ -111,35 +111,57 @@ uint64_t physical_memory_scale(uint64_t v, uint64_t max) {
 }
 
 uint64_t system_tasks_max(void) {
-        uint64_t a = TASKS_MAX, b = TASKS_MAX;
+        uint64_t a = TASKS_MAX, b = TASKS_MAX, c = TASKS_MAX;
         _cleanup_free_ char *root = NULL;
         int r;
 
-        /* Determine the maximum number of tasks that may run on this system. We check three sources to determine this
+        /* Determine the maximum number of tasks that may run on this system. We check three sources to
-         * limit:
+         * determine this limit:
          *
-         * a) the maximum tasks value the kernel allows on this architecture
+         * a) kernel.threads-max sysctl: the maximum number of tasks (threads) the kernel allows.
-         * b) the cgroups pids_max attribute for the system
-         * c) the kernel's configured maximum PID value
          *
-         * And then pick the smallest of the three */
+         *    This puts a direct limit on the number of concurrent tasks.
+         *
+         * b) kernel.pid_max sysctl: the maximum PID value.
+         *
+         *    This limits the numeric range PIDs can take, and thus indirectly also limits the number of
+         *    concurrent threads. It's primarily a compatibility concept: some crappy old code used a signed
+         *    16bit type for PIDs, hence the kernel provides a way to ensure the PIDs never go beyond
+         *    INT16_MAX by default.
+         *
+         *    Also note the weird definition: PIDs assigned will be kept below this value, which means
+         *    the number of tasks that can be created is one lower, as PID 0 is not a valid process ID.
+         *
+         * c) pids.max on the root cgroup: the kernel's configured maximum number of tasks.
+         *
+         * and then pick the smallest of the three.
+         *
+         * By default pid_max is set to much lower values than threads-max, hence the limit people come into
+         * contact with first, as it's the lowest boundary they need to bump when they want higher number of
+         * processes.
+         */
 
-        r = procfs_tasks_get_limit(&a);
+        r = procfs_get_threads_max(&a);
         if (r < 0)
-                log_debug_errno(r, "Failed to read maximum number of tasks from /proc, ignoring: %m");
+                log_debug_errno(r, "Failed to read kernel.threads-max, ignoring: %m");
+
+        r = procfs_get_pid_max(&b);
+        if (r < 0)
+                log_debug_errno(r, "Failed to read kernel.pid_max, ignoring: %m");
+        else if (b > 0)
+                /* Subtract one from pid_max, since PID 0 is not a valid PID */
+                b--;
 
         r = cg_get_root_path(&root);
         if (r < 0)
                 log_debug_errno(r, "Failed to determine cgroup root path, ignoring: %m");
         else {
-                r = cg_get_attribute_as_uint64("pids", root, "pids.max", &b);
+                r = cg_get_attribute_as_uint64("pids", root, "pids.max", &c);
                 if (r < 0)
-                        log_debug_errno(r, "Failed to read pids.max attribute of cgroup root, ignoring: %m");
+                        log_debug_errno(r, "Failed to read pids.max attribute of root cgroup, ignoring: %m");
         }
 
-        return MIN3(TASKS_MAX,
+        return MIN3(a, b, c);
-                    a <= 0 ? TASKS_MAX : a,
-                    b <= 0 ? TASKS_MAX : b);
 }
 
 uint64_t system_tasks_max_scale(uint64_t v, uint64_t max) {

src/basic/process-util.c

@@ -857,7 +857,7 @@ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout) {
 void sigkill_wait(pid_t pid) {
         assert(pid > 1);
 
-        if (kill(pid, SIGKILL) >= 0)
+        (void) kill(pid, SIGKILL);
         (void) wait_for_terminate(pid, NULL);
 }
 
@@ -875,7 +875,7 @@ void sigkill_waitp(pid_t *pid) {
 void sigterm_wait(pid_t pid) {
         assert(pid > 1);
 
-        if (kill_and_sigcont(pid, SIGTERM) >= 0)
+        (void) kill_and_sigcont(pid, SIGTERM);
         (void) wait_for_terminate(pid, NULL);
 }
 

src/basic/process-util.h

@@ -192,8 +192,9 @@ assert_cc(TASKS_MAX <= (unsigned long) PID_T_MAX);
 /* Like TAKE_PTR() but for child PIDs, resetting them to 0 */
 #define TAKE_PID(pid)                           \
         ({                                      \
-                pid_t _pid_ = (pid);            \
+                pid_t *_ppid_ = &(pid);         \
-                (pid) = 0;                      \
+                pid_t _pid_ = *_ppid_;          \
+                *_ppid_ = 0;                    \
                 _pid_;                          \
         })
 

src/basic/procfs-util.c

@@ -13,54 +13,34 @@
 #include "stdio-util.h"
 #include "string-util.h"
 
-int procfs_tasks_get_limit(uint64_t *ret) {
+int procfs_get_pid_max(uint64_t *ret) {
         _cleanup_free_ char *value = NULL;
-        uint64_t pid_max, threads_max;
         int r;
 
         assert(ret);
 
-        /* So there are two sysctl files that control the system limit of processes:
-         *
-         * 1. kernel.threads-max: this is probably the sysctl that makes more sense, as it directly puts a limit on
-         *    concurrent tasks.
-         *
-         * 2. kernel.pid_max: this limits the numeric range PIDs can take, and thus indirectly also limits the number
-         *    of concurrent threads. AFAICS it's primarily a compatibility concept: some crappy old code used a signed
-         *    16bit type for PIDs, hence the kernel provides a way to ensure the PIDs never go beyond INT16_MAX by
-         *    default.
-         *
-         * By default #2 is set to much lower values than #1, hence the limit people come into contact with first, as
-         * it's the lowest boundary they need to bump when they want higher number of processes.
-         *
-         * Also note the weird definition of #2: PIDs assigned will be kept below this value, which means the number of
-         * tasks that can be created is one lower, as PID 0 is not a valid process ID. */
-
         r = read_one_line_file("/proc/sys/kernel/pid_max", &value);
         if (r < 0)
                 return r;
 
-        r = safe_atou64(value, &pid_max);
+        return safe_atou64(value, ret);
-        if (r < 0)
+}
-                return r;
+
+int procfs_get_threads_max(uint64_t *ret) {
+        _cleanup_free_ char *value = NULL;
+        int r;
+
+        assert(ret);
 
-        value = mfree(value);
         r = read_one_line_file("/proc/sys/kernel/threads-max", &value);
         if (r < 0)
                 return r;
 
-        r = safe_atou64(value, &threads_max);
+        return safe_atou64(value, ret);
-        if (r < 0)
-                return r;
-
-        /* Subtract one from pid_max, since PID 0 is not a valid PID */
-        *ret = MIN(pid_max-1, threads_max);
-        return 0;
 }
 
 int procfs_tasks_set_limit(uint64_t limit) {
         char buffer[DECIMAL_STR_MAX(uint64_t)+1];
-        _cleanup_free_ char *value = NULL;
         uint64_t pid_max;
         int r;
 
@@ -75,10 +55,7 @@ int procfs_tasks_set_limit(uint64_t limit) {
          * set it to the maximum. */
         limit = CLAMP(limit, 20U, TASKS_MAX);
 
-        r = read_one_line_file("/proc/sys/kernel/pid_max", &value);
+        r = procfs_get_pid_max(&pid_max);
-        if (r < 0)
-                return r;
-        r = safe_atou64(value, &pid_max);
         if (r < 0)
                 return r;
 
@@ -99,14 +76,10 @@ int procfs_tasks_set_limit(uint64_t limit) {
                 /* Hmm, we couldn't write this? If so, maybe it was already set properly? In that case let's not
                  * generate an error */
 
-                value = mfree(value);
+                if (procfs_get_threads_max(&threads_max) < 0)
-                if (read_one_line_file("/proc/sys/kernel/threads-max", &value) < 0)
                         return r; /* return original error */
 
-                if (safe_atou64(value, &threads_max) < 0)
+                if (MIN(pid_max - 1, threads_max) != limit)
-                        return r; /* return original error */
-
-                if (MIN(pid_max-1, threads_max) != limit)
                         return r; /* return original error */
 
                 /* Yay! Value set already matches what we were trying to set, hence consider this a success. */

src/basic/procfs-util.h

@@ -5,7 +5,9 @@
 
 #include "time-util.h"
 
-int procfs_tasks_get_limit(uint64_t *ret);
+int procfs_get_pid_max(uint64_t *ret);
+int procfs_get_threads_max(uint64_t *ret);
+
 int procfs_tasks_set_limit(uint64_t limit);
 int procfs_tasks_get_current(uint64_t *ret);
 

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(ret_saved_stdin);
-        assert(_saved_stdout);
+        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);
+        r = rearrange_stdio(fd, fd, STDERR_FILENO); /* Invalidates 'fd' also on failure */
-        fd = -1;
+        TAKE_FD(fd);
         if (r < 0)
                 return r;
 
-        *_saved_stdin = saved_stdin;
+        *ret_saved_stdin = TAKE_FD(saved_stdin);
-        *_saved_stdout = saved_stdout;
+        *ret_saved_stdout = TAKE_FD(saved_stdout);
-
-        saved_stdin = saved_stdout = -1;
-
         return 0;
 }
 
@@ -2248,8 +2249,7 @@ static int setup_private_users(uid_t ouid, gid_t ogid, uid_t uid, gid_t gid) {
         if (n != 0) /* on success we should have read 0 bytes */
                 return -EIO;
 
-        r = wait_for_terminate_and_check("(sd-userns)", pid, 0);
+        r = wait_for_terminate_and_check("(sd-userns)", TAKE_PID(pid), 0);
-        pid = 0;
         if (r < 0)
                 return r;
         if (r != EXIT_SUCCESS) /* If something strange happened with the child, let's consider this fatal, too */

src/core/mount.c

@@ -236,8 +236,7 @@ static void mount_unwatch_control_pid(Mount *m) {
         if (m->control_pid <= 0)
                 return;
 
-        unit_unwatch_pid(UNIT(m), m->control_pid);
+        unit_unwatch_pid(UNIT(m), TAKE_PID(m->control_pid));
-        m->control_pid = 0;
 }
 
 static void mount_parameters_done(MountParameters *p) {

src/core/service.c

@@ -130,8 +130,7 @@ static void service_unwatch_control_pid(Service *s) {
         if (s->control_pid <= 0)
                 return;
 
-        unit_unwatch_pid(UNIT(s), s->control_pid);
+        unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid));
-        s->control_pid = 0;
 }
 
 static void service_unwatch_main_pid(Service *s) {
@@ -140,8 +139,7 @@ static void service_unwatch_main_pid(Service *s) {
         if (s->main_pid <= 0)
                 return;
 
-        unit_unwatch_pid(UNIT(s), s->main_pid);
+        unit_unwatch_pid(UNIT(s), TAKE_PID(s->main_pid));
-        s->main_pid = 0;
 }
 
 static void service_unwatch_pid_file(Service *s) {

src/core/socket.c

@@ -109,8 +109,7 @@ static void socket_unwatch_control_pid(Socket *s) {
         if (s->control_pid <= 0)
                 return;
 
-        unit_unwatch_pid(UNIT(s), s->control_pid);
+        unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid));
-        s->control_pid = 0;
 }
 
 static void socket_cleanup_fd_list(SocketPort *p) {

src/core/swap.c

@@ -155,8 +155,7 @@ static void swap_unwatch_control_pid(Swap *s) {
         if (s->control_pid <= 0)
                 return;
 
-        unit_unwatch_pid(UNIT(s), s->control_pid);
+        unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid));
-        s->control_pid = 0;
 }
 
 static void swap_done(Unit *u) {

src/fundamental/macro-fundamental.h

@@ -257,8 +257,9 @@
  * resets it to NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */
 #define TAKE_PTR(ptr)                           \
         ({                                      \
-                typeof(ptr) _ptr_ = (ptr);      \
+                typeof(ptr) *_pptr_ = &(ptr);   \
-                (ptr) = NULL;                   \
+                typeof(ptr) _ptr_ = *_pptr_;    \
+                *_pptr_ = NULL;                 \
                 _ptr_;                          \
         })
 

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. */

src/import/export-tar.c

@@ -56,10 +56,8 @@ TarExport *tar_export_unref(TarExport *e) {
 
         sd_event_source_unref(e->output_event_source);
 
-        if (e->tar_pid > 1) {
+        if (e->tar_pid > 1)
-                (void) kill_and_sigcont(e->tar_pid, SIGKILL);
+                sigkill_wait(e->tar_pid);
-                (void) wait_for_terminate(e->tar_pid, NULL);
-        }
 
         if (e->temp_path) {
                 (void) btrfs_subvol_remove(e->temp_path, BTRFS_REMOVE_QUOTA);
@@ -147,8 +145,7 @@ static int tar_export_finish(TarExport *e) {
         assert(e->tar_fd >= 0);
 
         if (e->tar_pid > 0) {
-                r = wait_for_terminate_and_check("tar", e->tar_pid, WAIT_LOG);
+                r = wait_for_terminate_and_check("tar", TAKE_PID(e->tar_pid), WAIT_LOG);
-                e->tar_pid = 0;
                 if (r < 0)
                         return r;
                 if (r != EXIT_SUCCESS)

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);

src/import/import-tar.c

@@ -69,10 +69,8 @@ TarImport* tar_import_unref(TarImport *i) {
 
         sd_event_source_unref(i->input_event_source);
 
-        if (i->tar_pid > 1) {
+        if (i->tar_pid > 1)
-                (void) kill_and_sigcont(i->tar_pid, SIGKILL);
+                sigkill_wait(i->tar_pid);
-                (void) wait_for_terminate(i->tar_pid, NULL);
-        }
 
         rm_rf_subvolume_and_free(i->temp_path);
 
@@ -167,8 +165,7 @@ static int tar_import_finish(TarImport *i) {
         i->tar_fd = safe_close(i->tar_fd);
 
         if (i->tar_pid > 0) {
-                r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG);
+                r = wait_for_terminate_and_check("tar", TAKE_PID(i->tar_pid), WAIT_LOG);
-                i->tar_pid = 0;
                 if (r < 0)
                         return r;
                 if (r != EXIT_SUCCESS)

src/import/importd.c

@@ -126,10 +126,8 @@ static Transfer *transfer_unref(Transfer *t) {
         free(t->format);
         free(t->object_path);
 
-        if (t->pid > 0) {
+        if (t->pid > 1)
-                (void) kill_and_sigcont(t->pid, SIGKILL);
+                sigkill_wait(t->pid);
-                (void) wait_for_terminate(t->pid, NULL);
-        }
 
         safe_close(t->log_fd);
         safe_close(t->stdin_fd);
@@ -391,9 +389,10 @@ static int transfer_start(Transfer *t) {
 
                 pipefd[0] = safe_close(pipefd[0]);
 
-                r = rearrange_stdio(t->stdin_fd,
+                r = rearrange_stdio(TAKE_FD(t->stdin_fd),
-                                    t->stdout_fd < 0 ? pipefd[1] : t->stdout_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);

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);
@@ -482,8 +482,7 @@ static int verify_gpg(
 
         gpg_pipe[1] = safe_close(gpg_pipe[1]);
 
-        r = wait_for_terminate_and_check("gpg", pid, WAIT_LOG_ABNORMAL);
+        r = wait_for_terminate_and_check("gpg", TAKE_PID(pid), WAIT_LOG_ABNORMAL);
-        pid = 0;
         if (r < 0)
                 goto finish;
         if (r != EXIT_SUCCESS)

src/import/pull-tar.c

@@ -71,10 +71,8 @@ TarPull* tar_pull_unref(TarPull *i) {
         if (!i)
                 return NULL;
 
-        if (i->tar_pid > 1) {
+        if (i->tar_pid > 1)
-                (void) kill_and_sigcont(i->tar_pid, SIGKILL);
+                sigkill_wait(i->tar_pid);
-                (void) wait_for_terminate(i->tar_pid, NULL);
-        }
 
         pull_job_unref(i->tar_job);
         pull_job_unref(i->checksum_job);
@@ -369,8 +367,7 @@ static void tar_pull_job_on_finished(PullJob *j) {
         pull_job_close_disk_fd(i->settings_job);
 
         if (i->tar_pid > 0) {
-                r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG);
+                r = wait_for_terminate_and_check("tar", TAKE_PID(i->tar_pid), WAIT_LOG);
-                i->tar_pid = 0;
                 if (r < 0)
                         goto finish;
                 if (r != EXIT_SUCCESS) {

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);

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();

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

@@ -1082,10 +1082,10 @@ static int bus_parse_next_address(sd_bus *b) {
 }
 
 static void bus_kill_exec(sd_bus *bus) {
-        if (pid_is_valid(bus->busexec_pid) > 0) {
+        if (!pid_is_valid(bus->busexec_pid))
-                sigterm_wait(bus->busexec_pid);
+                return;
-                bus->busexec_pid = 0;
+
-        }
+        sigterm_wait(TAKE_PID(bus->busexec_pid));
 }
 
 static int bus_start_address(sd_bus *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);

src/nspawn/nspawn.c

@@ -5248,8 +5248,7 @@ static int run_container(
                 }
         }
 
-        r = wait_for_container(*pid, &container_status);
+        r = wait_for_container(TAKE_PID(*pid), &container_status);
-        *pid = 0;
 
         /* Tell machined that we are gone. */
         if (bus)

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);
                 }

src/shared/pager.c

@@ -286,7 +286,7 @@ void pager_close(void) {
         stdout_redirected = stderr_redirected = false;
 
         (void) kill(pager_pid, SIGCONT);
-        (void) wait_for_terminate(pager_pid, NULL);
+        (void) wait_for_terminate(TAKE_PID(pager_pid), NULL);
         pager_pid = 0;
 }
 

src/shared/spawn-ask-password-agent.c

@@ -43,9 +43,7 @@ void ask_password_agent_close(void) {
                 return;
 
         /* Inform agent that we are done */
-        (void) kill_and_sigcont(agent_pid, SIGTERM);
+        sigterm_wait(TAKE_PID(agent_pid));
-        (void) wait_for_terminate(agent_pid, NULL);
-        agent_pid = 0;
 }
 
 int ask_password_agent_open_if_enabled(BusTransport transport, bool ask_password) {

src/shared/spawn-polkit-agent.c

@@ -69,9 +69,7 @@ void polkit_agent_close(void) {
                 return;
 
         /* Inform agent that we are done */
-        (void) kill_and_sigcont(agent_pid, SIGTERM);
+        sigterm_wait(TAKE_PID(agent_pid));
-        (void) wait_for_terminate(agent_pid, NULL);
-        agent_pid = 0;
 }
 
 #else

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);

src/test/test-fd-util.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: LGPL-2.1-or-later */
 
 #include <fcntl.h>
+#include <sys/eventfd.h>
 #include <unistd.h>
 
 #include "alloc-util.h"
@@ -486,6 +487,47 @@ static void test_fd_reopen(void) {
         fd1 = -1;
 }
 
+static void test_take_fd(void) {
+        _cleanup_close_ int fd1 = -1, fd2 = -1;
+        int array[2] = { -1, -1 }, i = 0;
+
+        log_info("/* %s */", __func__);
+
+        assert_se(fd1 == -1);
+        assert_se(fd2 == -1);
+
+        fd1 = eventfd(0, EFD_CLOEXEC);
+        assert_se(fd1 >= 0);
+
+        fd2 = TAKE_FD(fd1);
+        assert_se(fd1 == -1);
+        assert_se(fd2 >= 0);
+
+        assert_se(array[0] == -1);
+        assert_se(array[1] == -1);
+
+        array[0] = TAKE_FD(fd2);
+        assert_se(fd1 == -1);
+        assert_se(fd2 == -1);
+        assert_se(array[0] >= 0);
+        assert_se(array[1] == -1);
+
+        array[1] = TAKE_FD(array[i]);
+        assert_se(array[0] == -1);
+        assert_se(array[1] >= 0);
+
+        i = 1 - i;
+        array[0] = TAKE_FD(*(array + i));
+        assert_se(array[0] >= 0);
+        assert_se(array[1] == -1);
+
+        i = 1 - i;
+        fd1 = TAKE_FD(array[i]);
+        assert_se(fd1 >= 0);
+        assert_se(array[0] == -1);
+        assert_se(array[1] == -1);
+}
+
 int main(int argc, char *argv[]) {
 
         test_setup_logging(LOG_DEBUG);
@@ -500,6 +542,7 @@ int main(int argc, char *argv[]) {
         test_close_all_fds();
         test_format_proc_fd_path();
         test_fd_reopen();
+        test_take_fd();
 
         return 0;
 }

src/test/test-process-util.c

@@ -851,8 +851,14 @@ static void test_get_process_ppid(void) {
         assert_se(get_process_ppid(1, NULL) == -EADDRNOTAVAIL);
 
         /* the process with the PID above the global limit definitely doesn't exist. Verify that */
-        assert_se(procfs_tasks_get_limit(&limit) >= 0);
+        assert_se(procfs_get_pid_max(&limit) >= 0);
-        assert_se(limit >= INT_MAX || get_process_ppid(limit+1, NULL) == -ESRCH);
+        log_debug("kernel.pid_max = %"PRIu64, limit);
+
+        if (limit < INT_MAX) {
+                r = get_process_ppid(limit + 1, NULL);
+                log_debug_errno(r, "get_process_limit(%"PRIu64") → %d/%m", limit + 1, r);
+                assert(r == -ESRCH);
+        }
 
         for (pid_t pid = 0;;) {
                 _cleanup_free_ char *c1 = NULL, *c2 = NULL;

src/test/test-procfs-util.c

@@ -6,11 +6,12 @@
 #include "format-util.h"
 #include "log.h"
 #include "procfs-util.h"
+#include "process-util.h"
 #include "tests.h"
 
 int main(int argc, char *argv[]) {
         nsec_t nsec;
-        uint64_t v;
+        uint64_t v, w;
         int r;
 
         log_parse_environment();
@@ -25,26 +26,39 @@ int main(int argc, char *argv[]) {
         assert_se(procfs_tasks_get_current(&v) >= 0);
         log_info("Current number of tasks: %" PRIu64, v);
 
-        r = procfs_tasks_get_limit(&v);
+        v = TASKS_MAX;
-        if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
+        r = procfs_get_pid_max(&v);
-                return log_tests_skipped("can't read /proc/sys/kernel/pid_max");
+        assert(r >= 0 || r == -ENOENT || ERRNO_IS_PRIVILEGE(r));
+        log_info("kernel.pid_max: %"PRIu64, v);
+
+        w = TASKS_MAX;
+        r = procfs_get_threads_max(&w);
+        assert(r >= 0 || r == -ENOENT || ERRNO_IS_PRIVILEGE(r));
+        log_info("kernel.threads-max: %"PRIu64, w);
+
+        v = MIN(v - (v > 0), w);
 
         assert_se(r >= 0);
         log_info("Limit of tasks: %" PRIu64, v);
         assert_se(v > 0);
-        assert_se(procfs_tasks_set_limit(v) >= 0);
+        r = procfs_tasks_set_limit(v);
+        if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
+                return log_tests_skipped("can't set task limits");
+        assert(r >= 0);
 
         if (v > 100) {
-                uint64_t w;
+                log_info("Reducing limit by one to %"PRIu64"…", v-1);
-                r = procfs_tasks_set_limit(v-1);
-                assert_se(IN_SET(r, 0, -EPERM, -EACCES, -EROFS));
 
-                assert_se(procfs_tasks_get_limit(&w) >= 0);
+                r = procfs_tasks_set_limit(v-1);
-                assert_se((r == 0 && w == v - 1) || (r < 0 && w == v));
+                log_info_errno(r, "procfs_tasks_set_limit: %m");
+                assert_se(r >= 0 || ERRNO_IS_PRIVILEGE(r) || r == -EROFS);
+
+                assert_se(procfs_get_threads_max(&w) >= 0);
+                assert_se(r >= 0 ? w == v - 1 : w == v);
 
                 assert_se(procfs_tasks_set_limit(v) >= 0);
 
-                assert_se(procfs_tasks_get_limit(&w) >= 0);
+                assert_se(procfs_get_threads_max(&w) >= 0);
                 assert_se(v == w);
         }
 

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);