Re: [PATCH 4/5] x86: Use getopt to handle command line args

[Jan Beulich <jbeulich@xxxxxxxx>]

On 05.04.2024 14:11, Fouad Hilly wrote:
> @@ -82,9 +86,16 @@ static void show_curr_cpu(FILE *f)
>  int main(int argc, char *argv[])
>  {
>      int fd, ret;
> -    char *filename, *buf;
> +    char *filename = NULL, *buf;

Why?

>      size_t len;
>      struct stat st;
> +    int opt;
> +
> +    const static struct option options[] = {

Nit: Canonically "static" comes first and all type information is kept
together.

> +        {"help", no_argument, NULL, 'h'},
> +        {"show-cpu-info", no_argument, NULL, 's'},
> +        {NULL, no_argument, NULL, 0}
> +    };
>  
>      xch = xc_interface_open(NULL, NULL, 0);
>      if ( xch == NULL )
> @@ -94,20 +105,33 @@ int main(int argc, char *argv[])
>          exit(1);
>      }
>  
> -    if ( argc < 2 )
> +    if ( argc != 2 )
> +        goto ext_err;
> +
> +    while ( (opt = getopt_long(argc, argv, "hs:", options, NULL)) != -1 )
>      {
> -        usage(argv[0]);
> -        show_curr_cpu(stderr);
> -        exit(2);
> +        switch (opt)
> +        {
> +            case 'h':
> +                usage(argv[0]);
> +                exit(EXIT_SUCCESS);
> +            case 's':
> +                if ( argc > 2 )
> +                    goto ext_err;

This looks redundant with the earlier check that you adjust above.

> +                show_curr_cpu(stdout);
> +                exit(EXIT_SUCCESS);
> +            default:
> +                goto ext_err;
> +        }

Nit: Case labels want indenting the same as the immediately enclosing braces.

>      }
>  
> -    if ( !strcmp(argv[1], "show-cpu-info") )
> +    filename = argv[1];
> +    if ( filename == NULL )

Can this really happen when argc == 2?

>      {
> -        show_curr_cpu(stdout);
> -        return 0;
> +        printf("File name error\n");
> +        goto ext_err;
>      }
>  
> -    filename = argv[1];
>      fd = open(filename, O_RDONLY);
>      if ( fd < 0 )
>      {
> @@ -149,4 +173,9 @@ int main(int argc, char *argv[])
>      close(fd);
>  
>      return 0;
> +
> +ext_err:

Labels indented by at least on blank please.

Jan