This partially reverts commits b279e52f7 and ea18f8b2. It turns out our code base is full of: if ((struct.member = virBlahFromString(str)) < 0) goto error; Meanwhile, the C standard says it is up to the compiler whether an enum is signed or unsigned when all of its declared values happen to be positive. In my testing (Fedora 20, gcc 4.8.2), the compiler picked signed, and nothing changed. But others testing with gcc 4.7 got compiler warnings, because it picked the enum to be unsigned, but no unsigned value is less than 0. Even worse: if ((struct.member = virBlahFromString(str)) <= 0) goto error; is silently compiled without warning, but incorrectly treats -1 from a bad parse as a large positive number with no warning; and without the compiler's help to find these instances, it is a nightmare to maintain correctly. We could force signed enums a dummy negative declaration in each enum, or cast the result of virBlahFromString back to int after assigning to an enum value, but that's uglier than what we were trying to cure by directly using enum types for struct values. It's better off to just live with int members, and use 'switch ((virFoo) struct.member)' where we want the compiler to help, than to track down all the conversions from string to enum and ensure they don't suffer from type problems. * src/util/virstorageencryption.h: Revert back to int declarations with comment about enum usage. * src/util/virstoragefile.h: Likewise. * src/conf/domain_conf.c: Restore back to casts in switches. * src/qemu/qemu_driver.c: Likewise. * src/qemu/qemu_command.c: Add cast rather than revert. --- src/conf/domain_conf.c | 4 ++-- src/qemu/qemu_command.c | 2 +- src/qemu/qemu_driver.c | 8 ++++---- src/util/virstorageencryption.h | 4 ++-- src/util/virstoragefile.h | 14 +++++++------- 5 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index e5ae7c6..e65b62b 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -4972,7 +4972,7 @@ virDomainDiskSourceParse(xmlNodePtr node, memset(&host, 0, sizeof(host)); - switch (src->type) { + switch ((virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: src->path = virXMLPropString(node, "file"); break; @@ -14847,7 +14847,7 @@ virDomainDiskSourceFormat(virBufferPtr buf, startupPolicy = virDomainStartupPolicyTypeToString(policy); if (src->path || src->nhosts > 0 || src->srcpool || startupPolicy) { - switch (src->type) { + switch ((virStorageType)src->type) { case VIR_STORAGE_TYPE_FILE: virBufferAddLit(buf, "<source"); virBufferEscapeString(buf, " file='%s'", src->path); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 9ae1a96..193959f 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -11021,7 +11021,7 @@ qemuParseCommandLine(virCapsPtr qemuCaps, if (disk->src.type == VIR_STORAGE_TYPE_NETWORK) { char *port; - switch (disk->src.protocol) { + switch ((virStorageNetProtocol) disk->src.protocol) { case VIR_STORAGE_NET_PROTOCOL_NBD: if (qemuParseNBDString(disk) < 0) goto error; diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 55b4755..cab653b 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -12368,7 +12368,7 @@ qemuDomainSnapshotPrepareDiskExternalBackingInactive(virDomainDiskDefPtr disk) return 0; case VIR_STORAGE_TYPE_NETWORK: - switch (disk->src.protocol) { + switch ((virStorageNetProtocol) disk->src.protocol) { case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -12430,7 +12430,7 @@ qemuDomainSnapshotPrepareDiskExternalOverlayActive(virDomainSnapshotDiskDefPtr d return 0; case VIR_STORAGE_TYPE_NETWORK: - switch (disk->src.protocol) { + switch ((virStorageNetProtocol) disk->src.protocol) { case VIR_STORAGE_NET_PROTOCOL_GLUSTER: return 0; @@ -12575,7 +12575,7 @@ qemuDomainSnapshotPrepareDiskInternal(virConnectPtr conn, return 0; case VIR_STORAGE_TYPE_NETWORK: - switch (disk->src.protocol) { + switch ((virStorageNetProtocol) disk->src.protocol) { case VIR_STORAGE_NET_PROTOCOL_NBD: case VIR_STORAGE_NET_PROTOCOL_RBD: case VIR_STORAGE_NET_PROTOCOL_SHEEPDOG: @@ -12801,7 +12801,7 @@ qemuDomainSnapshotCreateSingleDiskActive(virQEMUDriverPtr driver, VIR_STRDUP(persistSource, snap->src.path) < 0) goto cleanup; - switch (snap->src.type) { + switch ((virStorageType)snap->src.type) { case VIR_STORAGE_TYPE_BLOCK: reuse = true; /* fallthrough */ diff --git a/src/util/virstorageencryption.h b/src/util/virstorageencryption.h index 0a9bf44..f63c9ee 100644 --- a/src/util/virstorageencryption.h +++ b/src/util/virstorageencryption.h @@ -39,7 +39,7 @@ VIR_ENUM_DECL(virStorageEncryptionSecret) typedef struct _virStorageEncryptionSecret virStorageEncryptionSecret; typedef virStorageEncryptionSecret *virStorageEncryptionSecretPtr; struct _virStorageEncryptionSecret { - virStorageEncryptionSecretType type; + int type; /* virStorageEncryptionSecretType */ unsigned char uuid[VIR_UUID_BUFLEN]; }; @@ -55,7 +55,7 @@ VIR_ENUM_DECL(virStorageEncryptionFormat) typedef struct _virStorageEncryption virStorageEncryption; typedef virStorageEncryption *virStorageEncryptionPtr; struct _virStorageEncryption { - virStorageEncryptionFormatType format; + int format; /* virStorageEncryptionFormatType */ size_t nsecrets; virStorageEncryptionSecretPtr *secrets; diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h index 0a19603..ed7ab1b 100644 --- a/src/util/virstoragefile.h +++ b/src/util/virstoragefile.h @@ -150,7 +150,7 @@ typedef virStorageNetHostDef *virStorageNetHostDefPtr; struct _virStorageNetHostDef { char *name; char *port; - virStorageNetHostTransport transport; + int transport; /* virStorageNetHostTransport */ char *socket; /* path to unix socket */ }; @@ -182,10 +182,10 @@ typedef struct _virStorageSourcePoolDef virStorageSourcePoolDef; struct _virStorageSourcePoolDef { char *pool; /* pool name */ char *volume; /* volume name */ - virStorageVolType voltype; /* internal only */ + int voltype; /* virStorageVolType, internal only */ int pooltype; /* virStoragePoolType from storage_conf.h, internal only */ - virStorageType actualtype; /* internal only */ - virStorageSourcePoolMode mode; + int actualtype; /* virStorageType, internal only */ + int mode; /* virStorageSourcePoolMode */ }; typedef virStorageSourcePoolDef *virStorageSourcePoolDefPtr; @@ -208,15 +208,15 @@ typedef virStorageSource *virStorageSourcePtr; * backing chains, multiple source disks join to form a single guest * view. */ struct _virStorageSource { - virStorageType type; + int type; /* virStorageType */ char *path; - virStorageNetProtocol protocol; + int protocol; /* virStorageNetProtocol */ size_t nhosts; virStorageNetHostDefPtr hosts; virStorageSourcePoolDefPtr srcpool; struct { char *username; - virStorageSecretType secretType; + int secretType; /* virStorageSecretType */ union { unsigned char uuid[VIR_UUID_BUFLEN]; char *usage; -- 1.9.0