Re: [PATCH 19/28] qemu-img: resize: do not always eat last argument

[Michael Tokarev]

26.02.2024 17:52, Daniel P. Berrangé wrote:

On Thu, Feb 22, 2024 at 12:16:00AM +0300, Michael Tokarev wrote:

'qemu-img resize --help' does not work, since it wants more
arguments.  Also it -size is only recognized as a very last
argument, but it is common for tools to handle other options
after positional arguments too.

Tell getopt_long() to return non-options together with options,
and process filename and size in the loop, and check if there's
an argument right after filename which looks like -N (number),
and treat it as size (decrement).  This way we can handle --help,
and we can also have options after filename and size, and `--'
will be handled fine too.

The only case which is not handled right is when there's an option
between filename and size, and size is given as decrement, - in
this case -size will be treated as option, not as size.

Signed-off-by: Michael Tokarev <m...@tls.msk.ru>
---
  qemu-img.c | 41 +++++++++++++++++++++++++++--------------
  1 file changed, 27 insertions(+), 14 deletions(-)

diff --git a/qemu-img.c b/qemu-img.c
index 2a4bff2872..c8b0b68d67 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -4296,7 +4296,7 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
  {
      Error *err = NULL;
      int c, ret, relative;
-    const char *filename, *fmt, *size;
+    const char *filename = NULL, *fmt = NULL, *size = NULL;
      int64_t n, total_size, current_size;
      bool quiet = false;
      BlockBackend *blk = NULL;
@@ -4319,17 +4319,7 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
      bool image_opts = false;
      bool shrink = false;

- /* Remove size from argv manually so that negative numbers are not treated

-     * as options by getopt. */
-    if (argc < 3) {
-        error_exit(argv[0], "Not enough arguments");
-        return 1;
-    }
-
-    size = argv[--argc];
-
      /* Parse getopt arguments */
-    fmt = NULL;
      for(;;) {
          static const struct option long_options[] = {
              {"help", no_argument, 0, 'h'},
@@ -4339,7 +4329,7 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
              {"shrink", no_argument, 0, OPTION_SHRINK},
              {0, 0, 0, 0}
          };
-        c = getopt_long(argc, argv, ":f:hq",
+        c = getopt_long(argc, argv, "-:f:hq",

In other patches you removed the initial ':' from gopt_long arg strings.

Yes, this is done in the next patch, "resize: refresh options/help".

                          long_options, NULL);
          if (c == -1) {
              break;
@@ -4377,12 +4367,35 @@ static int img_resize(const img_cmd_t *ccmd, int argc, 
char **argv)
          case OPTION_SHRINK:
              shrink = true;
              break;
+        case 1: /* a non-optional argument */
+            if (!filename) {
+                filename = optarg;
+                /* see if we have -size (number) next to filename */
+                if (optind < argc) {
+                    size = argv[optind];
+                    if (size[0] == '-' && size[1] >= '0' && size[1] <= '9') {
+                        ++optind;
+                    } else {
+                        size = NULL;
+                    }
+                }
+            } else if (!size) {
+                size = optarg;
+            } else {
+                error_exit(argv[0], "Extra argument(s) in command line");
+            }
+            break;

Can you say what scenario exercises this code 'case 1' ?  I couldn't
get it to run in any scenarios i tried, and ineed removing this,
and removing the 'getopt_long' change, I could still run  'qemu-img resize 
--help'
OK, and also run 'qemu-img resize foo -43' too.

I was thinking about
  qemu-img resize foo -43 -f qcow2 ..

if not only to make it all consistent with everything else
(options has always been recognized after non-optional args
in gnu/linux world, all utils does that).

But in all scenarios, after changing first char of optstring to include
'-', this code will be called for any non-optional argument.  In this
case, it will be done for argument `foo', and there. -43 will  be
recognized by this piece of code as a size modification since it
starts with minus and follows with a number.

The handling of positional args after the getopt loop is also needed
to handle situations like

  qemu-img resize -- foo 43

-- everything after `--' will be left to that code.

/mjt