Re: [qemu-s390x] [PATCH 10/14] usb-mtp: avoid warning about unaligned access to filename

[Daniel P . Berrangé]

On Fri, Mar 29, 2019 at 12:25:54PM +0100, Thomas Huth wrote:
> On 29/03/2019 12.11, Daniel P. Berrangé wrote:
> > The 'filename' field in ObjectInfo struct is declared as a
> > zero length array of uint16_t. Accessing it is equivalent
> > to taking the address of the field, and taking the address
> > of fields in a packed struct causes unaligned pointer
> > warnings:
> > 
> > hw/usb/dev-mtp.c: In function ‘usb_mtp_write_metadata’:
> > hw/usb/dev-mtp.c:1712:36: warning: taking address of packed member of 
> > ‘struct <anonymous>’ may result in an unaligned pointer value 
> > [-Waddress-of-packed-member]
> >  1712 |                             dataset->filename);
> >       |                             ~~~~~~~^~~~~~~~~~
> > 
> > The warning is in fact correct because the 'filename'
> > field is preceeded by a uint8_t field which causes it
> > to have bad alignment.
> > 
> > Using pointer arithmetic instead of accessing the zero
> > length array field directly avoids the compiler warning
> > but doesn't ultimately fix the bad alignment. Fixing
> > that probably requires allocating a new array of
> > uint16_t in the heap & then memcpy() the data before
> > accessing the array elements.
> 
> If we are really using an unaligned pointer here, this code will crash
> on Sparc host machines (and maybe some others). Thus I'd prefer if you
> could rather fix the misalignment issue here instead of papering over
> the compiler warning.

Yeah, I wasn't sure if uint16 mis-alignment would cause problems or
not in practice, so left this comment in the commit message. If you
expect this really will crash, then I guess we have to go the memcpy
to temporary buffer route.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|