On Wed, Nov 03, 2021 at 10:38:17AM +0100, Anders Roxell wrote:
On Tue, 2 Nov 2021 at 23:04, Anders Roxell anders.roxell@linaro.org wrote:
On Sat, 30 Oct 2021 at 00:08, Nick Desaulniers ndesaulniers@google.com wrote:
On Fri, Oct 29, 2021 at 11:19 AM Shuah Khan skhan@linuxfoundation.org wrote:
On 10/29/21 5:43 AM, Anders Roxell wrote:
When building kselftests/capabilities the following warning shows up:
clang -O2 -g -std=gnu99 -Wall test_execve.c -lcap-ng -lrt -ldl -o test_execve test_execve.c:121:13: warning: variable 'have_outer_privilege' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized] } else if (unshare(CLONE_NEWUSER | CLONE_NEWNS) == 0) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test_execve.c:136:9: note: uninitialized use occurs here return have_outer_privilege; ^~~~~~~~~~~~~~~~~~~~ test_execve.c:121:9: note: remove the 'if' if its condition is always true } else if (unshare(CLONE_NEWUSER | CLONE_NEWNS) == 0) { ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ test_execve.c:94:27: note: initialize the variable 'have_outer_privilege' to silence this warning bool have_outer_privilege; ^ = false
Rework so all the ksft_exit_*() functions have attribue '__attribute__((noreturn))' so the compiler knows that there wont be any return from the function. That said, without '__attribute__((noreturn))' the compiler warns about the above issue since it thinks that it will get back from the ksft_exit_skip() function, which it wont. Cleaning up the callers that rely on ksft_exit_*() return code, since the functions ksft_exit_*() have never returned anything.
Signed-off-by: Anders Roxell anders.roxell@linaro.org
Lot of changes to fix this warning. Is this necessary? I would like to explore if there is an easier and localized change that can fix the problem.
via `man 3 exit`:
The exit() function causes normal process termination ... ... RETURN VALUE The exit() function does not return.so seeing `ksft_exit_pass`, `ksft_exit_fail`, `ksft_exit_fail_msg`, `ksft_exit_xfail`, `ksft_exit_xpass`, and `ksft_exit_skip` all unconditional call `exit` yet return an `int` looks wrong to me on first glance. So on that point this patch and its resulting diffstat LGTM.
I'll respin the patch with these changes only.
That said, there are many changes that explicitly call `ksft_exit` with an expression; are those setting the correct exit code? Note that ksft_exit_pass is calling exit with KSFT_PASS which is 0. So some of the negations don't look quite correct to me. For example:
return !ksft_get_fail_cnt() ? ksft_exit_pass() : ksft_exit_fail();
ksft_exit(!ksft_get_fail_cnt());so if ksft_get_fail_cnt() returns 0, then we were calling ksft_exit_pass() which exited with 0. Now we'd be exiting with 1?
oh, right, thank you for your review. I will drop all the 'ksft_exit()' changes, they should be fixed and go in as separete patches.
tools/testing/selftests/vm/memfd_secret.c has the 'ksft_exit(!ksft_get_fail_cnt())' statement and when I looked at it it when I did this patch it looked correct. However, when I look at it now I get a bit confused how ksft_exit() can be used with ksft_get_fail_cnt(). @Mike can you please clarify the 'ksft_exit(!ksft_get_fail_cnt())' instance in tools/testing/selftests/vm/memfd_secret.c.
ksft_exit() does not take the error code but rather a condition:
/** * ksft_exit() - Exit selftest based on truth of condition * * @condition: if true, exit self test with success, otherwise fail. */ #define ksft_exit(condition) do { \ if (!!(condition)) \ ksft_exit_pass(); \ else \ ksft_exit_fail(); \ } while (0)
So
!ksft_get_fail_cnt() ? ksft_exit_pass() : ksft_exit_fail();
and
ksft_exit(!ksft_get_fail_cnt())
are equivalent.