From 4f62efe67f077db17dad03a1d4c9665000a3eb45 Mon Sep 17 00:00:00 2001 From: Alan Stern Date: Mon, 24 Oct 2005 16:24:14 -0400 Subject: [PATCH] [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 Signed-off-by: Greg Kroah-Hartman --- drivers/usb/core/config.c | 10 ++++---- drivers/usb/core/hub.c | 22 ++++------------- drivers/usb/core/message.c | 48 ++++++++++++++++++++++++-------------- drivers/usb/core/sysfs.c | 36 ++++++++++++---------------- drivers/usb/core/usb.h | 1 + include/linux/usb.h | 10 ++++---- 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 */ -- 2.30.2