On 02/19/2014 08:12 AM, Kevin Wolf wrote: > Multiple -o options has the same meaning as having a single option with > all settings in the order of their respective -o options.
Ah, this addresses (part of) the problem I raised about patch 1. But it feels a bit awkward to have the intermediate state. > case 'o': > if (is_help_option(optarg)) { > options_help = true; > } else if (!options) { > - options = optarg; > + options = g_strdup(optarg); > } else { > - error_report("-o cannot be used multiple times. Please use a > " > - "single -o option with comma-separated settings > " > - "instead."); > - return 1; > + options = g_strdup_printf("%s,%s", options, optarg); In addition to the memleak that Fam pointed out, I still think you have the problem that you aren't detecting: -o backing_file=/path/to/foo,help as specifying options_help. I think you instead need to collect ALL -o options without special-casing options_help, then at the end, call contains_help_option which looks for "help" or "?" among all the comma-separated options. > > + g_free(options); > return 0; > + > +fail: > + g_free(options); > + return 1; > } I'd also like if we started using EXIT_FAILURE instead of the magic number 1 (but that's probably a separate cleanup). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature