Re: [PATCH 31/68] uas: Pack iu struct definitions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Nov 15, 2013 at 03:56:31PM -0000, David Laight wrote:
> > From: Hans de Goede [mailto:hdegoede@xxxxxxxxxx]
> > On 11/15/2013 04:29 PM, David Laight wrote:
> > >> From: Of Hans de Goede
> > >>
> > >> The iu struct definitions are usb packet definitions, so no alignment should
> > >> happen. Notice that assuming 32 bit alignment this does not make any
> > >> difference at all.
> > >>
> > >> Signed-off-by: Hans de Goede <hdegoede@xxxxxxxxxx>
> > >> ---
> > >>   include/linux/usb/uas.h | 10 +++++-----
> > >>   1 file changed, 5 insertions(+), 5 deletions(-)
> > >>
> > >> diff --git a/include/linux/usb/uas.h b/include/linux/usb/uas.h
> > >> index 772b66b..3fc8e8b 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__));
> > >
> > > Won't this cause the compiler to generate multiple byte accesses
> > > for the 'tag' field in systems that don't support misaligned
> > > accesses.
> > 
> > The tag field is aligned to a multiple of its size, so that should not
> > happen, unless some system wants 32 bits alignment for 16 bit variables.
> 
> That isn't going to be true. It might even be against the C standard.
> In any case such a system wouldn't be able to define __be16
> (unless the compiler implemented it using byte accesses).
> 
> ...
> > Without packed struct members will never be mis-aligned in memory as the
> > compiler will simply add padding. packed is used when the struct represent
> > data coming from an outside source, ie a file or a socket, or in this case
> > an usb data packet.
> 
> What would be useful is an attribute that errorred if any implicit padding
> were added.
> 
> If you are worried about padding add a compile-time test on the size of the
> structure.

The packed attribute is consistently used across the USB subsystem for
data that's going out over the wire.  See the note in
include/linux/usb/ch9.h:

 * Note all descriptors are declared '__attribute__((packed))' so that:
 *
 * [a] they never get padded, either internally (USB spec writers
 *     probably handled that) or externally;
 *
 * [b] so that accessing bigger-than-a-bytes fields will never
 *     generate bus errors on any platform, even when the location of
 *     its descriptor inside a bundle isn't "naturally aligned", and
 *
 * [c] for consistency, removing all doubt even when it appears to
 *     someone that the two other points are non-issues for that
 *     particular descriptor type.
 */

It doesn't look like the networking subsystem (which seems to be the
code you're most familiar with) uses packed, but plenty of other
subsystems do.

Sarah Sharp
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux