[bringing this back to the list, seems like I accidentally hit the single reply button before]
On 29.11.19 21:02, Andy Shevchenko wrote:
On Fri, Nov 29, 2019 at 06:56:44PM +0100, Simon Goldschmidt wrote:
Andy Shevchenko andriy.shevchenko@linux.intel.com schrieb am Fr., 29. Nov. 2019, 18:48:
GCC 9.x starts complaining about potential misalignment of the pointer to the array (in this case alignment=2) in the packed (alignment=1) structures.
Repeating Linus' Torvalds commit 6f303d60534c in the Linux kernel.
Original commit message:
We already did this for clang, but now gcc has that warning too. Yes, yes, the address may be unaligned. And that's kind of the point.
This in particular hides the warnings like
drivers/usb/gadget/composite.c:545:23: warning: taking address of packed member of ‘struct usb_string_descriptor’ may result in an unaligned pointer value [-Waddress-of-packed-member] 545 | collect_langs(sp, s->wData);
Is it really ok to hide this warning? This address is used for an u16 pointer later, so it needs to be aligned. How do we ensure that wData is correctly aligned?
Why should it be?
Because this code is working on a packed struct, which may be unaligned. If it's not, why not remove packing on struct usb_string_descriptor instead of silencing this warning altogether?
It worked before it will work after.
Who guarantees this struct is aligned/will stay aligned in the future?
Most of the platforms I know nowadays do work with unaligned access but are slow, so I think having the compiler warn here is good, no?
I took a different approach and fixed the called function to use byte access:
So, that commit can be reverted then.
I admit this commit increases code size, but I'm not convinced that it's not necessary. If the access is always aligned, let's remove structure packing instead of reducing compiler warnings. (I still think most compiler warnings are good, not bad.)
Regards, Simon