| Message ID | 20240528152943.3915760-3-fouad.hilly@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/xen-ucode: Introduce --force option | expand |
On 28/05/2024 4:29 pm, Fouad Hilly wrote: > Use getopt_long() to handle command line arguments. > Introduce ext_err for common exit with errors. > xc_microcode_update() refactored to accept flags and utilize xenpf_microcode_update2 Importantly, not. That's deferred until the next patch. > 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> > --- > [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 | 51 ++++++++++++++++++++++++++++++++---------- > 1 file changed, 39 insertions(+), 12 deletions(-) > > diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c > index 390969db3d1c..6f9dd2a7e431 100644 > --- a/tools/misc/xen-ucode.c > +++ b/tools/misc/xen-ucode.c > @@ -11,12 +11,23 @@ > #include <sys/stat.h> > #include <fcntl.h> > #include <xenctrl.h> > +#include <getopt.h> > > static xc_interface *xch; > > static const char intel_id[] = "GenuineIntel"; > static const char amd_id[] = "AuthenticAMD"; > > +static void usage(FILE *stream, const char *name) > +{ > + fprintf(stream, "%s: Xen microcode updating tool\n" > + "Usage: %s [microcode file] [options]\n" Options are typically expressed first when writing this. > + "options:\n" > + " -h, --help display this help and exit\n" > + " -s, --show-cpu-info show CPU information and exit\n", The "and exit" is pointless to state like this. > + name, name); Alignment is out by one column here. Also, previously, usage() was always followed by show_curr_cpu(). That's still easy to retain by reordering the two static functions. > +} > + > static void show_curr_cpu(FILE *f) > { > int ret; > @@ -77,6 +88,13 @@ int main(int argc, char *argv[]) > char *filename, *buf; > size_t len; > struct stat st; > + int opt; > + > + static const struct option options[] = { > + {"help", no_argument, NULL, 'h'}, > + {"show-cpu-info", no_argument, NULL, 's'}, > + {NULL, no_argument, NULL, 0} > + }; Typically we prefer static variables before automatic variables. This block can just be repositioned. > > xch = xc_interface_open(NULL, NULL, 0); > if ( xch == NULL ) > @@ -86,22 +104,25 @@ 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); Blank line here, ... > + case 's': > + show_curr_cpu(stdout); > + exit(EXIT_SUCCESS); ... and here. > + default: > + goto ext_err; > + } > } > > - if ( !strcmp(argv[1], "show-cpu-info") ) > - { > - show_curr_cpu(stdout); > - return 0; > - } > + if ( optind == argc ) > + goto ext_err; This is a change in API, because now "show-cpu-info" needs to take a -- to be interpreted. I suspect we want: 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; } to retain compatibility. (Also, this means you won't have the code rejected by XenRT when this gets added to Xen.) All of this I can fix on commit, but there's quite a lot in the way of changes. ~Andrew
diff --git a/tools/misc/xen-ucode.c b/tools/misc/xen-ucode.c index 390969db3d1c..6f9dd2a7e431 100644 --- a/tools/misc/xen-ucode.c +++ b/tools/misc/xen-ucode.c @@ -11,12 +11,23 @@ #include <sys/stat.h> #include <fcntl.h> #include <xenctrl.h> +#include <getopt.h> static xc_interface *xch; static const char intel_id[] = "GenuineIntel"; static const char amd_id[] = "AuthenticAMD"; +static void usage(FILE *stream, const char *name) +{ + fprintf(stream, "%s: Xen microcode updating tool\n" + "Usage: %s [microcode file] [options]\n" + "options:\n" + " -h, --help display this help and exit\n" + " -s, --show-cpu-info show CPU information and exit\n", + name, name); +} + static void show_curr_cpu(FILE *f) { int ret; @@ -77,6 +88,13 @@ int main(int argc, char *argv[]) char *filename, *buf; size_t len; struct stat st; + int opt; + + static const struct option options[] = { + {"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 ) @@ -86,22 +104,25 @@ 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") ) - { - show_curr_cpu(stdout); - return 0; - } + if ( optind == argc ) + goto ext_err; - filename = argv[1]; + filename = argv[optind]; fd = open(filename, O_RDONLY); if ( fd < 0 ) { @@ -146,4 +167,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); }
Use getopt_long() to handle command line arguments. Introduce ext_err for common exit with errors. xc_microcode_update() refactored to accept flags and utilize xenpf_microcode_update2 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> --- [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 | 51 ++++++++++++++++++++++++++++++++---------- 1 file changed, 39 insertions(+), 12 deletions(-)