Re: [PATCH] hw/usb: Use __attribute__((packed)) vs __packed

[Thomas Huth <thuth@redhat.com>]

On 21/11/2024 11.01, Michael Tokarev wrote:
> 04.11.2024 10:37, Thomas Huth wrote:
>> On 01/11/2024 22.17, Roque Arcudia Hernandez wrote:
>>> __packed is non standard and is not present in clang-cl.
>>> __attribute__((packed)) has the same semantics.
>>>
>>> Signed-off-by: Erwin Jansen <jansene@google.com>
>>> Signed-off-by: Roque Arcudia Hernandez <roqueh@google.com>
>>> ---
>>>   include/hw/usb/dwc2-regs.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/hw/usb/dwc2-regs.h b/include/hw/usb/dwc2-regs.h
>>> index 523b112c5e..b8b4266543 100644
>>> --- a/include/hw/usb/dwc2-regs.h
>>> +++ b/include/hw/usb/dwc2-regs.h
>>> @@ -838,7 +838,7 @@
>>>   struct dwc2_dma_desc {
>>>           uint32_t status;
>>>           uint32_t buf;
>>> -} __packed;
>>> +} QEMU_PACKED;
>>
>> Reviewed-by: Thomas Huth <thuth@redhat.com>
>>
>> Actually, the struct only consists of two 32-bit values, so I doubt that 
>> the "packed" is needed here at all. Maybe we could even simply remove it?
> To me it is important to mark structures as packed if it is
> important for them to have strict layout like in this case,
> even if de-facto it does not change the actual layout.  It's
> just like an annotation saying this structure can be used
> on wire or somesuch.

Well, it can have an impact on your binary, too. On architectures that don't 
support unaligned memory accesses, the compiler has to generate code that 
reads the values of packed structures in a more cumbersome way. So if you 
used "packed" though it's not really necessary (i.e. everything is naturally 
aligned, and you can be sure that the starting address is also properly 
aligned), the compiler generates worse code than necessary in that case.

  Thomas