On Fri, 2 Feb 2007 17:32:24 -0800, Inaky Perez-Gonzalez <[EMAIL PROTECTED]>
wrote:
> usb: descriptor structures have to be packed
> Many of the Wireless USB decriptors added to usb_ch9.h don't have the
> __attribute__((packed)) tag, and thus, they don't reflect the wire
> size. This patch fixes that.
I am not cool with this.
> +++ b/include/linux/usb_ch9.h Fri Feb 02 17:17:09 2007 -0800
> @@ -366,7 +366,7 @@ struct usb_debug_descriptor {
> /* bulk endpoints with 8 byte maxpacket */
> __u8 bDebugInEndpoint;
> __u8 bDebugOutEndpoint;
> -};
> +} __attribute__((packed));
This is a 4-byte structure. The attribute is unnecessary.
> @@ -395,7 +395,7 @@ struct usb_security_descriptor {
>
> __le16 wTotalLength;
> __u8 bNumEncryptionTypes;
> -};
> +} __attribute__((packed));
This size is odd, but see below.
> @@ -409,7 +409,7 @@ struct usb_key_descriptor {
> __u8 tTKID[3];
> __u8 bReserved;
> __u8 bKeyData[0];
> -};
> +} __attribute__((packed));
This structure has no members larger than a byte, so the attribute
is unnecessary.
> @@ -425,7 +425,7 @@ struct usb_encryption_descriptor {
> #define USB_ENC_TYPE_RSA_1 3 /* rsa3072/sha1 auth */
> __u8 bEncryptionValue; /* use in SET_ENCRYPTION */
> __u8 bAuthKeyIndex;
> -};
> +} __attribute__((packed));
Ditto.
> @@ -437,7 +437,7 @@ struct usb_bos_descriptor {
>
> __le16 wTotalLength;
> __u8 bNumDeviceCaps;
> -};
> +} __attribute__((packed));
This is the same as usb_security_descriptor.
> @@ -446,7 +446,7 @@ struct usb_dev_cap_header {
> __u8 bLength;
> __u8 bDescriptorType;
> __u8 bDevCapabilityType;
> -};
> +} __attribute__((packed));
This one might need the attribute because it's smaller than 4 bytes.
> @@ -474,7 +474,7 @@ struct usb_wireless_cap_descriptor { /*
> __u8 bmFFITXPowerInfo; /* FFI power levels */
> __le16 bmBandGroup;
> __u8 bReserved;
> -};
> +} __attribute__((packed));
This is the same as usb_security_descriptor.
> @@ -495,7 +495,7 @@ struct usb_wireless_ep_comp_descriptor {
> #define USB_ENDPOINT_SWITCH_NO 0
> #define USB_ENDPOINT_SWITCH_SWITCH 1
> #define USB_ENDPOINT_SWITCH_SCALE 2
> -};
> +} __attribute__((packed));
This descriptor is naturally aligned, attribute is unnecessary.
> @@ -511,7 +511,7 @@ struct usb_handshake {
> __u8 CDID[16];
> __u8 nonce[16];
> __u8 MIC[8];
> -};
> +} __attribute__((packed));
Byte sized members only, attribute is unnecessary.
> @@ -523,7 +523,7 @@ struct usb_connection_context {
> __u8 CHID[16]; /* persistent host id */
> __u8 CDID[16]; /* device id (unique w/in host context) */
> __u8 CK[16]; /* connection key */
> -};
> +} __attribute__((packed));
Byte sized members only, attribute is unnecessary.
So, this leaves usb_security_descriptor, usb_bos_descriptor,
usb_wireless_cap_descriptor, and usb_dev_cap_header. IMHO, our original
setup was the correct one. The code should have used sizes such as
USB_DT_CONFIG_SIZE instead of sizeof(). It's elementary. The use of
__attribute__((packed)) adds nontrivial costs... on some architectures.
I think the funniest part of this is that the architecture which suffers
most from the abuse of __attribute__((packed)) is ia64 (with sparc being
the second). And yet, Inaky works for Intel. Do we have to take this as
a sign that Intel is abandoning Itanium? Poor, poor SGI. ^_^
I am not going to raise a stink myself, but I would like to point out
the obvious. The packed attributes were added by people who do not
care about non-x86, because the cost of __attribute__((packed)) is
miniscule there (although, not zero either). The matter went so far
that even usb_config_descriptor has the attribute now, even though
it always had a perfectly useable size define.
-- Pete
-------------------------------------------------------------------------
Using Tomcat but need to do more? Need to support web services, security?
Get stuff done quickly with pre-integrated technology to make your job easier.
Download IBM WebSphere Application Server v.1.0.1 based on Apache Geronimo
http://sel.as-us.falkag.net/sel?cmd=lnk&kid=120709&bid=263057&dat=121642
_______________________________________________
[email protected]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
