Re: [libvirt PATCH v2 09/51] conf: Use virTristateXXX in virDomainMemballoonDef
On Fri, Mar 19, 2021 at 14:40:23 +0100, Tim Wiederhake wrote:
Signed-off-by: Tim Wiederhake <twiederh(a)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.