[PATCH] usbcore: Fix handling of sysfs strings and other attributes

This patch (as592) makes a few small improvements to the way device
strings are handled, and it fixes some bugs in a couple of other sysfs
attribute routines.  (Look at show_configuration_string() to see what I
mean.)

Signed-off-by: Alan Stern <stern@rowland.harvard.edu>
Signed-off-by: Greg Kroah-Hartman <gregkh@suse.de>

6 files changed, 62 insertions, 65 deletions

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 63f374e62db2..993019500cc3 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -112,8 +112,12 @@ void usb_release_interface_cache(struct kref *ref)
         struct usb_interface_cache *intfc = ref_to_usb_interface_cache(ref);
         int j;
 
-        for (j = 0; j < intfc->num_altsetting; j++)
-                kfree(intfc->altsetting[j].endpoint);
+        for (j = 0; j < intfc->num_altsetting; j++) {
+                struct usb_host_interface *alt = &intfc->altsetting[j];
+
+                kfree(alt->endpoint);
+                kfree(alt->string);
+        }
         kfree(intfc);
 }
 
@@ -420,8 +424,6 @@ void usb_destroy_configuration(struct usb_device *dev)
                 struct usb_host_config *cf = &dev->config[c];
 
                 kfree(cf->string);
-                cf->string = NULL;
-
                 for (i = 0; i < cf->desc.bNumInterfaces; i++) {
                         if (cf->intf_cache[i])
                                 kref_put(&cf->intf_cache[i]->ref, 
diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 8ba5854e5387..1bacb374b007 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -1204,21 +1204,6 @@ static inline void show_string(struct usb_device *udev, char *id, char *string)
 {}
 #endif
 
-static void get_string(struct usb_device *udev, char **string, int index)
-{
-        char *buf;
-
-        if (!index)
-                return;
-        buf = kmalloc(256, GFP_KERNEL);
-        if (!buf)
-                return;
-        if (usb_string(udev, index, buf, 256) > 0)
-                *string = buf;
-        else
-                kfree(buf);
-}
-
 
 #ifdef  CONFIG_USB_OTG
 #include "otg_whitelist.h"
@@ -1257,9 +1242,10 @@ int usb_new_device(struct usb_device *udev)
         }
 
         /* read the standard strings and cache them if present */
-        get_string(udev, &udev->product, udev->descriptor.iProduct);
-        get_string(udev, &udev->manufacturer, udev->descriptor.iManufacturer);
-        get_string(udev, &udev->serial, udev->descriptor.iSerialNumber);
+        udev->product = usb_cache_string(udev, udev->descriptor.iProduct);
+        udev->manufacturer = usb_cache_string(udev,
+                        udev->descriptor.iManufacturer);
+        udev->serial = usb_cache_string(udev, udev->descriptor.iSerialNumber);
 
         /* Tell the world! */
         dev_dbg(&udev->dev, "new device strings: Mfr=%d, Product=%d, "
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 3519f317898e..644a3d4f12aa 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -787,6 +787,31 @@ int usb_string(struct usb_device *dev, int index, char *buf, size_t size)
         return err;
 }
 
+/**
+ * usb_cache_string - read a string descriptor and cache it for later use
+ * @udev: the device whose string descriptor is being read
+ * @index: the descriptor index
+ *
+ * Returns a pointer to a kmalloc'ed buffer containing the descriptor string,
+ * or NULL if the index is 0 or the string could not be read.
+ */
+char *usb_cache_string(struct usb_device *udev, int index)
+{
+        char *buf;
+        char *smallbuf = NULL;
+        int len;
+
+        if (index > 0 && (buf = kmalloc(256, GFP_KERNEL)) != NULL) {
+                if ((len = usb_string(udev, index, buf, 256)) > 0) {
+                        if ((smallbuf = kmalloc(++len, GFP_KERNEL)) == NULL)
+                                return buf;
+                        memcpy(smallbuf, buf, len);
+                }
+                kfree(buf);
+        }
+        return smallbuf;
+}
+
 /*
  * usb_get_device_descriptor - (re)reads the device descriptor (usbcore)
  * @dev: the device whose device descriptor is being updated
@@ -1008,8 +1033,6 @@ void usb_disable_device(struct usb_device *dev, int skip_ep0)
                         dev_dbg (&dev->dev, "unregistering interface %s\n",
                                 interface->dev.bus_id);
                         usb_remove_sysfs_intf_files(interface);
-                        kfree(interface->cur_altsetting->string);
-                        interface->cur_altsetting->string = NULL;
                         device_del (&interface->dev);
                 }
 
@@ -1422,12 +1445,9 @@ free_interfaces:
                 }
                 kfree(new_interfaces);
 
-                if ((cp->desc.iConfiguration) &&
-                    (cp->string == NULL)) {
-                        cp->string = kmalloc(256, GFP_KERNEL);
-                        if (cp->string)
-                                usb_string(dev, cp->desc.iConfiguration, cp->string, 256);
-                }
+                if (cp->string == NULL)
+                        cp->string = usb_cache_string(dev,
+                                        cp->desc.iConfiguration);
 
                 /* Now that all the interfaces are set up, register them
                  * to trigger binding of drivers to interfaces.  probe()
@@ -1437,13 +1457,12 @@ free_interfaces:
                  */
                 for (i = 0; i < nintf; ++i) {
                         struct usb_interface *intf = cp->interface[i];
-                        struct usb_interface_descriptor *desc;
+                        struct usb_host_interface *alt = intf->cur_altsetting;
 
-                        desc = &intf->altsetting [0].desc;
                         dev_dbg (&dev->dev,
                                 "adding %s (config #%d, interface %d)\n",
                                 intf->dev.bus_id, configuration,
-                                desc->bInterfaceNumber);
+                                alt->desc.bInterfaceNumber);
                         ret = device_add (&intf->dev);
                         if (ret != 0) {
                                 dev_err(&dev->dev,
@@ -1452,13 +1471,6 @@ free_interfaces:
                                         ret);
                                 continue;
                         }
-                        if ((intf->cur_altsetting->desc.iInterface) &&
-                            (intf->cur_altsetting->string == NULL)) {
-                                intf->cur_altsetting->string = kmalloc(256, GFP_KERNEL);
-                                if (intf->cur_altsetting->string)
-                                        usb_string(dev, intf->cur_altsetting->desc.iInterface,
-                                                   intf->cur_altsetting->string, 256);
-                        }
                         usb_create_sysfs_intf_files (intf);
                 }
         }
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index 4cca77cf0c48..edd83e014452 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -249,18 +249,12 @@ static ssize_t show_configuration_string(struct device *dev,
 {
         struct usb_device *udev;
         struct usb_host_config *actconfig;
-        int len;
 
         udev = to_usb_device (dev);
         actconfig = udev->actconfig;
         if ((!actconfig) || (!actconfig->string))
                 return 0;
-        len = sprintf(buf, actconfig->string, PAGE_SIZE);
-        if (len < 0)
-                return 0;
-        buf[len] = '\n';
-        buf[len+1] = 0;
-        return len+1;
+        return sprintf(buf, "%s\n", actconfig->string);
 }
 static DEVICE_ATTR(configuration, S_IRUGO, show_configuration_string, NULL);
 
@@ -291,15 +285,9 @@ static ssize_t  show_##name(struct device *dev,				\
                 struct device_attribute *attr, char *buf)               \
 {                                                                       \
         struct usb_device *udev;                                        \
-        int len;                                                        \
                                                                         \
         udev = to_usb_device (dev);                                     \
-        len = snprintf(buf, 256, "%s", udev->name);                     \
-        if (len < 0)                                                    \
-                return 0;                                               \
-        buf[len] = '\n';                                                \
-        buf[len+1] = 0;                                                 \
-        return len+1;                                                   \
+        return sprintf(buf, "%s\n", udev->name);                        \
 }                                                                       \
 static DEVICE_ATTR(name, S_IRUGO, show_##name, NULL);
 
@@ -449,11 +437,11 @@ void usb_remove_sysfs_dev_files (struct usb_device *udev)
         usb_remove_ep_files(&udev->ep0);
         sysfs_remove_group(&dev->kobj, &dev_attr_grp);
 
-        if (udev->descriptor.iManufacturer)
+        if (udev->manufacturer)
                 device_remove_file(dev, &dev_attr_manufacturer);
-        if (udev->descriptor.iProduct)
+        if (udev->product)
                 device_remove_file(dev, &dev_attr_product);
-        if (udev->descriptor.iSerialNumber)
+        if (udev->serial)
                 device_remove_file(dev, &dev_attr_serial);
         device_remove_file (dev, &dev_attr_configuration);
 }
