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
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
