Hi
On Tue, Jun 14, 2016 at 6:07 PM, Daniel P. Berrange <berra...@redhat.com> wrote:
> The opts-visitor.c opts_type_bool() method has code for
> parsing a string to set a bool value, as does the
> qemu-option.c parse_option_bool() method, except it
> handles fewer cases.
>
> To enable consistency across the codebase, extend
> parse_option_bool() to handle "yes", "no", "y" and
> "n", and make it non-static. Convert the opts
> visitor to call this method directly.
>
> Also make parse_option_number() non-static to allow
> for similar reuse later.
>
> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
> ---
> include/qemu/option.h | 4 ++++
> qapi/opts-visitor.c | 19 +------------------
> util/qemu-option.c | 27 ++++++++++++++++-----------
> 3 files changed, 21 insertions(+), 29 deletions(-)
>
> diff --git a/include/qemu/option.h b/include/qemu/option.h
> index 8542d2d..c37d8e9 100644
> --- a/include/qemu/option.h
> +++ b/include/qemu/option.h
> @@ -37,6 +37,10 @@ int get_param_value(char *buf, int buf_size,
> const char *tag, const char *str);
>
>
> +void parse_option_bool(const char *name, const char *value, bool *ret,
> + Error **errp);
> +void parse_option_number(const char *name, const char *value,
> + uint64_t *ret, Error **errp);
> void parse_option_size(const char *name, const char *value,
> uint64_t *ret, Error **errp);
> bool has_help_option(const char *param);
> diff --git a/qapi/opts-visitor.c b/qapi/opts-visitor.c
> index 4cf1cf8..b3757ca 100644
> --- a/qapi/opts-visitor.c
> +++ b/qapi/opts-visitor.c
> @@ -334,7 +334,6 @@ opts_type_str(Visitor *v, const char *name, char **obj,
> Error **errp)
> }
>
>
> -/* mimics qemu-option.c::parse_option_bool() */
> static void
> opts_type_bool(Visitor *v, const char *name, bool *obj, Error **errp)
> {
> @@ -346,23 +345,7 @@ opts_type_bool(Visitor *v, const char *name, bool *obj,
> Error **errp)
> return;
> }
>
> - if (opt->str) {
> - if (strcmp(opt->str, "on") == 0 ||
> - strcmp(opt->str, "yes") == 0 ||
> - strcmp(opt->str, "y") == 0) {
> - *obj = true;
> - } else if (strcmp(opt->str, "off") == 0 ||
> - strcmp(opt->str, "no") == 0 ||
> - strcmp(opt->str, "n") == 0) {
> - *obj = false;
> - } else {
> - error_setg(errp, QERR_INVALID_PARAMETER_VALUE, opt->name,
> - "on|yes|y|off|no|n");
> - return;
> - }
> - } else {
> - *obj = true;
> - }
> + parse_option_bool(opt->name, opt->str, obj, errp);
>
> processed(ov, name);
> }
> diff --git a/util/qemu-option.c b/util/qemu-option.c
> index 3467dc2..6fc9ccb 100644
> --- a/util/qemu-option.c
> +++ b/util/qemu-option.c
> @@ -125,25 +125,30 @@ int get_param_value(char *buf, int buf_size,
> return get_next_param_value(buf, buf_size, tag, &str);
> }
>
> -static void parse_option_bool(const char *name, const char *value, bool *ret,
> - Error **errp)
> +void parse_option_bool(const char *name, const char *value, bool *ret,
> + Error **errp)
> {
> if (value != NULL) {
> - if (!strcmp(value, "on")) {
> - *ret = 1;
> - } else if (!strcmp(value, "off")) {
> - *ret = 0;
> + if (strcmp(value, "on") == 0 ||
> + strcmp(value, "yes") == 0 ||
> + strcmp(value, "y") == 0) {
> + *ret = true;
> + } else if (strcmp(value, "off") == 0 ||
> + strcmp(value, "no") == 0 ||
> + strcmp(value, "n") == 0) {
> + *ret = false;
> } else {
> - error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
> - name, "'on' or 'off'");
> + error_setg(errp, QERR_INVALID_PARAMETER_VALUE, name,
> + "on|yes|y|off|no|n");
> + return;
> }
> } else {
> - *ret = 1;
> + *ret = true;
> }
> }
>
> -static void parse_option_number(const char *name, const char *value,
> - uint64_t *ret, Error **errp)
> +void parse_option_number(const char *name, const char *value,
> + uint64_t *ret, Error **errp)
> {
> char *postfix;
> uint64_t number;
> --
> 2.5.5
>
>
Reviewed-by: Marc-André Lureau <marcandre.lur...@redhat.com>
--
Marc-André Lureau
