[PATCH v6 2/4] x86/ucode: refactor xen-ucode to utilize getopt

Fouad Hilly posted 4 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH v6 2/4] x86/ucode: refactor xen-ucode to utilize getopt
Posted by Fouad Hilly 1 month, 3 weeks ago
Use getopt_long() to handle command line arguments.
Introduce ext_err for common exit with errors.
Introducing usage() to handle usage\help messages in a common block.
show_curr_cpu is printed to stdout only.

Signed-off-by: Fouad Hilly <fouad.hilly@cloud.com>
---
[v6]
1- Update usage() printed message format: [microcode file] [options] -> [microcode file | options]
2- Add missing blanks in switch ( opt )
[v5]
1- Update message description.
2- re-arrange static and automatic variables.
3- Fix indentations.
4- reverted the deletion of show-cpu-info for backwards compatibility.
[v4]
1- Merge three patches into one.
2- usage() to print messages to the correct stream.
3- Update commit message and description.
---
 tools/misc/xen-ucode.c | 52 +++++++++++++++++++++++++++++++++++-------
 1 file changed, 44 insertions(+), 8 deletions(-)

diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c
index 390969db3d1c..2c9f337b86cb 100644
--- a/tools/misc/xen-ucode.c
+++ b/tools/misc/xen-ucode.c
@@ -11,6 +11,7 @@
 #include <sys/stat.h>
 #include <fcntl.h>
 #include <xenctrl.h>
+#include <getopt.h>
 
 static xc_interface *xch;
 
@@ -71,12 +72,29 @@ static void show_curr_cpu(FILE *f)
     }
 }
 
+static void usage(FILE *stream, const char *name)
+{
+    fprintf(stream,
+            "%s: Xen microcode updating tool\n"
+            "options:\n"
+            "  -h, --help            display this help\n"
+            "  -s, --show-cpu-info   show CPU information\n"
+            "Usage: %s [microcode file | options]\n", name, name);
+    show_curr_cpu(stream);
+}
+
 int main(int argc, char *argv[])
 {
+    static const struct option options[] = {
+        {"help", no_argument, NULL, 'h'},
+        {"show-cpu-info", no_argument, NULL, 's'},
+        {NULL, no_argument, NULL, 0}
+    };
     int fd, ret;
     char *filename, *buf;
     size_t len;
     struct stat st;
+    int opt;
 
     xch = xc_interface_open(NULL, NULL, 0);
     if ( xch == NULL )
@@ -86,22 +104,34 @@ int main(int argc, char *argv[])
         exit(1);
     }
 
-    if ( argc < 2 )
+    while ( (opt = getopt_long(argc, argv, "hs", options, NULL)) != -1 )
     {
-        fprintf(stderr,
-                "xen-ucode: Xen microcode updating tool\n"
-                "Usage: %s [<microcode file> | show-cpu-info]\n", argv[0]);
-        show_curr_cpu(stderr);
-        exit(2);
+        switch ( opt )
+        {
+        case 'h':
+            usage(stdout, argv[0]);
+            exit(EXIT_SUCCESS);
+
+        case 's':
+            show_curr_cpu(stdout);
+            exit(EXIT_SUCCESS);
+
+        default:
+            goto ext_err;
+        }
     }
 
-    if ( !strcmp(argv[1], "show-cpu-info") )
+    if ( optind == argc )
+        goto ext_err;
+
+    /* For backwards compatibility to the pre-getopt() cmdline handling */
+    if ( !strcmp(argv[optind], "show-cpu-info") )
     {
         show_curr_cpu(stdout);
         return 0;
     }
 
-    filename = argv[1];
+    filename = argv[optind];
     fd = open(filename, O_RDONLY);
     if ( fd < 0 )
     {
@@ -146,4 +176,10 @@ int main(int argc, char *argv[])
     close(fd);
 
     return 0;
+
+ ext_err:
+    fprintf(stderr,
+            "%s: unable to process command line arguments\n", argv[0]);
+    usage(stderr, argv[0]);
+    exit(EXIT_FAILURE);
 }
