Re: [LTP] [PATCH v4 3/5] shell: add kconfig parse api

[Cyril Hrubis <chrubis@suse.cz>]

Hi!
> int main(int argc, char *argv[])
> {
>          char *str = argv[1];
>          char *delim = argv[2];
>          unsigned int i, cnt = 1;
>          int ret = 0;
> 
>          if (argc == 2) {
>                  delim = ",";
>          } else if (argc == 3) {
>                  if (strlen(delim) > 1) {
>                          fprintf(stderr, "The delim must be a single 
> character\n");
>                          return 1;
>                  }
>          } else {
>                  fprintf(stderr, "Please provide kernel kconfig list and 
> delim "
>                                  "(optinal, default value is ',')\n");
>                  return 1;
>          }

We can make the code a bit easier to read with a switch()

	switch (argc) {
	case 2:
		delim = ",";
	break;
	case 3:
		if (strlen(delim) > 1) {
			fprintf(stderr, "...");
			return 1;
		}
	break;
	default:
		fprintf(stderr, "...");
		return 1;
	}


>          for (i = 0; str[i]; i++) {
>                  if (str[i] == delim[0])
>                          cnt++;
>          }
> 
>          char **kconfigs = malloc(++cnt * sizeof(char *));

It's unlikely that this will fail the allocation, but for the sake of
correctness we should also do:

	if (!kconfigs) {
		fprintf(stderr, "malloc failed");
		return 1;
	}

>          for (i = 0; i < cnt; i++)
>                  kconfigs[i] = strtok_r(str, delim, &str);
> 
>          if (tst_kconfig_check((const char * const*)kconfigs))
>                  ret = 1;
> 
>          free(kconfigs);
>          return ret;
> }

Other than the two minor issues this version looks good to me:

Reviewed-by: Cyril Hrubis <chrubis@suse.cz>

-- 
Cyril Hrubis
chrubis@suse.cz

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp