Messages in this thread | | | Subject | Re: [PATCH] selftests: kselftest.h: mark functions with 'noreturn' | From | Shuah Khan <> | Date | Fri, 29 Oct 2021 16:23:01 -0600 |
| |
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
| |