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

'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(-)

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

20.02.2024 20:57, 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:

Well, we'll also have to repeat the same switch after getopt, since
there, I only test for -[0-9], not +[0-9] or [0-9].   But yes, it
can be done too.

But this is more interesting, - I think we should switch getopt to
use 'return in order' option, and process options together with
getopt, looking at the next option at each iteration, - if it looks
like [+-]size if we already got the filename part.  Lemme try to
do that..

/mjt