[PATCH] USB: Remove interface/altsettings assumption from audio driver

This patch updates the USB audio class driver to use the usb_ifnum_to_if()
and usb_altnum_to_altsetting() routines, thereby removing assumptions
about which interface or altsetting is stored in which array entry.

It also simplifies the driver's probe() routine by using the raw
configuration descriptor already loaded into memory instead of reading the
descriptor from the device.  Now, either the current driver has a bug and
never deallocates the buffer used to hold the descriptor, or else I've
introduced a double-free error.  There's no obvious place where the buffer
gets freed, but it's hard to be certain.

It would be good if someone could try out this patch.  I can't test it,
not having any USB audio devices handy.  If the double-free error is
present, it will show up when the device is disconnected and the
configuration data is released.

drivers/usb/class/audio.c

@@ -102,6 +102,9 @@
  * 2003-04-08:	Oliver Neukum (oliver@neukum.name):
  *		Setting a configuration is done by usbcore and must not be overridden
  * 2004-02-27:  Workaround for broken synch descriptors
+ * 2004-03-07:	Alan Stern <stern@rowland.harvard.edu>
+ *		Add usb_ifnum_to_if() and usb_altnum_to_altsetting() support.
+ *		Use the in-memory descriptors instead of reading them from the device.
  * 
  */
 
@@ -143,8 +146,8 @@
  *
  * How does the parsing work? First, all interfaces are searched
  * for an AudioControl class interface. If found, the config descriptor
