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.

No, Linux relies on the compiler to pack things like this together and
not put padding in, with the __packed__ attribute (don't we have a
__packed macro instead of the __attribute__() thing?)

So this is correct, if it changes, then huge swatches of Linux kernel
code will break.

Oh, and gcc, llvm, and icc all do things this way, and any new c
compiler will do it as well if they want to build the kernel.

greg k-h
--
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