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

[Paolo Bonzini <pbonzini@redhat.com>]

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.

Paolo

> Vitaly, by the way, this code is a bit rotten. The only caller of
> hv_post_message calls it with message type exactly written
> as "1", which is not defined in the enum.
> 
> /*
>  * vmbus_post_msg - Send a msg on the vmbus's message connection
>  */
> int vmbus_post_msg(void *buffer, size_t buflen)
> {
>     ...
>     ret = hv_post_message(conn_id, 1, buffer, buflen);
> 
> Den