| Message ID | 20240725082725.2685481-3-fouad.hilly@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | x86/xen-ucode: Introduce --force option | expand |
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
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
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
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); }
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(-)