Libguestfs - May 2016 - [PATCH] tools: improve reporting for option errors (RHBZ#1316041)

Improve the error messages produced by C-based tools in case of issues
with the command line options:
- explicitly mention to use -a/-d (and -A/-D in virt-diff)
- when extra arguments are found, mention the correct way to pass
  options to certain command line switches (like --format)
- in virt-inspector, give a cleaner error message when neither -i nor
  any -m is specified

In all the cases, keep the extra notice to use 'TOOL --help' to get more
help with it.
---
 cat/cat.c             |  5 ++++-
 cat/filesystems.c     | 12 ++++++++++--
 cat/log.c             | 12 ++++++++++--
 cat/ls.c              |  5 ++++-
 diff/diff.c           | 21 +++++++++++++++------
 edit/edit.c           |  5 ++++-
 format/format.c       | 12 ++++++++++--
 fuse/guestmount.c     | 13 ++++++++++---
 inspector/inspector.c | 12 ++++++++++--
 rescue/rescue.c       | 12 ++++++++++--
 10 files changed, 87 insertions(+), 22 deletions(-)

diff --git a/cat/cat.c b/cat/cat.c
index 0370fbd..4d671ca 100644
--- a/cat/cat.c
+++ b/cat/cat.c
@@ -224,8 +224,11 @@ main (int argc, char *argv[])
   CHECK_OPTION_format_consumed;
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Add drives, inspect and mount. */
   add_drives (drvs, 'a');
diff --git a/cat/filesystems.c b/cat/filesystems.c
index 3f9d931..f1c2852 100644
--- a/cat/filesystems.c
+++ b/cat/filesystems.c
@@ -291,8 +291,13 @@ main (int argc, char *argv[])
   assert (live == 0);
 
   /* Must be no extra arguments on the command line. */
-  if (optind != argc)
+  if (optind != argc) {
+    fprintf (stderr, _("%s: error: extra argument '%s' on command
line.\n"
+             "Make sure to specify the argument for --format "
+             "like '--format=%s'.\n"),
+             guestfs_int_program_name, argv[optind], argv[optind]);
     usage (EXIT_FAILURE);
+  }
 
   CHECK_OPTION_format_consumed;
 
@@ -329,8 +334,11 @@ main (int argc, char *argv[])
     title = 0;
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Add drives. */
   add_drives (drvs, 'a');
diff --git a/cat/log.c b/cat/log.c
index daefda7..6632f5a 100644
--- a/cat/log.c
+++ b/cat/log.c
@@ -184,14 +184,22 @@ main (int argc, char *argv[])
   assert (live == 0);
 
   /* User must not specify more arguments on the command line. */
-  if (optind != argc)
+  if (optind != argc) {
+    fprintf (stderr, _("%s: error: extra argument '%s' on command
line.\n"
+             "Make sure to specify the argument for --format "
+             "like '--format=%s'.\n"),
+             guestfs_int_program_name, argv[optind], argv[optind]);
     usage (EXIT_FAILURE);
+  }
 
   CHECK_OPTION_format_consumed;
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Add drives, inspect and mount.  Note that inspector is always true,
    * and there is no -m option.
diff --git a/cat/ls.c b/cat/ls.c
index 91f2125..f990737 100644
--- a/cat/ls.c
+++ b/cat/ls.c
@@ -340,8 +340,11 @@ main (int argc, char *argv[])
     usage (EXIT_FAILURE);
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Add drives, inspect and mount. */
   add_drives (drvs, 'a');
diff --git a/diff/diff.c b/diff/diff.c
index d7542fc..210abc2 100644
--- a/diff/diff.c
+++ b/diff/diff.c
@@ -311,10 +311,14 @@ main (int argc, char *argv[])
     }
   }
 
