On Mon, Sep 25, 2023 at 04:37:02PM -0700, Tony Ambardar wrote:
> Current test_verifier provides little feedback or argument validation,
> instead silently falling back to running all tests in case of user error
> or even expected use cases. Trying to do manual exploratory testing,
> switching between kernel versions (e.g. with varying tests), or working
> around problematic tests (e.g. kernel hangs/crashes) can be a frustrating
> experience.
> 
> Rework argument parsing to be more robust and strict, and provide basic
> help on errors. Clamp test ranges to valid values and add an option to
> list available built-in tests ("-l"). Default "test_verifier" behaviour
> (run all tests) is unchanged and backwards-compatible. Updated examples:
> 
>      $ test_verifier die die die     # previously ran all tests
>      Usage: test_verifier -l | [-v|-vv] [<tst_lo> [<tst_hi>]]
> 
>      $ test_verifier 700 9999        # runs test subset from 700 to end
> 
> Signed-off-by: Tony Ambardar <tony.ambar...@gmail.com>
> ---
>  tools/testing/selftests/bpf/test_verifier.c | 54 ++++++++++++---------
>  1 file changed, 30 insertions(+), 24 deletions(-)
> 
> diff --git a/tools/testing/selftests/bpf/test_verifier.c 
> b/tools/testing/selftests/bpf/test_verifier.c
> index 98107e0452d3..3712b5363f60 100644
> --- a/tools/testing/selftests/bpf/test_verifier.c
> +++ b/tools/testing/selftests/bpf/test_verifier.c
> @@ -10,9 +10,11 @@
>  #include <endian.h>
>  #include <asm/types.h>
>  #include <linux/types.h>
> +#include <linux/minmax.h>

this fails to compile

  BINARY   test_verifier
test_verifier.c:13:10: fatal error: linux/minmax.h: No such file or directory
   13 | #include <linux/minmax.h>
      |          ^~~~~~~~~~~~~~~~

looks like you could use perhaps <linux/kernel.h> instead?

jirka


>  #include <stdint.h>
>  #include <stdio.h>
>  #include <stdlib.h>
> +#include <ctype.h>
>  #include <unistd.h>
>  #include <errno.h>
>  #include <string.h>
> @@ -1848,36 +1850,40 @@ int main(int argc, char **argv)
>  {
>       unsigned int from = 0, to = ARRAY_SIZE(tests);
>       bool unpriv = !is_admin();
> -     int arg = 1;
> -
> -     if (argc > 1 && strcmp(argv[1], "-v") == 0) {
> +     int i, arg = 1;
> +
> +     while (argc > 1 && *argv[arg] == '-') {
> +             if (strcmp(argv[arg], "-l") == 0) {
> +                     for (i = from; i < to; i++)
> +                             printf("#%d %s\n", i, tests[i].descr);
> +                     return EXIT_SUCCESS;
> +             } else if (strcmp(argv[arg], "-v") == 0) {
> +                     verbose = true;
> +                     verif_log_level = 1;
> +             } else if (strcmp(argv[arg], "-vv") == 0) {
> +                     verbose = true;
> +                     verif_log_level = 2;
> +             } else
> +                     goto out_help;
>               arg++;
> -             verbose = true;
> -             verif_log_level = 1;
>               argc--;
>       }
> -     if (argc > 1 && strcmp(argv[1], "-vv") == 0) {
> -             arg++;
> -             verbose = true;
> -             verif_log_level = 2;
> -             argc--;
> -     }
> -
> -     if (argc == 3) {
> -             unsigned int l = atoi(argv[arg]);
> -             unsigned int u = atoi(argv[arg + 1]);
>  
> -             if (l < to && u < to) {
> -                     from = l;
> -                     to   = u + 1;
> -             }
> -     } else if (argc == 2) {
> -             unsigned int t = atoi(argv[arg]);
> +     for (i = 1; i <= 2 && argc > 1; i++, arg++, argc--) {
> +             unsigned int t = min(atoi(argv[arg]), ARRAY_SIZE(tests) - 1);
>  
> -             if (t < to) {
> +             if (!isdigit(*argv[arg]))
> +                     goto out_help;
> +             if (i == 1)
>                       from = t;
> -                     to   = t + 1;
> -             }
> +             to = t + 1;
> +     }
> +
> +     if (argc > 1) {
> +out_help:
> +             printf("Usage: %s -l | [-v|-vv] [<tst_lo> [<tst_hi>]]\n",
> +                    argv[0]);
> +             return EXIT_FAILURE;
>       }
>  
>       unpriv_disabled = get_unpriv_disabled();
> -- 
> 2.34.1
> 
> 

Reply via email to