| Message ID | 7e0e7cb2470d572e8c0a48ba85c993be3bdb1c07.1707513011.git.mjt@tls.msk.ru (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qemu-img: refersh options and --help handling | expand |
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
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; } - 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]) {
'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(-)