-- 
2.42.0
Re: [PATCH v6 2/4] x86/ucode: refactor xen-ucode to utilize getopt
Posted by Jan Beulich 1 month, 3 weeks ago
On 25.07.2024 10:27, Fouad Hilly wrote:
> @@ -71,12 +72,29 @@ static void show_curr_cpu(FILE *f)
>      }
>  }
>  
> +static void usage(FILE *stream, const char *name)
> +{
> +    fprintf(stream,
> +            "%s: Xen microcode updating tool\n"
> +            "options:\n"
> +            "  -h, --help            display this help\n"
> +            "  -s, --show-cpu-info   show CPU information\n"
> +            "Usage: %s [microcode file | options]\n", name, name);

Oh, and: While I gave this precise layout as an outline, it wasn't really
meant to be used literally. Note how "microcode" and "file" now suggest
there need to be two separate command line elements. Perhaps using
"microcode-file" instead may already make this less ambiguous.

Jan
Re: [PATCH v6 2/4] x86/ucode: refactor xen-ucode to utilize getopt
Posted by Fouad Hilly 4 weeks ago
On Thu, Jul 25, 2024 at 9:41 AM Jan Beulich <jbeulich@suse.com> wrote:

> On 25.07.2024 10:27, Fouad Hilly wrote:
> > @@ -71,12 +72,29 @@ static void show_curr_cpu(FILE *f)
> >      }
> >  }
> >
> > +static void usage(FILE *stream, const char *name)
> > +{
> > +    fprintf(stream,
> > +            "%s: Xen microcode updating tool\n"
> > +            "options:\n"
> > +            "  -h, --help            display this help\n"
> > +            "  -s, --show-cpu-info   show CPU information\n"
> > +            "Usage: %s [microcode file | options]\n", name, name);
>
> Oh, and: While I gave this precise layout as an outline, it wasn't really
> meant to be used literally. Note how "microcode" and "file" now suggest
> there need to be two separate command line elements. Perhaps using
> "microcode-file" instead may already make this less ambiguous.
>

Yes indeed, I will fix in v7:
  static void usage(FILE *stream, const char *name)
{
    fprintf(stream,
            "%s: Xen microcode updating tool\n"
            "Usage: %s [options | microcode-file]\n"
            "options:\n"
            "  -h, --help                       display this help\n"
            "  -s, --show-cpu-info              show CPU information\n"
            "  -f, --force <microcode-file>     skip certain checks; do not
\n"
            "                                   use unless you know exactly
\n"
            "                                   what you are doing\n",
            name, name);
    show_curr_cpu(stream);
}

>
> Jan
>

Thanks,

Fouad
Re: [PATCH v6 2/4] x86/ucode: refactor xen-ucode to utilize getopt
Posted by Jan Beulich 1 month, 3 weeks ago
On 25.07.2024 10:27, Fouad Hilly wrote:
> @@ -71,12 +72,29 @@ static void show_curr_cpu(FILE *f)
>      }
>  }
>  
> +static void usage(FILE *stream, const char *name)
> +{
> +    fprintf(stream,
> +            "%s: Xen microcode updating tool\n"
> +            "options:\n"
> +            "  -h, --help            display this help\n"
> +            "  -s, --show-cpu-info   show CPU information\n"
> +            "Usage: %s [microcode file | options]\n", name, name);

You did see Anthony's comments on this before sending the new version,
didn't you? I agree with him (and I'm somewhat embarrassed that I didn't
notice this myself earlier on).

> @@ -146,4 +176,10 @@ int main(int argc, char *argv[])
>      close(fd);
>  
>      return 0;
> +
> + ext_err:
> +    fprintf(stderr,
> +            "%s: unable to process command line arguments\n", argv[0]);
> +    usage(stderr, argv[0]);
> +    exit(EXIT_FAILURE);
>  }

And there was a comment on this, too.

Jan