Re: [libvirt] [PATCH 1/2] util: add virStringParseYesNo()

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(a)gmail.com&gt; > --- > 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