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@xxxxxxxxxx>
> > ---
> > 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
