On Mon, 11 Jun 2007, Craig W. Nadler wrote: > diff -uprN a/drivers/usb/core/config.c b/drivers/usb/core/config.c > --- a/drivers/usb/core/config.c 2007-05-24 17:22:47.000000000 -0400 > +++ b/drivers/usb/core/config.c 2007-06-09 03:20:51.000000000 -0400 > @@ -232,6 +232,7 @@ static int usb_parse_configuration(struc > struct usb_descriptor_header *header; > int len, retval; > u8 inums[USB_MAXINTERFACES], nalts[USB_MAXINTERFACES]; > + u8 iad_num = 0;
Remember what I said about smaller-than-word-size local variables? About the only reason ever to use a short or char is if you really need it to occupy exactly that much space (if it is part of an array or a structure, for example). Make iad_num plain old unsigned. > @@ -309,6 +310,17 @@ static int usb_parse_configuration(struc > ++n; > } > > + } else if (header->bDescriptorType == > + USB_DT_INTERFACE_ASSOCIATION) { > + if (iad_num == USB_MAXIADS) { > + dev_err(ddev, "found more Interface " > + "Association Descriptors " > + "than allocated for\n"); This should be a warning, not an error. Also you should include cfgno in the error message so that people will know which configuration has too many IADs. > diff -uprN a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c > --- a/drivers/usb/core/devices.c 2007-05-24 17:22:47.000000000 -0400 > +++ b/drivers/usb/core/devices.c 2007-06-09 03:20:51.000000000 -0400 > @@ -324,6 +344,11 @@ static char *usb_dump_config ( > if (!config) /* getting these some in 2.3.7; none in 2.3.6 */ > return start + sprintf(start, "(null Cfg. desc.)\n"); > start = usb_dump_config_descriptor(start, end, &config->desc, active); > + for (i = 0; i < USB_MAXIADS; i++) { > + if (config->intf_assoc[i] == NULL) break; Style: the break statement should be on a separate line from the if statement. > diff -uprN a/drivers/usb/core/message.c b/drivers/usb/core/message.c > --- a/drivers/usb/core/message.c 2007-05-24 17:22:47.000000000 -0400 > +++ b/drivers/usb/core/message.c 2007-06-09 03:30:02.000000000 -0400 > @@ -1315,6 +1315,39 @@ static void release_interface(struct dev ... > + intf_assoc = config->intf_assoc[i]; > + if (intf_assoc->bInterfaceCount == 0) continue; Same style violation. > + > + first_intf = intf_assoc->bFirstInterface; > + last_intf = first_intf + (intf_assoc->bInterfaceCount - 1); > + if (inum >= first_intf && inum <= last_intf) { > + if (!retval) { > + retval = intf_assoc; > + } else { > + dev_err (&dev->dev, "Interface #%d referenced" > + " by multiple IADs\n", inum); > + } Are you really sure this needs to be an error and not a warning? Remember also what I said earlier about putting extraneous space characters between a function name and the following open paren. > diff -uprN a/include/linux/usb.h b/include/linux/usb.h > --- a/include/linux/usb.h 2007-05-24 17:22:47.000000000 -0400 > +++ b/include/linux/usb.h 2007-06-09 03:20:51.000000000 -0400 > @@ -245,6 +250,11 @@ struct usb_host_config { > struct usb_config_descriptor desc; > > char *string; /* iConfiguration string, if present */ > + > + /* List of any Interface Association Descriptors in this > + * configuration. */ > + struct usb_interface_assoc_descriptor *intf_assoc[USB_MAXIADS]; This may be just my own confusion, but is it necessary to list interface association descriptors in both the host_interface and the host_config structures? Alan Stern ------------------------------------------------------------------------- This SF.net email is sponsored by DB2 Express Download DB2 Express C - the FREE version of DB2 express and take control of your XML. No limits. Just data. Click to get it now. http://sourceforge.net/powerbar/db2/ _______________________________________________ linux-usb-devel@lists.sourceforge.net To unsubscribe, use the last form field at: https://lists.sourceforge.net/lists/listinfo/linux-usb-devel