@@ -535,7 +523,8 @@ static struct attribute_group intf_attr_grp = {
         .attrs = intf_attrs,
 };
 
-static inline void usb_create_intf_ep_files(struct usb_interface *intf)
+static inline void usb_create_intf_ep_files(struct usb_interface *intf,
+                struct usb_device *udev)
 {
         struct usb_host_interface *iface_desc;
         int i;
@@ -543,7 +532,7 @@ static inline void usb_create_intf_ep_files(struct usb_interface *intf)
         iface_desc = intf->cur_altsetting;
         for (i = 0; i < iface_desc->desc.bNumEndpoints; ++i)
                 usb_create_ep_files(&intf->dev.kobj, &iface_desc->endpoint[i],
-                                interface_to_usbdev(intf));
+                                udev);
 }
 
 static inline void usb_remove_intf_ep_files(struct usb_interface *intf)
@@ -558,11 +547,16 @@ static inline void usb_remove_intf_ep_files(struct usb_interface *intf)
 
 void usb_create_sysfs_intf_files (struct usb_interface *intf)
 {
+        struct usb_device *udev = interface_to_usbdev(intf);
+        struct usb_host_interface *alt = intf->cur_altsetting;
+
         sysfs_create_group(&intf->dev.kobj, &intf_attr_grp);
 
-        if (intf->cur_altsetting->string)
+        if (alt->string == NULL)
+                alt->string = usb_cache_string(udev, alt->desc.iInterface);
+        if (alt->string)
                 device_create_file(&intf->dev, &dev_attr_interface);
-        usb_create_intf_ep_files(intf);
+        usb_create_intf_ep_files(intf, udev);
 }
 
 void usb_remove_sysfs_intf_files (struct usb_interface *intf)
