From: Mike Yuan Date: Sat, 20 Dec 2025 17:47:03 +0000 (+0100) Subject: pidref: several follow-ups for pidref_wait_for_terminate_full() X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=f3c6660b9c17d1c779e82ec99384896489e748d8;p=thirdparty%2Fsystemd.git pidref: several follow-ups for pidref_wait_for_terminate_full() Follow-up for e74b571004661ff39fbcbddfe0cbf36d2fda0046 * Do not reuse si * Refuse timeout == 0 * Use usec_add() * Rename ret to ret_si - unlike others this one is not so obvious --- diff --git a/src/basic/pidref.c b/src/basic/pidref.c index de92d2fffbe..3c41e2a41eb 100644 --- a/src/basic/pidref.c +++ b/src/basic/pidref.c @@ -452,10 +452,11 @@ bool pidref_is_self(PidRef *pidref) { return pidref->fd_id == self_id; } -int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *ret) { - siginfo_t si = {}; +int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *ret_si) { int r; + assert(timeout > 0); + if (!pidref_is_set(pidref)) return -ESRCH; @@ -468,10 +469,10 @@ int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *re if (timeout != USEC_INFINITY && pidref->fd < 0) return -ENOMEDIUM; - usec_t ts = timeout == USEC_INFINITY ? USEC_INFINITY : now(CLOCK_MONOTONIC) + timeout; + usec_t ts = timeout == USEC_INFINITY ? USEC_INFINITY : usec_add(now(CLOCK_MONOTONIC), timeout); for (;;) { - if (timeout != USEC_INFINITY) { + if (ts != USEC_INFINITY) { usec_t left = usec_sub_unsigned(ts, now(CLOCK_MONOTONIC)); if (left == 0) return -ETIMEDOUT; @@ -485,22 +486,20 @@ int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *re return r; } + siginfo_t si = {}; + if (pidref->fd >= 0) r = RET_NERRNO(waitid(P_PIDFD, pidref->fd, &si, WEXITED)); else r = RET_NERRNO(waitid(P_PID, pidref->pid, &si, WEXITED)); - if (r == -EINTR) - continue; - if (r < 0) + if (r >= 0) { + if (ret_si) + *ret_si = si; + return 0; + } + if (r != -EINTR) return r; - - break; } - - if (ret) - *ret = si; - - return 0; } bool pidref_is_automatic(const PidRef *pidref) { diff --git a/src/basic/pidref.h b/src/basic/pidref.h index 864acb569df..2f1d95f9971 100644 --- a/src/basic/pidref.h +++ b/src/basic/pidref.h @@ -96,9 +96,9 @@ int pidref_kill(const PidRef *pidref, int sig); int pidref_kill_and_sigcont(const PidRef *pidref, int sig); int pidref_sigqueue(const PidRef *pidref, int sig, int value); -int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *ret); -static inline int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret) { - return pidref_wait_for_terminate_full(pidref, USEC_INFINITY, ret); +int pidref_wait_for_terminate_full(PidRef *pidref, usec_t timeout, siginfo_t *ret_si); +static inline int pidref_wait_for_terminate(PidRef *pidref, siginfo_t *ret_si) { + return pidref_wait_for_terminate_full(pidref, USEC_INFINITY, ret_si); } static inline void pidref_done_sigterm_wait(PidRef *pidref) { diff --git a/src/shutdown/shutdown.c b/src/shutdown/shutdown.c index 3496ad33ea8..151c1654c24 100644 --- a/src/shutdown/shutdown.c +++ b/src/shutdown/shutdown.c @@ -250,7 +250,7 @@ int sync_with_progress(int fd) { * SYNC_PROGRESS_ATTEMPTS lapse without progress being made, * we assume that the sync is stalled */ for (unsigned checks = 0; checks < SYNC_PROGRESS_ATTEMPTS; checks++) { - r = pidref_wait_for_terminate_full(&pidref, SYNC_TIMEOUT_USEC, /* ret= */ NULL); + r = pidref_wait_for_terminate_full(&pidref, SYNC_TIMEOUT_USEC, /* ret_si= */ NULL); if (r == 0) /* Sync finished without error (sync() call itself does not return an error code) */ return 0; diff --git a/src/test/test-async.c b/src/test/test-async.c index e65acfe6110..46156d724dc 100644 --- a/src/test/test-async.c +++ b/src/test/test-async.c @@ -18,7 +18,7 @@ TEST(asynchronous_sync) { _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; ASSERT_OK(asynchronous_sync(&pidref)); - ASSERT_OK(pidref_wait_for_terminate(&pidref, /* ret= */ NULL)); + ASSERT_OK(pidref_wait_for_terminate(&pidref, NULL)); } static void wait_fd_closed(int fd) { diff --git a/src/test/test-namespace.c b/src/test/test-namespace.c index 1c43c7ab10e..4890ecc7295 100644 --- a/src/test/test-namespace.c +++ b/src/test/test-namespace.c @@ -326,7 +326,7 @@ TEST(process_is_owned_by_uid) { ASSERT_OK_ZERO(process_is_owned_by_uid(&pid, getuid())); ASSERT_OK(pidref_kill(&pid, SIGKILL)); - ASSERT_OK(pidref_wait_for_terminate(&pid, /* ret= */ NULL)); + ASSERT_OK(pidref_wait_for_terminate(&pid, NULL)); /* Test a child that runs in a userns as uid 1, but the userns is owned by us */ ASSERT_OK_ERRNO(pipe2(p, O_CLOEXEC)); @@ -373,7 +373,7 @@ TEST(process_is_owned_by_uid) { ASSERT_OK_POSITIVE(process_is_owned_by_uid(&pid, getuid())); ASSERT_OK(pidref_kill(&pid, SIGKILL)); - ASSERT_OK(pidref_wait_for_terminate(&pid, /* ret= */ NULL)); + ASSERT_OK(pidref_wait_for_terminate(&pid, NULL)); } TEST(namespace_get_leader) { diff --git a/src/test/test-pidref.c b/src/test/test-pidref.c index f682b42d46e..eb4bfa223bd 100644 --- a/src/test/test-pidref.c +++ b/src/test/test-pidref.c @@ -243,7 +243,7 @@ TEST(pidref_is_remote) { ASSERT_FALSE(pidref_is_automatic(&p)); ASSERT_ERROR(pidref_kill(&p, SIGTERM), EREMOTE); ASSERT_ERROR(pidref_kill_and_sigcont(&p, SIGTERM), EREMOTE); - ASSERT_ERROR(pidref_wait_for_terminate(&p, /* ret= */ NULL), EREMOTE); + ASSERT_ERROR(pidref_wait_for_terminate(&p, NULL), EREMOTE); ASSERT_ERROR(pidref_verify(&p), EREMOTE); }