Re: [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32

[Paolo Bonzini]

On 04/12/2015 17:55, Denis V. Lunev wrote:
> On 12/04/2015 05:41 PM, Paolo Bonzini wrote:
>>
>> On 04/12/2015 15:33, Denis V. Lunev wrote:
>>> On 12/02/2015 03:22 PM, Paolo Bonzini wrote:
>>>> On 30/11/2015 17:22, Andrey Smetanin wrote:
>>>>> enum hv_message_type inside struct hv_message, hv_post_message
>>>>> is not size portable. Replace enum by u32.
>>>> It's only non-portable inside structs.  Okay to apply just these:
>>>>
>>>> @@ -172,7 +174,7 @@ union hv_message_flags {
>>>>
>>>>    /* Define synthetic interrupt controller message header. */
>>>>    struct hv_message_header {
>>>> -    u32 message_type;
>>>> +    enum hv_message_type message_type;
>>>>        u8 payload_size;
>>>>        union hv_message_flags message_flags;
>>>>        u8 reserved[2];
>>>> @@ -345,7 +347,7 @@ enum hv_call_code {
>>>>    struct hv_input_post_message {
>>>>        union hv_connection_id connectionid;
>>>>        u32 reserved;
>>>> -    u32 message_type;
>>>> +    enum hv_message_type message_type;
>>>>        u32 payload_size;
>>>>        u64 payload[HV_MESSAGE_PAYLOAD_QWORD_COUNT];
>>>>    };
>>>>
>>>> ?
>>>>
>>>> Paolo
>>> sorry for the delay.
>>>
>>> Andrey is on vacation till the end of the week.
>>>
>>> This could be not enough for some compilers as this exact
>>> enum could be signed and unsigned depends upon the
>>> implementation of the compiler and if it is signed we
>>> can face signed/unsigned comparison in ifs.
>> But why is that a problem?  The issue is pre-existing anyway; the only
>> one that can cause bugs when moving code to uapi/ (i.e. which means it
>> can be used on non-x86 platforms) is the size of the enum, not the
>> signedness.
>
> we are now comparing enum with enum which are the same type.
> With the change you are proposing we will compare enum
> with u32 which are different.

This is only an issue in C++.

> Original suggestion from Andrey was safe in this respect.

Sure, but it makes code less clear.

Paolo