[Spice-devel] [PATCH spice-protocol 1/2 v2] qxl_dev: Fix alignment for QXLReleaseInfo

[Snir Sheriber]

On 7/18/19 5:51 PM, Frediano Ziglio wrote:
>> Hi,
>>
>>
>> IIRC this was related to some compiler warning, no?
> Yes, recent compilers are reporting it, see below.
>
>> If it is I'd mentioning it , otherwise, ack.
>>
> Just this patch or the entire series?


No, actually i started looking at the second one and wondered why
we are using #include end/start-packed.h

It is mentioned in start-packed.h it's because "its not possible to put 
pragmas into header files"
and just after that we put pragma, into this header file.
What am i missing? or the comment is wrong?


>
>> Snir.
>>
>>
>> On 7/4/19 4:56 PM, Frediano Ziglio wrote:
>>> Do not declare the structure as aligned.
>>> The start/end-packed.h headers affects structures without
>>> specification only using MingW or Microsoft compilers. For other
>>> platform SPICE_ATTR_PACKED macro should be used.  This way the
>>> definition are the same for all compiler.
>>> This structure is used in a lot of QXL structures which are not
>>> aligned causing to have an aligned structure to be potentially
>>> unaligned.
> What about changing this paragraph to:
>
> "This structure is used in a lot of QXL structures which are not
>   aligned causing to have an aligned structure to be potentially
>   unaligned. Some compilers report a warning for some usage."


Not a big deal but maybe "Some compilers may report a warning"?

Anyway, ack for the change.

>
>>> As this structure has no holes this change does not make any size
>>> change using any compiler.
>>> The change will only change the alignment from 4/8 to 1.
>>> This could affect structures containing this union however beside
>>> packed structure in qxl_dev.h (which are not affected) there are no
>>> other usages affecting ABI by spice-gtk, Qemu or spice-server.
>>>
>>> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
>>> ---
>>> Changes since v1:
>>> - update commit message
>>> ---
>>>    spice/qxl_dev.h | 2 +-
>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/spice/qxl_dev.h b/spice/qxl_dev.h
>>> index a9cc4f4..659f930 100644
>>> --- a/spice/qxl_dev.h
>>> +++ b/spice/qxl_dev.h
>>> @@ -275,7 +275,7 @@ typedef struct SPICE_ATTR_ALIGNED(4) SPICE_ATTR_PACKED
>>> QXLRam {
>>>    
>>>    } QXLRam;
>>>    
>>> -typedef union QXLReleaseInfo {
>>> +typedef union SPICE_ATTR_PACKED QXLReleaseInfo {
>>>        uint64_t id;      // in
>>>        uint64_t next;    // out
>>>    } QXLReleaseInfo;
> Frediano