ChangeSet 1.1315.8.22, 2003/09/17 17:09:04-07:00, stern@rowland.harvard.edu [PATCH] USB: Changes to core/config.c (3 of 9) This patch includes a bunch of little local improvements to the code, listed in the patch comments. There are only two notable changes. If a device has more configurations than our maximum, the code doesn't reject the device but simply parses as many configurations as it can and ignores the rest. Likewise, if a configuration contains too many interfaces, the code parses as many as it can and skips the excess. That way such devices will be at least partially useable. Since these limits are arbitrary and set by the implementation (not part of the USB spec), it doesn't make sense to reject a device that violates them. Numerous local programming improvements: Don't initialize to 0 fields in structures that have been memset to 0. Don't constantly keep track of how many bytes are parsed. Use local variables to hold unwieldy values. Remove redundant tests. Allow devices to have more configurations or interfaces than we can handle. drivers/usb/core/config.c | 154 +++++++++++++++++----------------------------- 1 files changed, 60 insertions(+), 94 deletions(-) diff -Nru a/drivers/usb/core/config.c b/drivers/usb/core/config.c --- a/drivers/usb/core/config.c Fri Sep 19 17:10:16 2003 +++ b/drivers/usb/core/config.c Fri Sep 19 17:10:16 2003 @@ -15,19 +15,12 @@ static int usb_parse_endpoint(struct usb_host_endpoint *endpoint, unsigned char *buffer, int size) { + unsigned char *buffer0 = buffer; struct usb_descriptor_header *header; unsigned char *begin; - int parsed = 0, len, numskipped; + int len, numskipped; header = (struct usb_descriptor_header *)buffer; - - /* Everything should be fine being passed into here, but we sanity */ - /* check JIC */ - if (header->bLength > size) { - err("ran out of descriptors parsing"); - return -EINVAL; - } - if (header->bDescriptorType != USB_DT_ENDPOINT) { warn("unexpected descriptor 0x%X, expecting endpoint, 0x%X", header->bDescriptorType, USB_DT_ENDPOINT); @@ -43,7 +36,6 @@ buffer += header->bLength; size -= header->bLength; - parsed += header->bLength; /* Skip over the rest of the Class Specific or Vendor Specific */ /* descriptors */ @@ -64,38 +56,30 @@ (header->bDescriptorType == USB_DT_DEVICE)) break; - dbg("skipping descriptor 0x%X", - header->bDescriptorType); + dbg("skipping descriptor 0x%X", header->bDescriptorType); numskipped++; buffer += header->bLength; size -= header->bLength; - parsed += header->bLength; } - if (numskipped) + if (numskipped) { dbg("skipped %d class/vendor specific endpoint descriptors", numskipped); - /* Copy any unknown descriptors into a storage area for drivers */ - /* to later parse */ - len = (int)(buffer - begin); - if (!len) { - endpoint->extra = NULL; - endpoint->extralen = 0; - return parsed; - } - - endpoint->extra = kmalloc(len, GFP_KERNEL); + /* Copy any unknown descriptors into a storage area for drivers */ + /* to later parse */ + len = buffer - begin; + + endpoint->extra = kmalloc(len, GFP_KERNEL); + if (!endpoint->extra) { + err("couldn't allocate memory for endpoint extra descriptors"); + return -ENOMEM; + } - if (!endpoint->extra) { - err("couldn't allocate memory for endpoint extra descriptors"); - endpoint->extralen = 0; - return parsed; + memcpy(endpoint->extra, begin, len); + endpoint->extralen = len; } - memcpy(endpoint->extra, begin, len); - endpoint->extralen = len; - - return parsed; + return buffer - buffer0; } static void usb_release_intf(struct device *dev) @@ -126,17 +110,15 @@ static int usb_parse_interface(struct usb_interface *interface, unsigned char *buffer, int size) { - int i, len, numskipped, retval, parsed = 0; + unsigned char *buffer0 = buffer; + int i, len, numskipped, retval; struct usb_descriptor_header *header; struct usb_host_interface *ifp; unsigned char *begin; - interface->act_altsetting = 0; - interface->num_altsetting = 0; interface->max_altsetting = USB_ALTSETTINGALLOC; interface->altsetting = kmalloc(sizeof(*interface->altsetting) * interface->max_altsetting, GFP_KERNEL); - if (!interface->altsetting) { err("couldn't kmalloc interface->altsetting"); return -ENOMEM; @@ -168,16 +150,13 @@ } ifp = interface->altsetting + interface->num_altsetting; - ifp->endpoint = NULL; - ifp->extra = NULL; - ifp->extralen = 0; + memset(ifp, 0, sizeof(*ifp)); interface->num_altsetting++; - memcpy(ifp, buffer, USB_DT_INTERFACE_SIZE); + memcpy(&ifp->desc, buffer, USB_DT_INTERFACE_SIZE); /* Skip over the interface */ buffer += ifp->desc.bLength; - parsed += ifp->desc.bLength; size -= ifp->desc.bLength; begin = buffer; @@ -199,25 +178,23 @@ (header->bDescriptorType == USB_DT_DEVICE)) break; + dbg("skipping descriptor 0x%X", header->bDescriptorType); numskipped++; buffer += header->bLength; - parsed += header->bLength; size -= header->bLength; } - if (numskipped) + if (numskipped) { dbg("skipped %d class/vendor specific interface descriptors", numskipped); - /* Copy any unknown descriptors into a storage area for */ - /* drivers to later parse */ - len = (int)(buffer - begin); - if (len) { + /* Copy any unknown descriptors into a storage area for */ + /* drivers to later parse */ + len = buffer - begin; ifp->extra = kmalloc(len, GFP_KERNEL); if (!ifp->extra) { err("couldn't allocate memory for interface extra descriptors"); - ifp->extralen = 0; return -ENOMEM; } memcpy(ifp->extra, begin, len); @@ -229,28 +206,23 @@ if ((size >= sizeof(struct usb_descriptor_header)) && ((header->bDescriptorType == USB_DT_CONFIG) || (header->bDescriptorType == USB_DT_DEVICE))) - return parsed; + break; if (ifp->desc.bNumEndpoints > USB_MAXENDPOINTS) { warn("too many endpoints"); return -EINVAL; } - ifp->endpoint = (struct usb_host_endpoint *) - kmalloc(ifp->desc.bNumEndpoints * - sizeof(struct usb_host_endpoint), GFP_KERNEL); + len = ifp->desc.bNumEndpoints * sizeof(struct usb_host_endpoint); + ifp->endpoint = kmalloc(len, GFP_KERNEL); if (!ifp->endpoint) { err("out of memory"); return -ENOMEM; } - - memset(ifp->endpoint, 0, ifp->desc.bNumEndpoints * - sizeof(struct usb_host_endpoint)); + memset(ifp->endpoint, 0, len); for (i = 0; i < ifp->desc.bNumEndpoints; i++) { - header = (struct usb_descriptor_header *)buffer; - - if (header->bLength > size) { + if (size < USB_DT_ENDPOINT_SIZE) { err("ran out of descriptors parsing"); return -EINVAL; } @@ -260,7 +232,6 @@ return retval; buffer += retval; - parsed += retval; size -= retval; } @@ -269,32 +240,32 @@ if (size < USB_DT_INTERFACE_SIZE || d->bDescriptorType != USB_DT_INTERFACE || !d->bAlternateSetting) - return parsed; + break; } - return parsed; + return buffer - buffer0; } int usb_parse_configuration(struct usb_host_config *config, char *buffer) { + int nintf; int i, size; struct usb_interface *interface; - int retval = -EINVAL; + int retval; struct usb_descriptor_header *header; memcpy(&config->desc, buffer, USB_DT_CONFIG_SIZE); le16_to_cpus(&config->desc.wTotalLength); size = config->desc.wTotalLength; - for (i = 0; i < USB_MAXINTERFACES; ++i) - config->interface[i] = NULL; - - if (config->desc.bNumInterfaces > USB_MAXINTERFACES) { - warn("too many interfaces"); - return -EINVAL; + nintf = config->desc.bNumInterfaces; + if (nintf > USB_MAXINTERFACES) { + warn("too many interfaces (%d max %d)", + nintf, USB_MAXINTERFACES); + config->desc.bNumInterfaces = nintf = USB_MAXINTERFACES; } - for (i = 0; i < config->desc.bNumInterfaces; ++i) { + for (i = 0; i < nintf; ++i) { interface = config->interface[i] = kmalloc(sizeof(struct usb_interface), GFP_KERNEL); dbg("kmalloc IF %p, numif %i", interface, i); @@ -313,10 +284,7 @@ buffer += config->desc.bLength; size -= config->desc.bLength; - config->extra = NULL; - config->extralen = 0; - - for (i = 0; i < config->desc.bNumInterfaces; i++) { + for (i = 0; i < nintf; i++) { int numskipped, len; char *begin; @@ -345,20 +313,18 @@ buffer += header->bLength; size -= header->bLength; } - if (numskipped) - dbg("skipped %d class/vendor specific endpoint descriptors", numskipped); + if (numskipped) { + dbg("skipped %d class/vendor specific configuration descriptors", numskipped); - /* Copy any unknown descriptors into a storage area for */ - /* drivers to later parse */ - len = (int)(buffer - begin); - if (len) { + /* Copy any unknown descriptors into a storage area for */ + /* drivers to later parse */ + len = buffer - begin; if (config->extralen) { warn("extra config descriptor"); } else { config->extra = kmalloc(len, GFP_KERNEL); if (!config->extra) { err("couldn't allocate memory for config extra descriptors"); - config->extralen = 0; return -ENOMEM; } @@ -413,39 +379,39 @@ // (used by real hubs and virtual root hubs) int usb_get_configuration(struct usb_device *dev) { + int ncfg = dev->descriptor.bNumConfigurations; int result; unsigned int cfgno, length; unsigned char *buffer; unsigned char *bigbuffer; struct usb_config_descriptor *desc; - if (dev->descriptor.bNumConfigurations > USB_MAXCONFIG) { - warn("too many configurations"); - return -EINVAL; + if (ncfg > USB_MAXCONFIG) { + warn("too many configurations (%d max %d)", + ncfg, USB_MAXCONFIG); + dev->descriptor.bNumConfigurations = ncfg = USB_MAXCONFIG; } - if (dev->descriptor.bNumConfigurations < 1) { - warn("not enough configurations"); + if (ncfg < 1) { + warn("no configurations"); return -EINVAL; } - dev->config = (struct usb_host_config *) - kmalloc(dev->descriptor.bNumConfigurations * - sizeof(struct usb_host_config), GFP_KERNEL); + length = ncfg * sizeof(struct usb_host_config); + dev->config = kmalloc(length, GFP_KERNEL); if (!dev->config) { err("out of memory"); return -ENOMEM; } - memset(dev->config, 0, dev->descriptor.bNumConfigurations * - sizeof(struct usb_host_config)); + memset(dev->config, 0, length); - dev->rawdescriptors = (char **)kmalloc(sizeof(char *) * - dev->descriptor.bNumConfigurations, GFP_KERNEL); + length = ncfg * sizeof(char *); + dev->rawdescriptors = kmalloc(length, GFP_KERNEL); if (!dev->rawdescriptors) { err("out of memory"); return -ENOMEM; } - memset(dev->rawdescriptors, 0, sizeof(char *) * dev->descriptor.bNumConfigurations); + memset(dev->rawdescriptors, 0, length); buffer = kmalloc(8, GFP_KERNEL); if (!buffer) { @@ -454,7 +420,7 @@ } desc = (struct usb_config_descriptor *)buffer; - for (cfgno = 0; cfgno < dev->descriptor.bNumConfigurations; cfgno++) { + for (cfgno = 0; cfgno < ncfg; cfgno++) { /* We grab the first 8 bytes so we know how long the whole */ /* configuration is */ result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, buffer, 8);