- * that belongs to the current configuration is fetched from the device.
- * Then the HEADER descriptor is fetched. It contains a list of
+ * that belongs to the current configuration is searched and
+ * the HEADER descriptor is found. It contains a list of
  * all AudioStreaming and MIDIStreaming devices. This list is then walked,
  * and all AudioStreaming interfaces are classified into input and output
  * interfaces (according to the endpoint0 direction in altsetting1) (MIDIStreaming
@@ -1514,7 +1517,6 @@ static int find_format(struct audioformat *afp, unsigned int nr, unsigned int fm
 static int set_format_in(struct usb_audiodev *as)
 {
 	struct usb_device *dev = as->state->usbdev;
-	struct usb_host_config *config = dev->actconfig;
 	struct usb_host_interface *alts;
 	struct usb_interface *iface;
 	struct usbin *u = &as->usbin;
@@ -1524,9 +1526,9 @@ static int set_format_in(struct usb_audiodev *as)
 	unsigned char data[3];
 	int fmtnr, ret;
 
-	if (u->interface < 0 || u->interface >= config->desc.bNumInterfaces)
+	iface = usb_ifnum_to_if(dev, u->interface);
+	if (!iface)
 		return 0;
-	iface = config->interface[u->interface];
 
 	fmtnr = find_format(as->fmtin, as->numfmtin, d->format, d->srate);
 	if (fmtnr < 0) {
@@ -1535,7 +1537,7 @@ static int set_format_in(struct usb_audiodev *as)
 	}
 
 	fmt = as->fmtin + fmtnr;
-	alts = &iface->altsetting[fmt->altsetting];
+	alts = usb_altnum_to_altsetting(iface, fmt->altsetting);
 	u->format = fmt->format;
 	u->datapipe = usb_rcvisocpipe(dev, alts->endpoint[0].desc.bEndpointAddress & 0xf);
 	u->syncpipe = u->syncinterval = 0;
@@ -1556,7 +1558,8 @@ static int set_format_in(struct usb_audiodev *as)
 		d->srate = fmt->sratelo;
 	if (d->srate > fmt->sratehi)
 		d->srate = fmt->sratehi;
-	dprintk((KERN_DEBUG "usbaudio: set_format_in: usb_set_interface %u %u\n", alts->desc.bInterfaceNumber, fmt->altsetting));
+	dprintk((KERN_DEBUG "usbaudio: set_format_in: usb_set_interface %u %u\n",
+			u->interface, fmt->altsetting));
 	if (usb_set_interface(dev, alts->desc.bInterfaceNumber, fmt->altsetting) < 0) {
 		printk(KERN_WARNING "usbaudio: usb_set_interface failed, device %d interface %d altsetting %d\n",
 		       dev->devnum, u->interface, fmt->altsetting);
@@ -1603,7 +1606,6 @@ static int set_format_in(struct usb_audiodev *as)
 static int set_format_out(struct usb_audiodev *as)
 {
 	struct usb_device *dev = as->state->usbdev;
-	struct usb_host_config *config = dev->actconfig;
 	struct usb_host_interface *alts;
 	struct usb_interface *iface;	
 	struct usbout *u = &as->usbout;
@@ -1613,9 +1615,9 @@ static int set_format_out(struct usb_audiodev *as)
 	unsigned char data[3];
 	int fmtnr, ret;
 
-	if (u->interface < 0 || u->interface >= config->desc.bNumInterfaces)
+	iface = usb_ifnum_to_if(dev, u->interface);
+	if (!iface)
 		return 0;
-	iface = config->interface[u->interface];
 
 	fmtnr = find_format(as->fmtout, as->numfmtout, d->format, d->srate);
 	if (fmtnr < 0) {
@@ -1625,7 +1627,7 @@ static int set_format_out(struct usb_audiodev *as)
 
 	fmt = as->fmtout + fmtnr;
 	u->format = fmt->format;
-	alts = &iface->altsetting[fmt->altsetting];
+	alts = usb_altnum_to_altsetting(iface, fmt->altsetting);
 	u->datapipe = usb_sndisocpipe(dev, alts->endpoint[0].desc.bEndpointAddress & 0xf);
 	u->syncpipe = u->syncinterval = 0;
 	if ((alts->endpoint[0].desc.bmAttributes & 0x0c) == 0x04) {
@@ -1652,7 +1654,8 @@ static int set_format_out(struct usb_audiodev *as)
 		d->srate = fmt->sratelo;
 	if (d->srate > fmt->sratehi)
 		d->srate = fmt->sratehi;
-	dprintk((KERN_DEBUG "usbaudio: set_format_out: usb_set_interface %u %u\n", alts->desc.bInterfaceNumber, fmt->altsetting));
+	dprintk((KERN_DEBUG "usbaudio: set_format_out: usb_set_interface %u %u\n",
+			u->interface, fmt->altsetting));
 	if (usb_set_interface(dev, u->interface, fmt->altsetting) < 0) {
 		printk(KERN_WARNING "usbaudio: usb_set_interface failed, device %d interface %d altsetting %d\n",
 		       dev->devnum, u->interface, fmt->altsetting);
@@ -2701,7 +2704,6 @@ static int usb_audio_release(struct inode *inode, struct file *file)
 	struct usb_audiodev *as = (struct usb_audiodev *)file->private_data;
 	struct usb_audio_state *s;
 	struct usb_device *dev;
-	struct usb_interface *iface;
 
 	lock_kernel();
 	s = as->state;
@@ -2711,19 +2713,15 @@ static int usb_audio_release(struct inode *inode, struct file *file)
 	down(&open_sem);
 	if (file->f_mode & FMODE_WRITE) {
 		usbout_stop(as);
-		if (dev && as->usbout.interface >= 0) {
-			iface = dev->actconfig->interface[as->usbout.interface];
-			usb_set_interface(dev, iface->altsetting->desc.bInterfaceNumber, 0);
-		}
+		if (dev && as->usbout.interface >= 0)
+			usb_set_interface(dev, as->usbout.interface, 0);
 		dmabuf_release(&as->usbout.dma);
 		usbout_release(as);
 	}
 	if (file->f_mode & FMODE_READ) {
 		usbin_stop(as);
-		if (dev && as->usbin.interface >= 0) {
-			iface = dev->actconfig->interface[as->usbin.interface];
-			usb_set_interface(dev, iface->altsetting->desc.bInterfaceNumber, 0);
-		}
+		if (dev && as->usbin.interface >= 0)
+			usb_set_interface(dev, as->usbin.interface, 0);
 		dmabuf_release(&as->usbin.dma);
 		usbin_release(as);
 	}
@@ -2828,12 +2826,11 @@ static void usb_audio_parsestreaming(struct usb_audio_state *s, unsigned char *b
 {
 	struct usb_device *dev = s->usbdev;
 	struct usb_audiodev *as;
-	struct usb_host_config *config = dev->actconfig;
 	struct usb_host_interface *alts;
 	struct usb_interface *iface;
 	struct audioformat *fp;
 	unsigned char *fmt, *csep;
-	unsigned int i, j, k, format;
+	unsigned int i, j, k, format, idx;
 
 	if (!(as = kmalloc(sizeof(struct usb_audiodev), GFP_KERNEL)))
 		return;
@@ -2874,9 +2871,10 @@ static void usb_audio_parsestreaming(struct usb_audio_state *s, unsigned char *b
 	/* search for input formats */
 	if (asifin >= 0) {
 		as->usbin.flags = FLG_CONNECTED;
-		iface = config->interface[asifin];
-		for (i = 0; i < iface->num_altsetting; i++) {
-			alts = &iface->altsetting[i];
+		iface = usb_ifnum_to_if(dev, asifin);
+		for (idx = 0; idx < iface->num_altsetting; idx++) {
+			alts = &iface->altsetting[idx];
+			i = alts->desc.bAlternateSetting;
 			if (alts->desc.bInterfaceClass != USB_CLASS_AUDIO || alts->desc.bInterfaceSubClass != 2)
 				continue;
 			if (alts->desc.bNumEndpoints < 1) {
@@ -2955,14 +2953,15 @@ static void usb_audio_parsestreaming(struct usb_audio_state *s, unsigned char *b
 	/* search for output formats */
 	if (asifout >= 0) {
 		as->usbout.flags = FLG_CONNECTED;
-		iface = config->interface[asifout];
-		for (i = 0; i < iface->num_altsetting; i++) {
-			alts = &iface->altsetting[i];
+		iface = usb_ifnum_to_if(dev, asifout);
+		for (idx = 0; idx < iface->num_altsetting; idx++) {
+			alts = &iface->altsetting[idx];
+			i = alts->desc.bAlternateSetting;
 			if (alts->desc.bInterfaceClass != USB_CLASS_AUDIO || alts->desc.bInterfaceSubClass != 2)
 				continue;
 			if (alts->desc.bNumEndpoints < 1) {
 				/* altsetting 0 should never have iso EPs */
-				if (alts->desc.bAlternateSetting != 0)
+				if (i != 0)
 				printk(KERN_ERR "usbaudio: device %u interface %u altsetting %u does not have an endpoint\n", 
 				       dev->devnum, asifout, i);
 				continue;
@@ -3659,8 +3658,8 @@ static void usb_audio_constructmixer(struct usb_audio_state *s, unsigned char *b
 static struct usb_audio_state *usb_audio_parsecontrol(struct usb_device *dev, unsigned char *buffer, unsigned int buflen, unsigned int ctrlif)
 {
 	struct usb_audio_state *s;
-	struct usb_host_config *config = dev->actconfig;
 	struct usb_interface *iface;
+	struct usb_host_interface *alt;
 	unsigned char ifin[USB_MAXINTERFACES], ifout[USB_MAXINTERFACES];
 	unsigned char *p1;
 	unsigned int i, j, k, numifin = 0, numifout = 0;
@@ -3689,54 +3688,63 @@ static struct usb_audio_state *usb_audio_parsecontrol(struct usb_device *dev, un
 		       dev->devnum, ctrlif);
 	for (i = 0; i < p1[7]; i++) {
 		j = p1[8+i];
-		if (j >= config->desc.bNumInterfaces) {
+		iface = usb_ifnum_to_if(dev, j);
+		if (!iface) {
 			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u does not exist\n",
 			       dev->devnum, ctrlif, j);
 			continue;
 		}
-		iface = config->interface[j];
-		if (iface->altsetting[0].desc.bInterfaceClass != USB_CLASS_AUDIO) {
-			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u is not an AudioClass interface\n",
-			       dev->devnum, ctrlif, j);
+		if (iface->num_altsetting == 1) {
+			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u has only 1 altsetting.\n", dev->devnum, ctrlif);
 			continue;
 		}
-		if (iface->altsetting[0].desc.bInterfaceSubClass == 3) {
-			printk(KERN_INFO "usbaudio: device %d audiocontrol interface %u interface %u MIDIStreaming not supported\n",
+		alt = usb_altnum_to_altsetting(iface, 0);
+		if (!alt) {
+			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u has no altsetting 0\n",
 			       dev->devnum, ctrlif, j);
 			continue;
 		}
-		if (iface->altsetting[0].desc.bInterfaceSubClass != 2) {
-			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u invalid AudioClass subtype\n",
+		if (alt->desc.bInterfaceClass != USB_CLASS_AUDIO) {
+			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u is not an AudioClass interface\n",
 			       dev->devnum, ctrlif, j);
 			continue;
 		}
-		if (iface->num_altsetting == 0) {
-			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u has no working interface.\n", dev->devnum, ctrlif);
+		if (alt->desc.bInterfaceSubClass == 3) {
+			printk(KERN_INFO "usbaudio: device %d audiocontrol interface %u interface %u MIDIStreaming not supported\n",
+			       dev->devnum, ctrlif, j);
 			continue;
 		}
-		if (iface->num_altsetting == 1) {
-			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u has only 1 altsetting.\n", dev->devnum, ctrlif);
+		if (alt->desc.bInterfaceSubClass != 2) {
+			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u invalid AudioClass subtype\n",
+			       dev->devnum, ctrlif, j);
 			continue;
 		}
-		if (iface->altsetting[0].desc.bNumEndpoints > 0) {
+		if (alt->desc.bNumEndpoints > 0) {
 			/* Check all endpoints; should they all have a bandwidth of 0 ? */
-			for (k = 0; k < iface->altsetting[0].desc.bNumEndpoints; k++) {
-				if (iface->altsetting[0].endpoint[k].desc.wMaxPacketSize > 0) {
+			for (k = 0; k < alt->desc.bNumEndpoints; k++) {
+				if (alt->endpoint[k].desc.wMaxPacketSize > 0) {
 					printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u endpoint %d does not have 0 bandwidth at alt[0]\n", dev->devnum, ctrlif, k);
 					break;
 				}
 			}
-			if (k < iface->altsetting[0].desc.bNumEndpoints)
+			if (k < alt->desc.bNumEndpoints)
 				continue;
 		}
-		if (iface->altsetting[1].desc.bNumEndpoints < 1) {
+
+		alt = usb_altnum_to_altsetting(iface, 1);
+		if (!alt) {
+			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u has no altsetting 1\n",
+			       dev->devnum, ctrlif, j);
+			continue;
+		}
+		if (alt->desc.bNumEndpoints < 1) {
 			printk(KERN_ERR "usbaudio: device %d audiocontrol interface %u interface %u has no endpoint\n",
 			       dev->devnum, ctrlif, j);
 			continue;
 		}
 		/* note: this requires the data endpoint to be ep0 and the optional sync
 		   ep to be ep1, which seems to be the case */
-		if (iface->altsetting[1].endpoint[0].desc.bEndpointAddress & USB_DIR_IN) {
+		if (alt->endpoint[0].desc.bEndpointAddress & USB_DIR_IN) {
 			if (numifin < USB_MAXINTERFACES) {
 				ifin[numifin++] = j;
 				usb_driver_claim_interface(&usb_audio_driver, iface, (void *)-1);
@@ -3783,12 +3791,9 @@ static int usb_audio_probe(struct usb_interface *intf,
 			   const struct usb_device_id *id)
 {
 	struct usb_device *dev = interface_to_usbdev (intf);
-	struct usb_host_config *config = dev->actconfig;	
 	struct usb_audio_state *s;
 	unsigned char *buffer;
-	unsigned char buf[8];
-	unsigned int i, buflen;
-	int ret;
+	unsigned int buflen;
 
 #if 0
 	printk(KERN_DEBUG "usbaudio: Probing if %i: IC %x, ISC %x\n", ifnum,
@@ -3800,26 +3805,8 @@ static int usb_audio_probe(struct usb_interface *intf,
 	 * audiocontrol interface found
 	 * find which configuration number is active
 	 */
-	i = dev->actconfig - config;
-
-	ret = usb_get_descriptor(dev, USB_DT_CONFIG, i, buf, 8);
-	if (ret < 0) {
-		printk(KERN_ERR "usbaudio: cannot get first 8 bytes of config descriptor %d of device %d (error %d)\n", i, dev->devnum, ret);
-		return -EIO;
-	}
-	if (buf[1] != USB_DT_CONFIG || buf[0] < 9) {
-		printk(KERN_ERR "usbaudio: invalid config descriptor %d of device %d\n", i, dev->devnum);
-		return -EIO;
-	}
-	buflen = buf[2] | (buf[3] << 8);
-	if (!(buffer = kmalloc(buflen, GFP_KERNEL)))
-		return -ENOMEM;
-	ret = usb_get_descriptor(dev, USB_DT_CONFIG, i, buffer, buflen);
-	if (ret < 0) {
-		kfree(buffer);
-		printk(KERN_ERR "usbaudio: cannot get config descriptor %d of device %d (error %d)\n", i, dev->devnum, ret);
-		return -EIO;
-	}
+	buffer = dev->rawdescriptors[dev->actconfig - dev->config];
+	buflen = dev->actconfig->desc.wTotalLength;
 	s = usb_audio_parsecontrol(dev, buffer, buflen, intf->altsetting->desc.bInterfaceNumber);
 	if (s) {
 		usb_set_intfdata (intf, s);