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.
Cheers, Anders