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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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

[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]

  Powered by Linux