[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v3 1/6] qemu-option: has_help_option() and is_va
From: |
Eric Blake |
Subject: |
Re: [Qemu-devel] [PATCH v3 1/6] qemu-option: has_help_option() and is_valid_option_list() |
Date: |
Fri, 21 Feb 2014 13:22:43 -0700 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:24.0) Gecko/20100101 Thunderbird/24.3.0 |
On 02/21/2014 08:24 AM, Kevin Wolf wrote:
> has_help_option() checks if any help option ('help' or '?') occurs
> anywhere in an option string, so that things like 'cluster_size=4k,help'
> are recognised.
>
> is_valid_option_list() ensures that the option list doesn't have options
> with leading commas or trailing unescaped commas.
>
> Signed-off-by: Kevin Wolf <address@hidden>
> ---
> +
> + while (*p) {
> + p = get_opt_value(buf, buflen, p);
> + if (*p) {
> + p++;
> + }
> +
> + if (is_help_option(buf)) {
> + result = true;
> + goto out;
If this were 'break;',
> + }
> + }
> +
> +out:
then you wouldn't need this label. But that's cosmetic.
> + free(buf);
> + return result;
> +}
> +
> +bool is_valid_option_list(const char *param)
> +{
> + size_t buflen = strlen(param) + 1;
> + char *buf = g_malloc0(buflen);
> + const char *p = param;
> + bool result = true;
> +
> + while (*p) {
> + p = get_opt_value(buf, buflen, p);
> + if (*p && !*++p) {
> + result = false;
> + goto out;
> + }
Rejects trailing commas.
> +
> + if (!*buf || *buf == ',') {
Rejects empty options, but also rejects values beginning with a comma.
But we have legacy users that accept implicitly named first options (see
opts_do_parse()). For example, this is a valid command line (albeit one
that prints a list of valid machines):
qemu-kvm -machine ,,blah
as shorthand for
qemu-kvm -machine type=,,blah
and where *buf would indeed be validly ','.
> + result = false;
> + goto out;
> + }
> + }
> +
> +out:
Same observation about the possibility of using break to avoid goto.
> + free(buf);
> + return result;
> +}
That said, none of our users of -o options have an implicitly-named
first option, and therefore THIS patch is safe, even if we have a
ticking time-bomb if any other option with implicit first name starts
using this function. And break vs. goto is cosmetic. So I'm okay
taking this as-is.
Reviewed-by: Eric Blake <address@hidden>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
[Qemu-devel] [PATCH v3 2/6] qemu-img create: Support multiple -o options, Kevin Wolf, 2014/02/21
[Qemu-devel] [PATCH v3 3/6] qemu-img convert: Support multiple -o options, Kevin Wolf, 2014/02/21
[Qemu-devel] [PATCH v3 4/6] qemu-img amend: Support multiple -o options, Kevin Wolf, 2014/02/21
[Qemu-devel] [PATCH v3 5/6] qemu-img: Allow -o help with incomplete argument list, Kevin Wolf, 2014/02/21
[Qemu-devel] [PATCH v3 6/6] qemu-iotests: Check qemu-img command line parsing, Kevin Wolf, 2014/02/21
Re: [Qemu-devel] [PATCH v3 0/6] qemu-img: Support multiple -o options, Jeff Cody, 2014/02/21