qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: "Denis V. Lunev" <den@openvz.org>
To: Paolo Bonzini <pbonzini@redhat.com>,
	Andrey Smetanin <asmetanin@virtuozzo.com>,
	kvm@vger.kernel.org, Vitaly Kuznetsov <vkuznets@redhat.com>
Cc: Gleb Natapov <gleb@kernel.org>,
	Haiyang Zhang <haiyangz@microsoft.com>,
	"K. Y. Srinivasan" <kys@microsoft.com>,
	Roman Kagan <rkagan@virtuozzo.com>,
	qemu-devel@nongnu.org
Subject: Re: [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32
Date: Fri, 4 Dec 2015 19:55:02 +0300	[thread overview]
Message-ID: <5661C566.8000605@openvz.org> (raw)
In-Reply-To: <5661A610.5050401@redhat.com>

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.
>
> Paolo
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.

Original suggestion from Andrey was safe in this respect.

Den

  reply	other threads:[~2015-12-04 16:55 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-11-30 16:22 [Qemu-devel] [PATCH v2 0/9] KVM: Hyper-V SynIC timers Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32 Andrey Smetanin
2015-12-02 12:22   ` Paolo Bonzini
2015-12-04 14:33     ` Denis V. Lunev
2015-12-04 14:41       ` Paolo Bonzini
2015-12-04 16:55         ` Denis V. Lunev [this message]
2015-12-04 17:38           ` Paolo Bonzini
2015-12-04 18:00             ` Denis V. Lunev
2015-12-04 18:12               ` Paolo Bonzini
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 2/9] drivers/hv: Move HV_SYNIC_STIMER_COUNT into Hyper-V UAPI x86 header Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 3/9] drivers/hv: Move struct hv_message into UAPI Hyper-V " Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 4/9] drivers/hv: Move struct hv_timer_message_payload " Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 5/9] kvm/x86: Rearrange func's declarations inside Hyper-V header Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 6/9] kvm/x86: Added Hyper-V vcpu_to_hv_vcpu()/hv_vcpu_to_vcpu() helpers Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 7/9] kvm/x86: Hyper-V internal helper to read MSR HV_X64_MSR_TIME_REF_COUNT Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 8/9] kvm/x86: Hyper-V SynIC message slot pending clearing at SINT ack Andrey Smetanin
2015-11-30 16:22 ` [Qemu-devel] [PATCH v2 9/9] kvm/x86: Hyper-V SynIC timers Andrey Smetanin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=5661C566.8000605@openvz.org \
    --to=den@openvz.org \
    --cc=asmetanin@virtuozzo.com \
    --cc=gleb@kernel.org \
    --cc=haiyangz@microsoft.com \
    --cc=kvm@vger.kernel.org \
    --cc=kys@microsoft.com \
    --cc=pbonzini@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rkagan@virtuozzo.com \
    --cc=vkuznets@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).