[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 19/28] qemu-img: resize: do not always eat last argument
From: |
Daniel P . Berrangé |
Subject: |
Re: [PATCH 19/28] qemu-img: resize: do not always eat last argument |
Date: |
Mon, 26 Feb 2024 14:52:49 +0000 |
User-agent: |
Mutt/2.2.12 (2023-09-09) |
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.
> 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.
> }
> }
> - if (optind != argc - 1) {
> + if (!filename && optind < argc) {
> + filename = argv[optind++];
> + }
> + if (!size && optind < argc) {
> + size = argv[optind++];
> + }
> + if (!filename || !size || optind < argc) {
> error_exit(argv[0], "Expecting image file name and size");
> }
> - filename = argv[optind++];
>
> /* Choose grow, shrink, or absolute resize mode */
> switch (size[0]) {
> --
> 2.39.2
>
>
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 :|
- [PATCH 10/28] qemu-img: commit: refresh options/--help, (continued)
- [PATCH 10/28] qemu-img: commit: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 12/28] qemu-img: convert: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 11/28] qemu-img: compare: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 14/28] qemu-img: map: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 13/28] qemu-img: info: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 16/28] qemu-img: snapshot: make -l (list) the default, simplify option handling, Michael Tokarev, 2024/02/21
- [PATCH 15/28] qemu-img: snapshot: allow specifying -f fmt, Michael Tokarev, 2024/02/21
- [PATCH 17/28] qemu-img: snapshot: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 19/28] qemu-img: resize: do not always eat last argument, Michael Tokarev, 2024/02/21
- Re: [PATCH 19/28] qemu-img: resize: do not always eat last argument,
Daniel P . Berrangé <=
- [PATCH 20/28] qemu-img: resize: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 18/28] qemu-img: rebase: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 21/28] qemu-img: amend: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 22/28] qemu-img: bench: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 23/28] qemu-img: bitmap: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 24/28] qemu-img: dd: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 25/28] qemu-img: measure: refresh options/--help, Michael Tokarev, 2024/02/21
- [PATCH 27/28] qemu-img: inline list of supported commands, remove qemu-img-cmds.h include, Michael Tokarev, 2024/02/21