[libvirt PATCH v2 09/51] conf: Use virTristateXXX in virDomainMemballoonDef

[Tim Wiederhake]

On Fri, 2021-03-19 at 15:00 +0100, Peter Krempa wrote:
> On Fri, Mar 19, 2021 at 14:40:23 +0100, Tim Wiederhake wrote:
> > Signed-off-by: Tim Wiederhake <twiederh at redhat.com>
> > ---
> >  src/conf/domain_conf.h | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
> > index 09b697432d..2d342effb1 100644
> > --- a/src/conf/domain_conf.h
> > +++ b/src/conf/domain_conf.h
> > @@ -1921,8 +1921,8 @@ struct _virDomainMemballoonDef {
> >      int model;
> >      virDomainDeviceInfo info;
> >      int period; /* seconds between collections */
> > -    int autodeflate; /* enum virTristateSwitch */
> > -    int free_page_reporting; /* enum virTristateSwitch */
> > +    virTristateSwitch autodeflate;
> > +    virTristateSwitch free_page_reporting;
> 
> Sorry to jump in in the middle of the series without the intention to
> continue review, but this is wong, when coupled with the code
> assigning
> to the enum value directly from virTristateSwitchTypeFromString:
> 
> Such as:
> 
> (def->autodeflate = virTristateSwitchTypeFromString(deflate)) <= 0)
> 
> virTristateSwitch is an enum value, thus treated as an unsigned
> value,
> while here you compare it with a signed value.
> 
> Comparing it with a negative value doesn't work.
> 
> Here the compiler doesn't moan as it's a <= 0 check, but if you
> change
> it to < you get a compiler warning that the statement is a
> contradiction:
> 
> ../../../libvirt/src/conf/domain_conf.c:14520:71: error: result of
> comparison of unsigned enum expression < 0 is always false [-Werror,-
> Wtautological-unsigned-enum-zero-compare]
>         (def->autodeflate = virTristateSwitchTypeFromString(deflate))
> < 0) {
>         ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> ^ ~
> 
> 
> As of such, return value of virTristateSwitchTypeFromString must
> never
> be directly assigned to a variable of virTristateSwitch type.
> 

Ah, thanks, I thought I caught all of those (see e.g. 
https://listman.redhat.com/archives/libvir-list/2021-March/msg01006.html
).

Will double check, and to your all delight, resend the entire series.

Cheers,
Tim