-  if (drvs == NULL || drvs2 == NULL) {
-    fprintf (stderr,
-             _("%s: you must specify some -a|-A|-d|-D options, see
%s(1)\n"),
-             guestfs_int_program_name, guestfs_int_program_name);
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
+    usage (EXIT_FAILURE);
+  }
+  if (drvs2 == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -A or -D
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
   }
 
@@ -324,8 +328,13 @@ main (int argc, char *argv[])
   if (human && csv)
     error (EXIT_FAILURE, 0, _("you cannot use -h and --csv options
together."));
 
-  if (optind != argc)
-    error (EXIT_FAILURE, 0, _("extra arguments on the command
line"));
+  if (optind != argc) {
+    fprintf (stderr, _("%s: error: extra argument '%s' on command
line.\n"
+             "Make sure to specify the argument for --checksum or --format
"
+             "like '--format=%s'.\n"),
+             guestfs_int_program_name, argv[optind], argv[optind]);
+    usage (EXIT_FAILURE);
+  }
 
   /* These are really constants, but they have to be variables for the
    * options parsing code.  Assert here that they have known-good
diff --git a/edit/edit.c b/edit/edit.c
index 6e7aee4..2d3ccc6 100644
--- a/edit/edit.c
+++ b/edit/edit.c
@@ -253,8 +253,11 @@ main (int argc, char *argv[])
   CHECK_OPTION_format_consumed;
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Add drives. */
   add_drives (drvs, 'a');
diff --git a/format/format.c b/format/format.c
index 4aa31de..5026aff 100644
--- a/format/format.c
+++ b/format/format.c
@@ -219,14 +219,22 @@ main (int argc, char *argv[])
   assert (live == 0);
 
   /* Must be no extra arguments on the command line. */
-  if (optind != argc)
+  if (optind != argc) {
+    fprintf (stderr, _("%s: error: extra argument '%s' on command
line.\n"
+             "Make sure to specify the argument for --format, --lvm "
+             "or --partition like '--format=%s'.\n"),
+             guestfs_int_program_name, argv[optind], argv[optind]);
     usage (EXIT_FAILURE);
+  }
 
   CHECK_OPTION_format_consumed;
 
   /* The user didn't specify any drives to format. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Because the libguestfs kernel can get stuck rereading the
    * partition table after things have been erased, we sometimes need
diff --git a/fuse/guestmount.c b/fuse/guestmount.c
index fc03a9c..f72ecb8 100644
--- a/fuse/guestmount.c
+++ b/fuse/guestmount.c
@@ -311,9 +311,16 @@ main (int argc, char *argv[])
 
   /* Check we have the right options. */
   if (!live) {
-    if (!drvs || !(mps || inspector))
-      error (EXIT_FAILURE, 0,
-             _("must have at least one -a/-d and at least one -m/-i
option"));
+    if (drvs == NULL) {
+      fprintf (stderr, _("%s: error: you must specify at least one -a or
-d option.\n"),
+               guestfs_int_program_name);
+      usage (EXIT_FAILURE);
+    }
+    if (!(mps || inspector)) {
+      fprintf (stderr, _("%s: error: you must specify either -i at least
one -m option.\n"),
+               guestfs_int_program_name);
+      usage (EXIT_FAILURE);
+    }
   } else {
     size_t count_d = 0, count_other = 0;
     struct drv *drv;
diff --git a/inspector/inspector.c b/inspector/inspector.c
index 6d4694d..d8e455e 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -233,8 +233,13 @@ main (int argc, char *argv[])
   assert (live == 0);
 
   /* Must be no extra arguments on the command line. */
-  if (optind != argc)
+  if (optind != argc) {
+    fprintf (stderr, _("%s: error: extra argument '%s' on command
line.\n"
+             "Make sure to specify the argument for --format "
+             "like '--format=%s'.\n"),
+             guestfs_int_program_name, argv[optind], argv[optind]);
     usage (EXIT_FAILURE);
+  }
 
   CHECK_OPTION_format_consumed;
 
@@ -252,8 +257,11 @@ main (int argc, char *argv[])
   }
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Add drives, inspect and mount.  Note that inspector is always true,
    * and there is no -m option.
diff --git a/rescue/rescue.c b/rescue/rescue.c
index c46c775..135c9e6 100644
--- a/rescue/rescue.c
+++ b/rescue/rescue.c
@@ -275,14 +275,22 @@ main (int argc, char *argv[])
   assert (live == 0);
 
   /* Must be no extra arguments on the command line. */
-  if (optind != argc)
+  if (optind != argc) {
+    fprintf (stderr, _("%s: error: extra argument '%s' on command
line.\n"
+             "Make sure to specify the argument for --format or --scratch
"
+             "like '--format=%s'.\n"),
+             guestfs_int_program_name, argv[optind], argv[optind]);
     usage (EXIT_FAILURE);
+  }
 
   CHECK_OPTION_format_consumed;
 
   /* User must have specified some drives. */
-  if (drvs == NULL)
+  if (drvs == NULL) {
+    fprintf (stderr, _("%s: error: you must specify at least one -a or -d
option.\n"),
+             guestfs_int_program_name);
     usage (EXIT_FAILURE);
+  }
 
   /* Setting "direct mode" is required for the rescue appliance. */
   if (guestfs_set_direct (g, 1) == -1)
-- 
2.5.5

On Thu, May 05, 2016 at 02:32:46PM +0200, Pino Toscano
wrote:
> Improve the error messages produced by C-based tools in case of issues
> with the command line options:
> - explicitly mention to use -a/-d (and -A/-D in virt-diff)
> - when extra arguments are found, mention the correct way to pass
>   options to certain command line switches (like --format)
> - in virt-inspector, give a cleaner error message when neither -i nor
>   any -m is specified
> 
> In all the cases, keep the extra notice to use 'TOOL --help' to get
more
> help with it.
Looks good, ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
Fedora Windows cross-compiler. Compile Windows programs, test, and
build Windows installers. Over 100 libraries supported.
http://fedoraproject.org/wiki/MinGW