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

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