From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:60545) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4uJM-0003UH-Hq for qemu-devel@nongnu.org; Fri, 04 Dec 2015 12:38:25 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4uJJ-00031W-7x for qemu-devel@nongnu.org; Fri, 04 Dec 2015 12:38:24 -0500 Received: from mx1.redhat.com ([209.132.183.28]:44145) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4uJJ-00031R-2x for qemu-devel@nongnu.org; Fri, 04 Dec 2015 12:38:21 -0500 References: <1448900541-19939-1-git-send-email-asmetanin@virtuozzo.com> <1448900541-19939-2-git-send-email-asmetanin@virtuozzo.com> <565EE292.7010004@redhat.com> <5661A44E.1040503@openvz.org> <5661A610.5050401@redhat.com> <5661C566.8000605@openvz.org> From: Paolo Bonzini Message-ID: <5661CF80.7080704@redhat.com> Date: Fri, 4 Dec 2015 18:38:08 +0100 MIME-Version: 1.0 In-Reply-To: <5661C566.8000605@openvz.org> Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32 List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: "Denis V. Lunev" , Andrey Smetanin , kvm@vger.kernel.org, Vitaly Kuznetsov Cc: Gleb Natapov , Haiyang Zhang , "K. Y. Srinivasan" , Roman Kagan , qemu-devel@nongnu.org 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