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 <mjt@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