
On Tue, Mar 12, 2019 at 08:38:18AM +0100, Peter Krempa wrote:
On Tue, Mar 12, 2019 at 00:38:05 +0900, Shotaro Gotanda wrote:
This function parse and convert string "yes|no" into bool true|false.
Signed-off-by: Shotaro Gotanda <g.sho1500@gmail.com> --- src/util/virstring.c | 32 ++++++++++++++++++++++++++++++++ src/util/virstring.h | 3 +++ 2 files changed, 35 insertions(+)
diff --git a/src/util/virstring.c b/src/util/virstring.c index 33f8191f45..70018459de 100644 --- a/src/util/virstring.c +++ b/src/util/virstring.c @@ -1548,3 +1548,35 @@ virStringParsePort(const char *str,
return 0; } + + +/** + * virStringParseYesNo: + * @str: "yes|no" to parse + * @port: pointer to parse and convert "yes|no" into + * + * Parses a string "yes|no" and convert it into true|false. Returns + * 0 on success and -1 on error. + */ +int virStringParseYesNo(const char *str, + bool *result) +{ + bool r = false;
+ + if (!str)
Here you don't report an error ...
+ return -1; + + if (STREQ(str, "yes")) { + r = true; + } else if (STREQ(str, "no")) { + r = false;
This variable is not very useful. If the string matches you can unconditionally overwrite *result.
+ } else { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid value, '%s' must be either 'yes' or 'no'"), str);
... but here you do. We try to do consistent error reporting since then it's hard to figure out whether to report an error or not.
+ return -1; + } + + *result = r; + + return 0; +} diff --git a/src/util/virstring.h b/src/util/virstring.h index 1e36ac459c..b921d5407b 100644 --- a/src/util/virstring.h +++ b/src/util/virstring.h @@ -316,6 +316,9 @@ int virStringParsePort(const char *str, unsigned int *port) ATTRIBUTE_NONNULL(2) ATTRIBUTE_RETURN_CHECK;
+int virStringParseYesNo(const char *str, + bool *result);
This should have a ATTRIBUTE_NONNULL(2) since 'result' is dereferenced
ATTRIBUTE_NONNULL expands only if we're running under static analysis, IIRC there was an issue that if we enabled it unconditionally, GCC then optimized if (!var) statements; away...Instead, I believe we want to enforce 'if (!var)' checks. Erik