On Wed, Apr 17, 2024 at 08:22:22AM -0700, Nathan Chancellor wrote:

Hi Christian,

I am looking at tools/testing/selftests/clone3/clone3_set_tid.c as part of a patch to clean up the uses of 'return ksft_exit_...();' throughout the selftests (as they call exit() so they do not return) and I noticed that it seems to always pass even when there may have been an error?

  if (waitpid(ns_pid, &status, 0) < 0) {
      ksft_print_msg("Child returned %s\n", strerror(errno));
      ret = -errno;
      goto out;
  }

  ...

out: ret = 0;

  return !ret ? ksft_exit_pass() : ksft_exit_fail();

}

Should the ret and out label positions be switched? Alternatively, it seems like ret and out do not have that many uses, perhaps it would just be better to call the ksft_exit_...() directly in their respective paths? I am not going to touch it as part of my patch but I felt it was worth reporting since it appears to have been there since the introduction of this test in commit 41585bbeeef9 ("selftests: add tests for clone3() with *set_tid").

Uh, good point. Let me Cc the original author.