[libvirt] [PATCH] Revert "maint: prefer enum over int for virstoragefile structs"

[Roman Bogorodskiy]

  Eric Blake wrote:

> 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:

This problem happens with clang as well (at least with clang 3.4).

> 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(-)

This patch fixes the problem for me and the approach looks good,
as other workarounds look quite ugly indeed.

Roman Bogorodskiy
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 488 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140517/8b269470/attachment-0001.sig>