pidref: several follow-ups for pidref_wait_for_terminate_full()

Follow-up for e74b571004

* Do not reuse si
* Refuse timeout == 0
* Use usec_add()
* Rename ret to ret_si - unlike others this one is not so obvious

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

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

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;

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

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

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