]> xenbits.xensource.com Git - libvirt.git/commit
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)
commit71bce84a06b9274ef3322f5850df59af331f0249
tree4ab153ba1a00b0884e0143c50c9bdbad15bf7dcb
parent88e36a69de86c2cd172b3838395736f48288f69d
Revert "maint: prefer enum over int for virstoragefile structs"

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