Greg:
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.
Please apply.
Alan Stern
--- 2.6/drivers/usb/core/config.c.orig Fri Mar 12 16:36:55 2004
+++ 2.6/drivers/usb/core/config.c Fri Mar 12 16:32:08 2004
@@ -1,18 +1,25 @@
+#include <linux/config.h>
+
+#ifdef CONFIG_USB_DEBUG
+#define DEBUG
+#endif
+
#include <linux/usb.h>
#include <linux/module.h>
#include <linux/init.h>
#include <linux/slab.h>
+#include <linux/device.h>
#include <asm/byteorder.h>
#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;
}
-
-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id=1470&alloc_id=3638&op=click
_______________________________________________
[EMAIL PROTECTED]
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel
