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

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

 



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.




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

  Powered by Linux