Re: [kvm-unit-tests PATCH v4 05/13] lib/util: add args_parse_keyval

From: Andrew Jones <drjones@redhat.com>
To: "Radim Krčmář" <rkrcmar@redhat.com>
Cc: kvm@vger.kernel.org, Paolo Bonzini <pbonzini@redhat.com>
Subject: Re: [kvm-unit-tests PATCH v4 05/13] lib/util: add args_parse_keyval
Date: Wed, 11 May 2016 18:58:18 +0200	[thread overview]
Message-ID: <20160511165818.dzk6muttgr5fctw5@hawk.localdomain> (raw)
In-Reply-To: <1462983171-4208-6-git-send-email-rkrcmar@redhat.com>

On Wed, May 11, 2016 at 06:12:47PM +0200, Radim Krčmář wrote:
> The function parses command line arguments in "key=val" format, and
> treats "key" as "key=1".
> 
> Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
> ---
>  v4: new
> 
>  lib/util.c | 17 +++++++++++++++++
>  lib/util.h | 10 ++++++++++
>  2 files changed, 27 insertions(+)
> 
> diff --git a/lib/util.c b/lib/util.c
> index 69b18100c972..8b33d474f4c0 100644
> --- a/lib/util.c
> +++ b/lib/util.c
> @@ -16,3 +16,20 @@ int parse_keyval(char *s, long *val)
>  	*val = atol(p+1);
>  	return p - s;
>  }
> +
> +long args_parse_keyval(int argc, char **argv, char *key)
> +{

I like this better than parse_keyval, except for...

> +	int i;
> +	size_t keylen = strlen(key);
> +
> +	for (i = 1; i < argc; i++) {
> +		if (keylen > 0 && strncmp(argv[i], key, keylen - 1))
> +			continue;
> +		if (argv[i][keylen] == '\0')
> +			return 1;
> +		if (argv[i][keylen] == '=')
> +			return atol(argv[i] + keylen + 1);
> +	}
> +
> +	return 0;

...this. Here we have ambiguous results. Either key was there
and had a value of 0, or wasn't there, and we still get zero.
How about merging the two, and then fixing-up the arm and powerpc
uses of the old one. I'd drop the "args_" from the name then
too.

Thanks,
drew

> +}
> diff --git a/lib/util.h b/lib/util.h
> index 4c4b44132277..d475b058526b 100644
> --- a/lib/util.h
> +++ b/lib/util.h
> @@ -20,4 +20,14 @@
>   */
>  extern int parse_keyval(char *s, long *val);
>  
> +/*
> + * argc and argv are standard C arguments to main().
> + * args_parse_keyval looks for an element of argv that matches the key.
> + * Returns val interpreted as a long if the element is in key=val format.
> + * Returns 1 if the element is key.
> + * Returns 0 otherwise.
> + *
> + */
> +long args_parse_keyval(int argc, char **argv, char *key);
> +
>  #endif
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kvm" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html