Re: [PATCH v3 3/5] x86: Add usage() to print out usage message

[Jan Beulich <jbeulich@xxxxxxxx>]

On 30.04.2024 14:47, Fouad Hilly wrote:
> Refactor xen-ucode tool by adding usage() to handle usage\help messages.
> As we add more command options this will keep help\usage messages in a common 
> block.
> Only generic error message is printed to stderr. usage and show_curr_cpu are 
> printed to stdout.
> 
> Signed-off-by: Fouad Hilly <fouad.hilly@xxxxxxxxx>
> ---
> [v3]
> 1- Reverted usage message.
> 2- Utilized usage().
> 
> [v2]
> 1- Improved message description.
> 2- Fixed formatting and indentation.
> 3- Error message to print to stderr.

Already on v2 I pointed out that this change wasn't really done. And now,
in v3, ...

>  tools/misc/xen-ucode.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
> index c6ae6498d659..005bf85b6551 100644
> --- a/tools/misc/xen-ucode.c
> +++ b/tools/misc/xen-ucode.c
> @@ -17,6 +17,13 @@ static xc_interface *xch;
>  static const char intel_id[] = "GenuineIntel";
>  static const char   amd_id[] = "AuthenticAMD";
>  
> +static void usage(const char *name)
> +{
> +    printf("%s: Xen microcode updating tool\n"
> +           "Usage: %s [<microcode file> | show-cpu-info]\n",
> +           name, name);
> +}

... you're still unconditionally printing to stdout, when ...

>  static void show_curr_cpu(FILE *f)
>  {
>      int ret;
> @@ -89,9 +96,8 @@ int main(int argc, char *argv[])
>      if ( argc < 2 )
>      {
>          fprintf(stderr,
> -                "xen-ucode: Xen microcode updating tool\n"
> -                "Usage: %s [<microcode file> | show-cpu-info]\n", argv[0]);
> -        show_curr_cpu(stderr);
> +                "%s: unable to process command line arguments\n", argv[0]);
> +        usage(argv[0]);
>          exit(2);
>      }

... originally the usage message (correctly) went to stderr in this case.
It ought to go to stdout only in response to something like a --help
command line option.

Jan