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

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