[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>

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 63f374e..9930195 100644 (file)
--- 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 8ba5854..1bacb37 100644 (file)
--- 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 3519f31..644a3d4 100644 (file)
--- 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 4cca77c..edd83e0 100644 (file)
--- 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 888dbe4..1c4a684 100644 (file)
--- 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 c500d6b..748d043 100644 (file)
--- 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 */