Re: [PATCH] selftests: kselftest.h: mark functions with 'noreturn'

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On 10/29/21 4:08 PM, Nick Desaulniers wrote:
On Fri, Oct 29, 2021 at 11:19 AM Shuah Khan <skhan@xxxxxxxxxxxxxxxxxxx> 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@xxxxxxxxxx>

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



[Index of Archives]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Share Photos]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Samba]     [Device Mapper]

  Powered by Linux