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 <kw...@redhat.com>
> ---
> +
> + 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 <ebl...@redhat.com>
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
