Re: [PATCH bpf-next v1] bpf/selftests: improve arg parsing in test_verifier

[Jiri Olsa <olsajiri@gmail.com>]

On Mon, Sep 25, 2023 at 04:37:02PM -0700, Tony Ambardar wrote:

SNIP

> @@ -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);

this looks like unnecessary loop, the code before is easy to understand,
could we just do the args check on isdigit and valid index value in there?

jirka

>  
> -		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
> 
>