ChangeSet 1.1673.8.8, 2004/03/25 15:16:15-08:00, stern@rowland.harvard.edu [PATCH] USB: Improve core/config.c error messages This patch improves error reporting in the configuration parsing routines. It also adds a few extra minor tweaks. #include linux/config.h and make the usual DEBUG settings available. Use the driver-model dev_xxx() macros for log output. Be much more explicit about the nature of errors, including configuration, interface, and altsetting numbers where appropriate. Log fatal problems as errors, non-fatal ones as warnings. Remove a #define'd constant that is already set in linux/usb.h. Fix some variables declared as pointer to char that really should be pointers to unsigned char. Replace a whole bunch of "out-of-memory" error messages with a single message. Wrap source lines that are longer than 80 columns (but not log output lines!). Clean up the logic for detecting errors when retrieving a configuration descriptor. Apart from the log messages themselves, this introduces no functional changes. drivers/usb/core/config.c | 225 ++++++++++++++++++++++++++-------------------- 1 files changed, 132 insertions(+), 93 deletions(-) diff -Nru a/drivers/usb/core/config.c b/drivers/usb/core/config.c --- a/drivers/usb/core/config.c Wed Apr 14 14:39:47 2004 +++ b/drivers/usb/core/config.c Wed Apr 14 14:39:47 2004 @@ -1,18 +1,25 @@ +#include + +#ifdef CONFIG_USB_DEBUG +#define DEBUG +#endif + #include #include #include #include +#include #include #define USB_MAXALTSETTING 128 /* Hard limit */ #define USB_MAXENDPOINTS 30 /* Hard limit */ -/* these maximums are arbitrary */ -#define USB_MAXCONFIG 8 -#define USB_MAXINTERFACES 32 +#define USB_MAXCONFIG 8 /* Arbitrary limit */ -static int usb_parse_endpoint(struct usb_host_endpoint *endpoint, unsigned char *buffer, int size) +static int usb_parse_endpoint(struct device *ddev, int cfgno, int inum, + int asnum, struct usb_host_endpoint *endpoint, + unsigned char *buffer, int size) { unsigned char *buffer0 = buffer; struct usb_descriptor_header *header; @@ -21,8 +28,11 @@ header = (struct usb_descriptor_header *)buffer; if (header->bDescriptorType != USB_DT_ENDPOINT) { - warn("unexpected descriptor 0x%X, expecting endpoint, 0x%X", - header->bDescriptorType, USB_DT_ENDPOINT); + dev_err(ddev, "config %d interface %d altsetting %d has an " + "unexpected descriptor of type 0x%X, " + "expecting endpoint type 0x%X\n", + cfgno, inum, asnum, + header->bDescriptorType, USB_DT_ENDPOINT); return -EINVAL; } @@ -31,13 +41,16 @@ else if (header->bLength >= USB_DT_ENDPOINT_SIZE) memcpy(&endpoint->desc, buffer, USB_DT_ENDPOINT_SIZE); else { - warn("invalid endpoint descriptor"); + dev_err(ddev, "config %d interface %d altsetting %d has an " + "invalid endpoint descriptor of length %d\n", + cfgno, inum, asnum, header->bLength); return -EINVAL; } if ((endpoint->desc.bEndpointAddress & ~USB_ENDPOINT_DIR_MASK) >= 16) { - warn("invalid endpoint address 0x%X", - endpoint->desc.bEndpointAddress); + dev_err(ddev, "config %d interface %d altsetting %d has an " + "invalid endpoint with address 0x%X\n", + cfgno, inum, asnum, endpoint->desc.bEndpointAddress); return -EINVAL; } @@ -57,14 +70,16 @@ (header->bDescriptorType == USB_DT_INTERFACE)) break; - dbg("skipping descriptor 0x%X", header->bDescriptorType); + dev_dbg(ddev, "skipping descriptor 0x%X\n", + header->bDescriptorType); numskipped++; buffer += header->bLength; size -= header->bLength; } if (numskipped) { - dbg("skipped %d class/vendor specific endpoint descriptors", numskipped); + dev_dbg(ddev, "skipped %d class/vendor specific endpoint " + "descriptors\n", numskipped); endpoint->extra = begin; endpoint->extralen = buffer - begin; } @@ -87,7 +102,8 @@ kfree(intf); } -static int usb_parse_interface(struct usb_host_config *config, unsigned char *buffer, int size) +static int usb_parse_interface(struct device *ddev, int cfgno, + struct usb_host_config *config, unsigned char *buffer, int size) { unsigned char *buffer0 = buffer; struct usb_interface_descriptor *d; @@ -101,8 +117,9 @@ d = (struct usb_interface_descriptor *) buffer; if (d->bDescriptorType != USB_DT_INTERFACE) { - warn("unexpected descriptor 0x%X, expecting interface, 0x%X", - d->bDescriptorType, USB_DT_INTERFACE); + dev_err(ddev, "config %d has an unexpected descriptor of type " + "0x%X, expecting interface type 0x%X\n", + cfgno, d->bDescriptorType, USB_DT_INTERFACE); return -EINVAL; } @@ -126,15 +143,16 @@ interface = config->interface[inum]; asnum = d->bAlternateSetting; if (asnum >= interface->num_altsetting) { - warn("invalid alternate setting %d for interface %d", - asnum, inum); + dev_err(ddev, "config %d interface %d has an invalid " + "alternate setting number: %d but max is %d\n", + cfgno, inum, asnum, interface->num_altsetting - 1); return -EINVAL; } ifp = &interface->altsetting[asnum]; if (ifp->desc.bLength) { - warn("duplicate descriptor for interface %d altsetting %d", - inum, asnum); + dev_err(ddev, "Duplicate descriptor for config %d " + "interface %d altsetting %d\n", cfgno, inum, asnum); return -EINVAL; } memcpy(&ifp->desc, buffer, USB_DT_INTERFACE_SIZE); @@ -153,39 +171,44 @@ (header->bDescriptorType == USB_DT_ENDPOINT)) break; - dbg("skipping descriptor 0x%X", header->bDescriptorType); + dev_dbg(ddev, "skipping descriptor 0x%X\n", + header->bDescriptorType); numskipped++; buffer += header->bLength; size -= header->bLength; } if (numskipped) { - dbg("skipped %d class/vendor specific interface descriptors", numskipped); + dev_dbg(ddev, "skipped %d class/vendor specific " + "interface descriptors\n", numskipped); ifp->extra = begin; ifp->extralen = buffer - begin; } if (ifp->desc.bNumEndpoints > USB_MAXENDPOINTS) { - warn("too many endpoints for interface %d altsetting %d", - inum, asnum); + dev_err(ddev, "too many endpoints for config %d interface %d " + "altsetting %d: %d, maximum allowed: %d\n", + cfgno, inum, asnum, ifp->desc.bNumEndpoints, + USB_MAXENDPOINTS); return -EINVAL; } len = ifp->desc.bNumEndpoints * sizeof(struct usb_host_endpoint); ifp->endpoint = kmalloc(len, GFP_KERNEL); - if (!ifp->endpoint) { - err("out of memory"); + if (!ifp->endpoint) return -ENOMEM; - } memset(ifp->endpoint, 0, len); for (i = 0; i < ifp->desc.bNumEndpoints; i++) { if (size < USB_DT_ENDPOINT_SIZE) { - warn("ran out of descriptors while parsing endpoints"); + dev_err(ddev, "too few endpoint descriptors for " + "config %d interface %d altsetting %d\n", + cfgno, inum, asnum); return -EINVAL; } - retval = usb_parse_endpoint(ifp->endpoint + i, buffer, size); + retval = usb_parse_endpoint(ddev, cfgno, inum, asnum, + ifp->endpoint + i, buffer, size); if (retval < 0) return retval; @@ -196,41 +219,45 @@ return buffer - buffer0; } -int usb_parse_configuration(struct usb_host_config *config, char *buffer, int size) +int usb_parse_configuration(struct device *ddev, int cfgidx, + struct usb_host_config *config, unsigned char *buffer, int size) { + int cfgno; int nintf, nintf_orig; int i, j; struct usb_interface *interface; - char *buffer2; + unsigned char *buffer2; int size2; struct usb_descriptor_header *header; int numskipped, len; - char *begin; + unsigned char *begin; int retval; memcpy(&config->desc, buffer, USB_DT_CONFIG_SIZE); if (config->desc.bDescriptorType != USB_DT_CONFIG || config->desc.bLength < USB_DT_CONFIG_SIZE) { - warn("invalid configuration descriptor"); + dev_err(ddev, "invalid descriptor for config index %d: " + "type = 0x%X, length = %d\n", cfgidx, + config->desc.bDescriptorType, config->desc.bLength); return -EINVAL; } config->desc.wTotalLength = size; + cfgno = config->desc.bConfigurationValue; nintf = nintf_orig = config->desc.bNumInterfaces; if (nintf > USB_MAXINTERFACES) { - warn("too many interfaces (%d max %d)", - nintf, USB_MAXINTERFACES); + dev_warn(ddev, "config %d has too many interfaces: %d, " + "using maximum allowed: %d\n", + cfgno, nintf, USB_MAXINTERFACES); config->desc.bNumInterfaces = nintf = USB_MAXINTERFACES; } 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); - if (!interface) { - err("out of memory"); + dev_dbg(ddev, "kmalloc IF %p, numif %i\n", interface, i); + if (!interface) return -ENOMEM; - } memset(interface, 0, sizeof(struct usb_interface)); } @@ -242,7 +269,8 @@ while (size2 >= sizeof(struct usb_descriptor_header)) { header = (struct usb_descriptor_header *) buffer2; if ((header->bLength > size2) || (header->bLength < 2)) { - warn("invalid descriptor of length %d", header->bLength); + dev_err(ddev, "config %d has an invalid descriptor " + "of length %d\n", cfgno, header->bLength); return -EINVAL; } @@ -250,14 +278,17 @@ struct usb_interface_descriptor *d; if (header->bLength < USB_DT_INTERFACE_SIZE) { - warn("invalid interface descriptor"); + dev_err(ddev, "config %d has an invalid " + "interface descriptor of length %d\n", + cfgno, header->bLength); return -EINVAL; } d = (struct usb_interface_descriptor *) header; i = d->bInterfaceNumber; if (i >= nintf_orig) { - warn("invalid interface number (%d/%d)", - i, nintf_orig); + dev_err(ddev, "config %d has an invalid " + "interface number: %d but max is %d\n", + cfgno, i, nintf_orig - 1); return -EINVAL; } if (i < nintf) @@ -265,7 +296,9 @@ } else if ((header->bDescriptorType == USB_DT_DEVICE || header->bDescriptorType == USB_DT_CONFIG) && j) { - warn("unexpected descriptor type 0x%X", header->bDescriptorType); + dev_err(ddev, "config %d contains an unexpected " + "descriptor of type 0x%X\n", + cfgno, header->bDescriptorType); return -EINVAL; } @@ -278,21 +311,24 @@ for (i = 0; i < config->desc.bNumInterfaces; ++i) { interface = config->interface[i]; if (interface->num_altsetting > USB_MAXALTSETTING) { - warn("too many alternate settings for interface %d (%d max %d)\n", - i, interface->num_altsetting, USB_MAXALTSETTING); + dev_err(ddev, "too many alternate settings for " + "config %d interface %d: %d, " + "maximum allowed: %d\n", + cfgno, i, interface->num_altsetting, + USB_MAXALTSETTING); return -EINVAL; } if (interface->num_altsetting == 0) { - warn("no alternate settings for interface %d", i); + dev_err(ddev, "config %d has no interface number " + "%d\n", cfgno, i); return -EINVAL; } - len = sizeof(*interface->altsetting) * interface->num_altsetting; + len = sizeof(*interface->altsetting) * + interface->num_altsetting; interface->altsetting = kmalloc(len, GFP_KERNEL); - if (!interface->altsetting) { - err("couldn't kmalloc interface->altsetting"); + if (!interface->altsetting) return -ENOMEM; - } memset(interface->altsetting, 0, len); } @@ -310,21 +346,24 @@ (header->bDescriptorType == USB_DT_INTERFACE)) break; - dbg("skipping descriptor 0x%X", header->bDescriptorType); + dev_dbg(ddev, "skipping descriptor 0x%X\n", + header->bDescriptorType); numskipped++; buffer += header->bLength; size -= header->bLength; } if (numskipped) { - dbg("skipped %d class/vendor specific configuration descriptors", numskipped); + dev_dbg(ddev, "skipped %d class/vendor specific configuration " + "descriptors\n", numskipped); config->extra = begin; config->extralen = buffer - begin; } /* Parse all the interface/altsetting descriptors */ while (size >= sizeof(struct usb_descriptor_header)) { - retval = usb_parse_interface(config, buffer, size); + retval = usb_parse_interface(ddev, cfgno, config, + buffer, size); if (retval < 0) return retval; @@ -337,7 +376,8 @@ interface = config->interface[i]; for (j = 0; j < interface->num_altsetting; ++j) { if (!interface->altsetting[j].desc.bLength) { - warn("missing altsetting %d for interface %d", j, i); + dev_err(ddev, "config %d interface %d has no " + "altsetting %d\n", cfgno, i, j); return -EINVAL; } } @@ -380,81 +420,77 @@ // (used by real hubs and virtual root hubs) int usb_get_configuration(struct usb_device *dev) { + struct device *ddev = &dev->dev; int ncfg = dev->descriptor.bNumConfigurations; - int result; + int result = -ENOMEM; unsigned int cfgno, length; unsigned char *buffer; unsigned char *bigbuffer; struct usb_config_descriptor *desc; if (ncfg > USB_MAXCONFIG) { - warn("too many configurations (%d max %d)", - ncfg, USB_MAXCONFIG); + dev_warn(ddev, "too many configurations: %d, " + "using maximum allowed: %d\n", ncfg, USB_MAXCONFIG); dev->descriptor.bNumConfigurations = ncfg = USB_MAXCONFIG; } if (ncfg < 1) { - warn("no configurations"); + dev_err(ddev, "no configurations\n"); return -EINVAL; } length = ncfg * sizeof(struct usb_host_config); dev->config = kmalloc(length, GFP_KERNEL); - if (!dev->config) { - err("out of memory"); - return -ENOMEM; - } + if (!dev->config) + goto err2; memset(dev->config, 0, length); length = ncfg * sizeof(char *); dev->rawdescriptors = kmalloc(length, GFP_KERNEL); - if (!dev->rawdescriptors) { - err("out of memory"); - return -ENOMEM; - } + if (!dev->rawdescriptors) + goto err2; memset(dev->rawdescriptors, 0, length); buffer = kmalloc(8, GFP_KERNEL); - if (!buffer) { - err("unable to allocate memory for configuration descriptors"); - return -ENOMEM; - } + if (!buffer) + goto err2; desc = (struct usb_config_descriptor *)buffer; 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); - if (result < 8) { - if (result < 0) - err("unable to get descriptor"); - else { - warn("config descriptor too short (expected %i, got %i)", 8, result); - result = -EINVAL; - } + /* configuration is */ + result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, + buffer, 8); + if (result < 0) { + dev_err(ddev, "unable to read config index %d " + "descriptor\n", cfgno); + goto err; + } else if (result < 8) { + dev_err(ddev, "config index %d descriptor too short " + "(expected %i, got %i)\n", cfgno, 8, result); + result = -EINVAL; goto err; } + length = max((int) le16_to_cpu(desc->wTotalLength), + USB_DT_CONFIG_SIZE); - /* Get the full buffer */ - length = max((int) le16_to_cpu(desc->wTotalLength), USB_DT_CONFIG_SIZE); - + /* Now that we know the length, get the whole thing */ bigbuffer = kmalloc(length, GFP_KERNEL); if (!bigbuffer) { - err("unable to allocate memory for configuration descriptors"); result = -ENOMEM; goto err; } - - /* Now that we know the length, get the whole thing */ - result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, bigbuffer, length); + result = usb_get_descriptor(dev, USB_DT_CONFIG, cfgno, + bigbuffer, length); if (result < 0) { - err("couldn't get all of config descriptors"); + dev_err(ddev, "unable to read config index %d " + "descriptor\n", cfgno); kfree(bigbuffer); goto err; } - if (result < length) { - err("config descriptor too short (expected %i, got %i)", length, result); + dev_err(ddev, "config index %d descriptor too short " + "(expected %i, got %i)\n", cfgno, length, result); result = -EINVAL; kfree(bigbuffer); goto err; @@ -462,20 +498,23 @@ dev->rawdescriptors[cfgno] = bigbuffer; - result = usb_parse_configuration(&dev->config[cfgno], bigbuffer, length); + result = usb_parse_configuration(&dev->dev, cfgno, + &dev->config[cfgno], bigbuffer, length); if (result > 0) - dbg("descriptor data left"); + dev_dbg(ddev, "config index %d descriptor has %d " + "excess byte(s)\n", cfgno, result); else if (result < 0) { ++cfgno; goto err; } } + result = 0; - kfree(buffer); - return 0; err: kfree(buffer); dev->descriptor.bNumConfigurations = cfgno; +err2: + if (result == -ENOMEM) + dev_err(ddev, "out of memory\n"); return result; } -