From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37574) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4ufB-0006ta-C9 for qemu-devel@nongnu.org; Fri, 04 Dec 2015 13:00:58 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1a4uf6-0002QV-78 for qemu-devel@nongnu.org; Fri, 04 Dec 2015 13:00:57 -0500 Received: from relay.parallels.com ([195.214.232.42]:42675) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1a4uf5-0002QR-Vn for qemu-devel@nongnu.org; Fri, 04 Dec 2015 13:00:52 -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> <5661CF80.7080704@redhat.com> From: "Denis V. Lunev" Message-ID: <5661D4BB.50206@openvz.org> Date: Fri, 4 Dec 2015 21:00:27 +0300 MIME-Version: 1.0 In-Reply-To: <5661CF80.7080704@redhat.com> Content-Type: text/plain; charset="windows-1252"; format=flowed 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: Paolo Bonzini , Andrey Smetanin , kvm@vger.kernel.org, Vitaly Kuznetsov Cc: Gleb Natapov , Haiyang Zhang , "K. Y. Srinivasan" , Roman Kagan , qemu-devel@nongnu.org On 12/04/2015 08:38 PM, Paolo Bonzini wrote: > > 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 ok, this seems reasonable. Why not to reduce the patch :) We'll send an update on Monday. Are there any other issue with the patchset? Den