]> xenbits.xensource.com Git - libvirt.git/commitdiff
Revert "maint: prefer enum over int for virstoragefile structs"
authorEric Blake <eblake@redhat.com>
Sat, 17 May 2014 00:50:03 +0000 (18:50 -0600)
committerEric Blake <eblake@redhat.com>
Mon, 19 May 2014 15:00:51 +0000 (09:00 -0600)
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
with a dummy negative declaration in each enum, or cast the
result of virBlahFromString back to int after assigning to an
enum value, or use a temporary int for collecting results from
virBlahFromString, but those actions are all uglier than what we
were trying to cure by directly using enum types for struct
values in the first place.  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.

Signed-off-by: Eric Blake <eblake@redhat.com>
src/conf/domain_conf.c
src/qemu/qemu_command.c
src/qemu/qemu_driver.c
src/util/virstorageencryption.h
src/util/virstoragefile.h

index b5a9a6691e2fc0afd15ab75cadc2491c86d0578a..055b9797a31301654ad420619d0c6c9739065baf 100644 (file)
@@ -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);
index 9ae1a968f3a5f61ef9a37725a6b0d41783a3aaa8..193959f5d158a58e835232eb35dc4bc732bec45b 100644 (file)
@@ -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;
index 55b47552f8e9a0b7df394035795f554179d7fb34..cab653b5d6d0b48da096e098ba2758f99cf855f0 100644 (file)
@@ -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 */
index 0a9bf44c0186ca95990551c07a720692bba5cfd4..f63c9eeb1a0599413ff0e009984fb9c663c39ac7 100644 (file)
@@ -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;
index bdc79440c0a41f316e4d3d4b15f599b879e81024..f875b6999d048fc13e88e7d49923dc25f14c27cd 100644 (file)
@@ -151,7 +151,7 @@ typedef virStorageNetHostDef *virStorageNetHostDefPtr;
 struct _virStorageNetHostDef {
     char *name;
     char *port;
-    virStorageNetHostTransport transport;
+    int transport; /* virStorageNetHostTransport */
     char *socket;  /* path to unix socket */
 };
 
@@ -183,10 +183,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;
 
@@ -209,15 +209,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;