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(-)
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);
}
--
2.42.0On 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
© 2016 - 2024 Red Hat, Inc.