Series comparison

Enum variable of type qemuMigrationCapability is checked for zero in
src/qemu/qemu_migration_params.c:729.

"if (item->optional) { ..."

Actualy, QEMU_MIGRATION_CAP_XBZRLE enum constant has value 0.
Thus, all uninitialized .optinnal fields of the static array
qemuMigrationParamsFlagMap[] will be implicitly initialized with
value 0 (QEMU_MIGRATION_CAP_XBZRLE).
To my opinion, introducing a separate enum for optional capabilities,
would be a better solution.

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
---
 src/qemu/qemu_migration_params.c | 16 +++++++++++-----
 src/qemu/qemu_migration_params.h | 12 ++++++++++--
 2 files changed, 21 insertions(+), 7 deletions(-)

diff --git a/src/qemu/qemu_migration_params.c b/src/qemu/qemu_migration_params.c
index XXXXXXX..XXXXXXX 100644
--- a/src/qemu/qemu_migration_params.c
+++ b/src/qemu/qemu_migration_params.c
@@ -XXX,XX +XXX,XX @@ VIR_ENUM_IMPL(qemuMigrationCapability,
               "dirty-bitmaps",
               "return-path",
               "zero-copy-send",
+);
+
+VIR_ENUM_IMPL(qemuMigrationOptCap,
+              QEMU_MIGRATION_OPTCAP_LAST,
+              "none",
               "postcopy-preempt",
               "switchover-ack",
 );
@@ -XXX,XX +XXX,XX @@ struct _qemuMigrationParamsFlagMapItem {
     /* An optional capability to set in addition to @cap in case it is
      * supported. Depending on @part either one or both sides of migration
      * has to support the optional capability to be enabled. */
-    qemuMigrationCapability optional;
+    qemuMigrationOptCap optional;
     /* Bit-wise OR of qemuMigrationParty. Determines whether the capability has
      * to be enabled on the source, on the destination, or on both sides of
      * migration. */
@@ -XXX,XX +XXX,XX @@ static const qemuMigrationParamsFlagMapItem qemuMigrationParamsFlagMap[] = {
     {.match = QEMU_MIGRATION_FLAG_REQUIRED,
      .flag = VIR_MIGRATE_POSTCOPY,
      .cap = QEMU_MIGRATION_CAP_POSTCOPY,
-     .optional = QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT,
+     .optional = QEMU_MIGRATION_OPTCAP_POSTCOPY_PREEMPT,
      .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION},
 
     {.match = QEMU_MIGRATION_FLAG_REQUIRED,
@@ -XXX,XX +XXX,XX @@ static const qemuMigrationParamsFlagMapItem qemuMigrationParamsFlagMap[] = {
     {.match = QEMU_MIGRATION_FLAG_FORBIDDEN,
      .flag = VIR_MIGRATE_TUNNELLED,
      .cap = QEMU_MIGRATION_CAP_RETURN_PATH,
-     .optional = QEMU_MIGRATION_CAP_SWITCHOVER_ACK,
+     .optional = QEMU_MIGRATION_OPTCAP_SWITCHOVER_ACK,
      .party = QEMU_MIGRATION_SOURCE | QEMU_MIGRATION_DESTINATION},
 
     {.match = QEMU_MIGRATION_FLAG_REQUIRED,
@@ -XXX,XX +XXX,XX @@ qemuMigrationParamsFromFlags(virTypedParameterPtr params,
                       qemuMigrationCapabilityTypeToString(item->cap));
             ignore_value(virBitmapSetBit(migParams->caps, item->cap));
 
-            if (item->optional) {
-                qemuMigrationCapability opt = item->optional;
+            if (item->optional > QEMU_MIGRATION_OPTCAP_NONE &&
+                item->optional < QEMU_MIGRATION_OPTCAP_LAST) {
+                qemuMigrationOptCap opt = item->optional;
                 ignore_value(virBitmapSetBit(migParams->optional, opt));
                 if (item->party != party)
                     ignore_value(virBitmapSetBit(migParams->remoteOptional, opt));
diff --git a/src/qemu/qemu_migration_params.h b/src/qemu/qemu_migration_params.h
index XXXXXXX..XXXXXXX 100644
--- a/src/qemu/qemu_migration_params.h
+++ b/src/qemu/qemu_migration_params.h
@@ -XXX,XX +XXX,XX @@ typedef enum {
     QEMU_MIGRATION_CAP_BLOCK_DIRTY_BITMAPS,
     QEMU_MIGRATION_CAP_RETURN_PATH,
     QEMU_MIGRATION_CAP_ZERO_COPY_SEND,
-    QEMU_MIGRATION_CAP_POSTCOPY_PREEMPT,
-    QEMU_MIGRATION_CAP_SWITCHOVER_ACK,
 
     QEMU_MIGRATION_CAP_LAST
 } qemuMigrationCapability;
 VIR_ENUM_DECL(qemuMigrationCapability);
 
+typedef enum {
+    QEMU_MIGRATION_OPTCAP_NONE,
+    QEMU_MIGRATION_OPTCAP_POSTCOPY_PREEMPT,
+    QEMU_MIGRATION_OPTCAP_SWITCHOVER_ACK,
+
+    QEMU_MIGRATION_OPTCAP_LAST
+} qemuMigrationOptCap;
+VIR_ENUM_DECL(qemuMigrationOptCap);
+
+
 typedef enum {
     QEMU_MIGRATION_PARAM_COMPRESS_LEVEL,
     QEMU_MIGRATION_PARAM_COMPRESS_THREADS,
-- 
2.34.1

Enum variable of type qemuMigrationCapability is checked for zero in
src/qemu/qemu_migration_params.c:729.

"if (item->optional) { ..."

Actualy, QEMU_MIGRATION_CAP_XBZRLE enum constant has value 0.
So, at least, the condition is incorrect.

Adding QEMU_MIGRATION_CAP_NONE == 0 to enum has several advantages:
- less invasive
- allows comparing with 0
- this approach is wide used in libvirt
- no need to document anything
and only one disadvantage:
- 0-th bit will be reserved (won`t be used) in the corresponding bitmaps.

v1: introducing a separate enum for optional capabilities
v2: another approach: fix only the incorrect condition
v3: third way: add 0-th constanat to enum

Found by Linux Verification Center (linuxtesting.org) with SVACE.

Signed-off-by: Dmitry Frolov <frolov@swemel.ru>
---
 src/qemu/qemu_migration_params.c | 1 +
 src/qemu/qemu_migration_params.h | 1 +
 2 files changed, 2 insertions(+)