[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

+#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 @@ static int usb_parse_endpoint(struct usb_host_endpoint *endpoint, unsigned char
 
 	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 @@ static int usb_parse_endpoint(struct usb_host_endpoint *endpoint, unsigned char
 	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 @@ static int usb_parse_endpoint(struct usb_host_endpoint *endpoint, unsigned char
 		    (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 @@ static void usb_free_intf(struct usb_interface *intf)
 	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 @@ static int usb_parse_interface(struct usb_host_config *config, unsigned char *bu
 
 	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 @@ static int usb_parse_interface(struct usb_host_config *config, unsigned char *bu
 	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 @@ static int usb_parse_interface(struct usb_host_config *config, unsigned char *bu
 		    (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 @@ static int usb_parse_interface(struct usb_host_config *config, unsigned char *bu
 	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 @@ int usb_parse_configuration(struct usb_host_config *config, char *buffer, int si
 	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 @@ int usb_parse_configuration(struct usb_host_config *config, char *buffer, int si
 			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 @@ int usb_parse_configuration(struct usb_host_config *config, char *buffer, int si
 
 		} 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 @@ int usb_parse_configuration(struct usb_host_config *config, char *buffer, int si
 	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 @@ int usb_parse_configuration(struct usb_host_config *config, char *buffer, int si
 		    (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 @@ int usb_parse_configuration(struct usb_host_config *config, char *buffer, int si
 		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 @@ void usb_destroy_configuration(struct usb_device *dev)
 // (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 @@ int usb_get_configuration(struct usb_device *dev)
 
 		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;
 }
-