On 10/29/21 4:08 PM, Nick Desaulniers 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.
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?
Right. This is another concern I have that the tests will return a different values and the wrapper will interpret them as failures.
So his doesn't look like the right change to fix the problem.
thanks, -- Shuah