On 08/22/2012 06:45 AM, Benoît Canet wrote: > This option --output=[human|json] make qemu-img info output on > human or JSON representation at the choice of the user. >
> @@ -1083,7 +1088,6 @@ out: > return 0; > } > > - > static void dump_snapshots(BlockDriverState *bs) > { Spurious whitespace change. > +static int img_info(int argc, char **argv) > +{ > + int c; > + bool human = false, json = false; > + const char *filename, *fmt, *output; > + BlockDriverState *bs; > + ImageInfo *info; > > fmt = NULL; > + output = NULL; > for(;;) { > - c = getopt(argc, argv, "f:h"); > + int option_index = 0; > + static struct option long_options[] = { > + {"help", no_argument, 0, 'h'}, > + {"format", required_argument, 0, 'f'}, > + {"output", required_argument, 0, 'm'}, I would define a constant, such as 'enum {OPTION_FORMAT=256};', so that you don't risk future confusion... > + {0, 0, 0, 0} > + }; > + c = getopt_long(argc, argv, "f:h", ...if we later do add an 'm' short option. > + long_options, &option_index); > if (c == -1) { > break; > } > @@ -1128,6 +1299,9 @@ static int img_info(int argc, char **argv) > case 'f': > fmt = optarg; > break; > + case 'm': Given the above, this would reuse your new named value. > @@ -1135,52 +1309,42 @@ static int img_info(int argc, char **argv) > } > filename = argv[optind++]; > > + if (output && !strncmp(output, "json", strlen("json"))) { Why strncmp? It ignores trailing garbage (as in --output=jsonoops). Stick to strcmp. > + json = true; > + } else if (output && !strncmp(output, "human", strlen("human"))) { And again. > + human = true; > + } else { > + fprintf(stderr, > + "Error: --output must be used with human or json as > argument.\n"); > + return 1; > + } If we get here, and --output=... was not given, then both human and json are false. That's a problem, since... > + > + if (human) { > + dump_human_image_info(info); > + dump_snapshots(bs); > } > - bdrv_get_backing_filename(bs, backing_filename, > sizeof(backing_filename)); > - if (backing_filename[0] != '\0') { > - bdrv_get_full_backing_filename(bs, backing_filename2, > - sizeof(backing_filename2)); > - printf("backing file: %s", backing_filename); > - if (strcmp(backing_filename, backing_filename2) != 0) { > - printf(" (actual path: %s)", backing_filename2); > - } > - putchar('\n'); > + > + if (json) { > + collect_snapshots(bs, info); > + dump_json_image_info(info); > } > - dump_snapshots(bs); > + > + qapi_free_ImageInfo(info); ...you will end up with no output. You want to default to human output for back-compat to older usage. -- Eric Blake ebl...@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature