Re: [PATCH v2 1/9] drivers/hv: replace enum hv_message_type by u32

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [KVM ARM]     [KVM ia64]     [KVM ppc]     [Virtualization Tools]     [Spice Development]     [Libvirt]     [Libvirt Users]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite Questions]     [Linux Kernel]     [Linux SCSI]     [XFree86]
  Powered by Linux