Re: [PATCH 03/16] virconf: add typed value accessor methods

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

 



On Mon, 2016-07-11 at 10:45 +0100, Daniel P. Berrange wrote:
> Currently many users of virConf APIs are defining the same
> macros for calling virConfValue() and then doing type
> checking. To remove this repeated code, add a set of
> typesafe accessor methods.
> 
> Signed-off-by: Daniel P. Berrange <berrange@xxxxxxxxxx>
> ---
>  src/libvirt_private.syms |  10 +
>  src/util/virconf.c       | 500 ++++++++++++++++++++++++++++++++++++++++++++++-
>  src/util/virconf.h       |  34 +++-
>  tests/virconftest.c      | 335 +++++++++++++++++++++++++++++++
>  4 files changed, 873 insertions(+), 6 deletions(-)

[...]

> +/**
> + * virConfGetValueSSizeT:
> + * @conf: the config object
> + * @setting: the config entry name
> + * @value: pointer to hold integer value
> + *
> + * Get the integer value of the config name @setting, storing
> + * it in @value. If the config entry is not present, then
> + * @value will be unmodified.
> + *
> + * Reports an error if the config entry is set but has
> + * an unexpected type, or if the value is outside the
> + * range that can be stored in an 'ssize_t'
> + *
> + * Returns: 1 if the value was present, 0 if missing, -1 on error
> + */
> +int virConfGetValueSSizeT(virConfPtr conf,
> +                          const char *setting,
> +                          ssize_t *value)
> +{
> +    virConfValuePtr cval = virConfGetValue(conf, setting);
> +
> +    VIR_DEBUG("Get value ssize_t %p %d",
> +              cval, cval ? cval->type : VIR_CONF_NONE);
> +
> +    if (!cval)
> +        return 0;
> +
> +    if (cval->type != VIR_CONF_LONG &&
> +        cval->type != VIR_CONF_ULONG) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("%s: expected a signed integer for '%s' parameter"),
> +                       conf->filename, setting);
> +        return -1;
> +    }
> +
> +    if (cval->l > SSIZE_MAX || cval->l < (-SSIZE_MAX - 1)) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR,
> +                       _("%s: value for '%s' parameter must be in range %zd:%zd"),
> +                       conf->filename, setting, -SSIZE_MAX - 1, SSIZE_MAX);
> +        return -1;
> +    }

This seems to have introduced a build failure on CI[1]:

  ../../src/util/virconf.c: In function 'virConfGetValueSSizeT':
  ../../src/util/virconf.c:1267:5: error: logical 'or' of collectively
    exhaustive tests is always true [-Werror=logical-op]

I can't reproduce it on my Debian sid builder, though.

Does this test even make sense on 64 bit architectures? cval->l
is a long long (8 bytes) and ssize_t is 8 bytes as well, so I
would expect the error above to pop up when compiling on x86_64,
if anything.

Suren, is the libvirt-debian host 32 bit or 64 bit? Can you
maybe try updating it to rule out a since-solved compiler bug?


[1] https://ci.centos.org/view/libvirt-project/job/libvirt-daemon-build/systems=libvirt-debian/1524/console
-- 
Andrea Bolognani / Red Hat / Virtualization

--
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]