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

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