diff mbox

[v7,2/7] option: make parse_option_bool/number non-static

Message ID 1467724312-9378-3-git-send-email-berrange@redhat.com
State New
Headers show

Commit Message

Daniel P. Berrangé July 5, 2016, 1:11 p.m. UTC 
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.

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
---
 include/qemu/option.h |  4 ++++
 qapi/opts-visitor.c   | 19 +------------------
 util/qemu-option.c    | 27 ++++++++++++++++-----------
 3 files changed, 21 insertions(+), 29 deletions(-)

Comments

Eric Blake July 14, 2016, 1:33 a.m. UTC | #1 
On 07/05/2016 07:11 AM, Daniel P. Berrange 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.
> 
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> ---
>  include/qemu/option.h |  4 ++++
>  qapi/opts-visitor.c   | 19 +------------------
>  util/qemu-option.c    | 27 ++++++++++++++++-----------
>  3 files changed, 21 insertions(+), 29 deletions(-)
> 

Reviewed-by: Eric Blake <eblake@redhat.com>

I'm going to include this in my proposed fallback patches on my qapi
subset F series, where we may need to "fix" netdev_add to care about
back-compat parsing of a string where an integer is expected.
diff mbox

Patch

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;