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