On Fri, 7 Mar 2014, Hans de Goede wrote:
> >> diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h
> >> index 772b66bcdd7d..3fc8e8b9f043 100644
> >> --- a/include/linux/usb/uas.h
> >> +++ b/include/linux/usb/uas.h
> >> @@ -9,7 +9,7 @@ struct iu {
> >> __u8 iu_id;
> >> __u8 rsvd1;
> >> __be16 tag;
> >> -};
> >> +} __attribute__((__packed__));
> >
> > I don't want to block these patches, but the above will make
> > a difference.
> >
> > On cpus that don't support misaligned memory transfers the
> > compiler will have to generate code that does byte accesses
> > and shifts in order to access 16 and 32 bit members of packed
> > structures.
> >
> > So you should really only mark structures as packed if they
> > will occur on misaligned boundaries.
>
> This is not about alignment, this is about holes in the
> structure not being allowed, as it represents on the wire data
> which has no holes in it.
>
> Since we cannot know in advance what the alignment rules for
> members inside structures on new archs will be, the only
> way to ensure this is to pack the struct.
Is that really true? I mean, does it seem at all reasonable that _any_
architecture in the future will align __u8 values on something stricter
than byte boundaries? Or __be16 values on something other than 2-byte
boundaries?
There's a second aspect to this: Can we ever have an instance of this
structure starting at a non-aligned address? For instances that are
created in memory, the answer is No. But for instances that are
embedded in data received in a USB packet, the answer is Yes -- at
least, in principle. (Maybe in practice some particular structures
such as usb_ctrlrequest only ever occur at the start of a packet and so
are properly aligned, but I wouldn't want to depend on this.)
> Note that *ALL* usb device drivers do the same.
That doesn't mean they are necessarily right...
Alan Stern
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