diff --git a/drivers/usb/core/usb.h b/drivers/usb/core/usb.h
index 888dbe443695..1c4a68499dce 100644
--- a/drivers/usb/core/usb.h
+++ b/drivers/usb/core/usb.h
@@ -13,6 +13,7 @@ extern void usb_disable_device (struct usb_device *dev, int skip_ep0);
 
 extern int usb_get_device_descriptor(struct usb_device *dev,
                 unsigned int size);
+extern char *usb_cache_string(struct usb_device *udev, int index);
 extern int usb_set_configuration(struct usb_device *dev, int configuration);
 
 extern void usb_lock_all_devices(void);
diff --git a/include/linux/usb.h b/include/linux/usb.h
index c500d6b5a16d..748d04385256 100644
--- a/include/linux/usb.h
+++ b/include/linux/usb.h
@@ -231,7 +231,7 @@ struct usb_interface_cache {
 struct usb_host_config {
         struct usb_config_descriptor    desc;
 
-        char *string;
+        char *string;           /* iConfiguration string, if present */
         /* the interfaces associated with this configuration,
          * stored in no particular order */
         struct usb_interface *interface[USB_MAXINTERFACES];
@@ -351,9 +351,11 @@ struct usb_device {
         int have_langid;                /* whether string_langid is valid */
         int string_langid;              /* language ID for strings */
 
-        char *product;
-        char *manufacturer;
-        char *serial;                   /* static strings from the device */
+        /* static strings from the device */
+        char *product;                  /* iProduct string, if present */
+        char *manufacturer;             /* iManufacturer string, if present */
+        char *serial;                   /* iSerialNumber string, if present */
+
         struct list_head filelist;
         struct class_device *class_dev;
         struct dentry *usbfs_dentry;    /* usbfs dentry entry for the device */