[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32 |
Date: |
Fri, 4 Dec 2015 15:41:20 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.3.0 |
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