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

[Daniel P . Berrangé]

On Sat, Feb 10, 2024 at 12:22:36AM +0300, Michael Tokarev wrote:
> 'qemu-img resize --help' does not work, since it wants more arguments.
> Only eat last option at the beginning if it starts like -N.., and allow
> getopt() to do its work, and eat it up at the end if not already eaten.
> This will not allow to mix options and size anyway, but it is better
> than now.
> 
> Signed-off-by: Michael Tokarev <mjt@tls.msk.ru>
> ---
>  qemu-img.c | 15 +++++++++------
>  1 file changed, 9 insertions(+), 6 deletions(-)
> 
> diff --git a/qemu-img.c b/qemu-img.c
> index 69d41e0a92..929a25a021 100644
> --- a/qemu-img.c
> +++ b/qemu-img.c
> @@ -4271,13 +4271,13 @@ static int img_resize(const img_cmd_t *ccmd, int 
> argc, char **argv)
>  
>      /* Remove size from argv manually so that negative numbers are not 
> treated
>       * as options by getopt. */
> -    if (argc < 3) {
> -        error_exit(ccmd, "Not enough arguments");
> -        return 1;
> +    if (argc > 1 && argv[argc - 1][0] == '-'
> +        && argv[argc-1][1] >= '0' && argv[argc-1][1] <= '9') {
> +        size = argv[--argc];
> +    } else {
> +        size = NULL;
>      }

We already have a variable 'int relative' that is set to '-1'
or '+1' depending on whether we have a -ve or +ve size.

I think it is clearer to follow if we just set 'relative' much
earlier before parsing by moving this chunk of code to before
the getopt:

    switch (size[0]) {
    case '+':
        relative = 1;
        size++;
        break;
    case '-':
        relative = -1;
        size++;
        break;
    default:
        relative = 0;
        break;
    }

once we've done that we can simply replace the '-' with '+'
to stop getopt getting upset.

>  
> -    size = argv[--argc];
> -
>      /* Parse getopt arguments */
>      fmt = NULL;
>      for(;;) {
> @@ -4329,10 +4329,13 @@ static int img_resize(const img_cmd_t *ccmd, int 
> argc, char **argv)
>              break;
>          }
>      }
> -    if (optind != argc - 1) {
> +    if (optind + 1 + (size == NULL) != argc) {
>          error_exit(ccmd, "Expecting image file name and size");
>      }
>      filename = argv[optind++];
> +    if (!size) {
> +        size = argv[optind++];
> +    }
>  
>      /* Choose grow, shrink, or absolute resize mode */
>      switch (size[0]) {

With regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|