[v11,2/6] option: make parse_option_bool/number non-static

Commit Message

Daniel P. Berrangé Sept. 5, 2016, 3:16 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 <[e-mail]>
Reviewed-by: Eric Blake <[e-mail]>
Reviewed-by: Markus Armbruster <[e-mail]>
Signed-off-by: Daniel P. Berrange <[e-mail]>
---
 include/qemu/option.h |  4 ++++
 qapi/opts-visitor.c   | 19 +------------------
 util/qemu-option.c    | 27 ++++++++++++++++-----------
 3 files changed, 21 insertions(+), 29 deletions(-)

Comments

Kevin Wolf Sept. 14, 2016, 2:33 p.m. UTC | #1

Am 05.09.2016 um 17:16 hat Daniel P. Berrange geschrieben:
> 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 <[e-mail]>
> Reviewed-by: Eric Blake <[e-mail]>
> Reviewed-by: Markus Armbruster <[e-mail]>
> Signed-off-by: Daniel P. Berrange <[e-mail]>

Reviewed-by: Kevin Wolf <[e-mail]>

Patch

diff --git a/include/qemu/option.h b/include/qemu/option.h
index 1f9e3f9..2a5266f 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 1048bbc..084f7cc 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;