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@xxxxxxxxx>
> ---
> 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
without checking and also ATTRIBUTE_RETURN_CHECK as it reports an error.
> +
> /**
> * VIR_AUTOSTRINGLIST:
> *
> --
> 2.19.1
>
> --
> libvir-list mailing list
> libvir-list@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
Attachment:
signature.asc
Description: PGP signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list
