[PATCH 6/7] usb: core: reuse kbasename()

[Andy Shevchenko]

Signed-off-by: Andy Shevchenko 
Cc: Greg Kroah-Hartman 
Cc: linux-usb@vger.kernel.org
---
 drivers/usb/core/file.c |9 ++---
 1 file changed, 2 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/core/file.c b/drivers/usb/core/file.c
index e5387a4..eff9af9 100644
--- a/drivers/usb/core/file.c
+++ b/drivers/usb/core/file.c
@@ -19,6 +19,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 
 #include "usb.h"
@@ -163,7 +164,6 @@ int usb_register_dev(struct usb_interface *intf,
int minor_base = class_driver->minor_base;
int minor;
char name[20];
-   char *temp;
 
 #ifdef CONFIG_USB_DYNAMIC_MINORS
/* 
@@ -200,14 +200,9 @@ int usb_register_dev(struct usb_interface *intf,
 
/* create a usb class device for this usb interface */
snprintf(name, sizeof(name), class_driver->name, minor - minor_base);
-   temp = strrchr(name, '/');
-   if (temp && (temp[1] != '\0'))
-   ++temp;
-   else
-   temp = name;
intf->usb_dev = device_create(usb_class->class, &intf->dev,
  MKDEV(USB_MAJOR, minor), class_driver,
- "%s", temp);
+ "%s", kbasename(name));
if (IS_ERR(intf->usb_dev)) {
down_write(&minor_rwsem);
usb_minors[minor] = NULL;
-- 
1.7.10.4

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] usb: core: reuse kbasename()

[Andy Shevchenko]

On Tue, Oct 2, 2012 at 6:00 PM, Andy Shevchenko
 wrote:

> --- a/drivers/usb/core/file.c
> +++ b/drivers/usb/core/file.c

> @@ -200,14 +200,9 @@ int usb_register_dev(struct usb_interface *intf,
>
> /* create a usb class device for this usb interface */
> snprintf(name, sizeof(name), class_driver->name, minor - minor_base);
> -   temp = strrchr(name, '/');
> -   if (temp && (temp[1] != '\0'))
I have checked current linux-next, the drivers define .name in the
usb_class_driver structure as '...%d'.
So, what is the reason to check for trailing '/' here? Historical
reasons or there is a (broken/3rd party/etc) driver with it?

> -   ++temp;
> -   else
> -   temp = name;
> intf->usb_dev = device_create(usb_class->class, &intf->dev,
>   MKDEV(USB_MAJOR, minor), class_driver,
> - "%s", temp);
> + "%s", kbasename(name));



-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] usb: core: reuse kbasename()

[Greg Kroah-Hartman]

On Wed, Oct 03, 2012 at 11:27:27AM +0300, Andy Shevchenko wrote:
> On Tue, Oct 2, 2012 at 6:00 PM, Andy Shevchenko
>  wrote:
> 
> > --- a/drivers/usb/core/file.c
> > +++ b/drivers/usb/core/file.c
> 
> > @@ -200,14 +200,9 @@ int usb_register_dev(struct usb_interface *intf,
> >
> > /* create a usb class device for this usb interface */
> > snprintf(name, sizeof(name), class_driver->name, minor - 
> > minor_base);
> > -   temp = strrchr(name, '/');
> > -   if (temp && (temp[1] != '\0'))
> I have checked current linux-next, the drivers define .name in the
> usb_class_driver structure as '...%d'.
> So, what is the reason to check for trailing '/' here? Historical
> reasons or there is a (broken/3rd party/etc) driver with it?

I really do not remember why it was done this way, sorry.  I have no
problem not doing it anymore, as long as you are willing to fix any
potential bugs that might pop up :)

And no, I don't worry about 3rd party drivers, that shouldn't be an
issue at all here.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCH 6/7] usb: core: reuse kbasename()

[Andy Shevchenko]

On Wed, Oct 3, 2012 at 6:08 PM, Greg Kroah-Hartman
 wrote:
> On Wed, Oct 03, 2012 at 11:27:27AM +0300, Andy Shevchenko wrote:
>> On Tue, Oct 2, 2012 at 6:00 PM, Andy Shevchenko
>>  wrote:
>>
>> > --- a/drivers/usb/core/file.c
>> > +++ b/drivers/usb/core/file.c
>>
>> > @@ -200,14 +200,9 @@ int usb_register_dev(struct usb_interface *intf,
>> >
>> > /* create a usb class device for this usb interface */
>> > snprintf(name, sizeof(name), class_driver->name, minor - 
>> > minor_base);
>> > -   temp = strrchr(name, '/');
>> > -   if (temp && (temp[1] != '\0'))
>> I have checked current linux-next, the drivers define .name in the
>> usb_class_driver structure as '...%d'.
>> So, what is the reason to check for trailing '/' here? Historical
>> reasons or there is a (broken/3rd party/etc) driver with it?
>
> I really do not remember why it was done this way, sorry.  I have no
> problem not doing it anymore, as long as you are willing to fix any
> potential bugs that might pop up :)

Hmm... this series about cleaning up. The bugs might pop up in the
drivers that still using something like '/foo/bar/' for their names
here.
Anyway, I tried to dig into history and only what I found is the patch
http://www.kernel.org/pub/linux/kernel//people/akpm/patches/2.5/2.5.69/2.5.69-mm3/broken-out/linus.patch
that brings a piece of code. And it the same time it brings the same
piece to the tty layer. I suspect that this piece was copied and
pasted in few place.

Currently the device_create() call uses the name parameter as a
parameter of kobject. But kobject doesn't accept '/' in the names, it
changes it to '!'.
So, I think the way of treating a trailing slash in the usb code is redundant.

> And no, I don't worry about 3rd party drivers, that shouldn't be an
> issue at all here.

Fair enough


-- 
With Best Regards,
Andy Shevchenko
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html