Re: [PATCH bpf-next] selftests/bpf: test_progs use another shell exit on non-actions

[Jesper Dangaard Brouer <brouer@redhat.com>]

On Thu, 2 Jul 2020 14:24:14 -0700
Yonghong Song <yhs@fb.com> wrote:

> On 7/2/20 10:59 AM, Jesper Dangaard Brouer wrote:
> > This is a follow up adjustment to commit 6c92bd5cd465 ("selftests/bpf:
> > Test_progs indicate to shell on non-actions"), that returns shell exit
> > indication EXIT_FAILURE (value 1) when user selects a non-existing test.
> > 
> > The problem with using EXIT_FAILURE is that a shell script cannot tell
> > the difference between a non-existing test and the test failing.
> > 
> > This patch uses value 2 as shell exit indication.
> > (Aside note unrecognized option parameters use value 64).
> > 
> > Fixes: 6c92bd5cd465 ("selftests/bpf: Test_progs indicate to shell on non-actions")
> > Signed-off-by: Jesper Dangaard Brouer <brouer@redhat.com>
> > ---
> >   tools/testing/selftests/bpf/test_progs.c |    4 +++-
> >   1 file changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/testing/selftests/bpf/test_progs.c b/tools/testing/selftests/bpf/test_progs.c
> > index 104e833d0087..e8f7cd5dbae4 100644
> > --- a/tools/testing/selftests/bpf/test_progs.c
> > +++ b/tools/testing/selftests/bpf/test_progs.c
> > @@ -12,6 +12,8 @@
> >   #include <string.h>
> >   #include <execinfo.h> /* backtrace */
> >   
> > +#define EXIT_NO_TEST 2  
>
> How do you ensure this won't collide with other exit code
> from other library functions (e.g., error code 64 is used
> for unrecognized option which I have no idea what 64 means)?

I expect 64 comes from: /usr/include/sysexits.h
 #define EX_USAGE        64      /* command line usage error */


> Maybe -2 for the exit code?

No. The process's exit status must be a number between 0 and 255, as
defined in man exit(3). (run: 'man 3 exit' as there are many manpages
named exit).

But don't use above 127, because that is usually used for indicating
signals.  E.g. 139 means 11=SIGSEGV $((139 & 127))=11.  POSIX defines
in man wait(3p) check WIFSIGNALED(STATUS) and WTERMSIG(139)=11.
(Hint: cmd 'kill -l' list signals and their numbers).

I bring up Segmentation fault explicitly, as we are seeing these happen
with different tests (that are part of test_progs).  CI people writing
these shell-scripts could pickup these hints and report them, if that
makes sense.


> test_progs already uses -1.

Well that is a bug then.  This will be seen by the shell (parent
process) as 255.

 
> > +
> >   /* defined in test_progs.h */
> >   struct test_env env = {};
> >   
> > @@ -740,7 +742,7 @@ int main(int argc, char **argv)
> >   	close(env.saved_netns_fd);
> >   
> >   	if (env.succ_cnt + env.fail_cnt + env.skip_cnt == 0)
> > -		return EXIT_FAILURE;
> > +		return EXIT_NO_TEST;
> >   
> >   	return env.fail_cnt ? EXIT_FAILURE : EXIT_SUCCESS;
> >   }
> > 

-- 
Best regards,
  Jesper Dangaard Brouer
  MSc.CS, Principal Kernel Engineer at Red Hat
  LinkedIn: http://www.linkedin.com/in/brouer