PATCPATCH -- add unlimited name lengths support to sysfs

PATCPATCH -- add unlimited name lengths support to sysfs

[Linda Xie]

Hi All,

In the development of RPA PCI Hot Plug Controller driver, I have found 
it is needed to create some kernel objects which have more than 20 
(KOBJECT_NAME_SIZE) charaters in their name strings. At a later time the 
names will be used for creating some symlinks. The attached patch adds 
unlimited name lengths support to sysfs symlink.c.

  Comments are welcome.

Linda


diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
--- a/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
+++ b/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
@@ -42,7 +42,10 @@
  	struct kobject * p = kobj;
  	int length = 1;
  	do {
-		length += strlen(p->name) + 1;
+		if (p->k_name)
+			length += strlen(p->k_name) + 1;
+		else
+			length += strlen(p->name) + 1;
  		p = p->parent;
  	} while (p);
  	return length;
@@ -54,11 +57,20 @@

  	--length;
  	for (p = kobj; p; p = p->parent) {
-		int cur = strlen(p->name);
-
+		int cur;
+		char *name;
+		
+		if (p->k_name) {
+			cur = strlen(p->k_name);
+			name = p->k_name;
+		}
+		else {
+			cur = strlen(p->name);
+			name = p->name;
+		}
  		/* back up enough to print this bus id with '/' */
  		length -= cur;
-		strncpy(buffer + length,p->name,cur);
+		strncpy(buffer + length,name,cur);
  		*(buffer + --length) = '/';
  	}
  }

Re: PATCPATCH -- add unlimited name lengths support to sysfs

[Greg KH]

On Tue, Dec 16, 2003 at 05:07:22PM -0600, Linda Xie wrote:
> diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> --- a/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
> +++ b/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
> @@ -42,7 +42,10 @@
>  	struct kobject * p = kobj;
>  	int length = 1;
>  	do {
> -		length += strlen(p->name) + 1;
> +		if (p->k_name)
> +			length += strlen(p->k_name) + 1;
> +		else
> +			length += strlen(p->name) + 1;

Shouldn't this just be:
		length += strlen(kobject_name(p)) + 1;


> @@ -54,11 +57,20 @@
> 
>  	--length;
>  	for (p = kobj; p; p = p->parent) {
> -		int cur = strlen(p->name);
> -
> +		int cur;
> +		char *name;
> +		
> +		if (p->k_name) {
> +			cur = strlen(p->k_name);
> +			name = p->k_name;
> +		}
> +		else {
> +			cur = strlen(p->name);
> +			name = p->name;
> +		}

Same here, just use kobject_name() to get the proper pointer.

thanks,

greg k-h

Re: PATCPATCH -- add unlimited name lengths support to sysfs

[Linda Xie]

Greg KH wrote:
> On Tue, Dec 16, 2003 at 05:07:22PM -0600, Linda Xie wrote:
> 
>>diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
>>--- a/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
>>+++ b/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
>>@@ -42,7 +42,10 @@
>> 	struct kobject * p = kobj;
>> 	int length = 1;
>> 	do {
>>-		length += strlen(p->name) + 1;
>>+		if (p->k_name)
>>+			length += strlen(p->k_name) + 1;
>>+		else
>>+			length += strlen(p->name) + 1;
> 
> 
> Shouldn't this just be:
> 		length += strlen(kobject_name(p)) + 1;
> 

That is correct. But here is my concern: Some of the callers of 
sysfs_create_link()
set p->name instead of p->k_name. So for them, the length calculated 
using kobject_name(p) will be incorrect. Correct me if I am wrong.

Thanks,

Linda

> 
>>@@ -54,11 +57,20 @@
>>
>> 	--length;
>> 	for (p = kobj; p; p = p->parent) {
>>-		int cur = strlen(p->name);
>>-
>>+		int cur;
>>+		char *name;
>>+		
>>+		if (p->k_name) {
>>+			cur = strlen(p->k_name);
>>+			name = p->k_name;
>>+		}
>>+		else {
>>+			cur = strlen(p->name);
>>+			name = p->name;
>>+		}
> 
> 
> Same here, just use kobject_name() to get the proper pointer.
> 
> thanks,
> 
> greg k-h


Thanks,

Linda

Re: PATCPATCH -- add unlimited name lengths support to sysfs

[Greg KH]

On Wed, Dec 17, 2003 at 02:27:57PM -0600, Linda Xie wrote:
> Greg KH wrote:
> >On Tue, Dec 16, 2003 at 05:07:22PM -0600, Linda Xie wrote:
> >
> >>diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> >>--- a/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
> >>+++ b/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
> >>@@ -42,7 +42,10 @@
> >>	struct kobject * p = kobj;
> >>	int length = 1;
> >>	do {
> >>-		length += strlen(p->name) + 1;
> >>+		if (p->k_name)
> >>+			length += strlen(p->k_name) + 1;
> >>+		else
> >>+			length += strlen(p->name) + 1;
> >
> >
> >Shouldn't this just be:
> >		length += strlen(kobject_name(p)) + 1;
> >
> 
> That is correct. But here is my concern: Some of the callers of 
> sysfs_create_link()
> set p->name instead of p->k_name. So for them, the length calculated 
> using kobject_name(p) will be incorrect. Correct me if I am wrong.

Well if a kobject only uses the .name field, .k_name will point to it
(see kobject_add()), so the kobject_name() call will work in the above
case (as it should always do.)  Actually that if (p->k_name) statement
will always be true because of this fact :)

This lets people like the edd driver which does:
	 snprintf(edev->kobj.name, EDD_DEVICE_NAME_SIZE, "int13_dev%02x", edd[i].device);
still work properly.  Ideally, callers like this should change to use
the kobject_set_name() function, but there's no rush.

thanks,

greg k-h

Re: PATCPATCH -- add unlimited name lengths support to sysfs

[Linda Xie]

Greg KH wrote:
> On Wed, Dec 17, 2003 at 02:27:57PM -0600, Linda Xie wrote:
> 
>>Greg KH wrote:
>>
>>>On Tue, Dec 16, 2003 at 05:07:22PM -0600, Linda Xie wrote:
>>>
>>>
>>>>diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
>>>>--- a/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
>>>>+++ b/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
>>>>@@ -42,7 +42,10 @@
>>>>	struct kobject * p = kobj;
>>>>	int length = 1;
>>>>	do {
>>>>-		length += strlen(p->name) + 1;
>>>>+		if (p->k_name)
>>>>+			length += strlen(p->k_name) + 1;
>>>>+		else
>>>>+			length += strlen(p->name) + 1;
>>>
>>>
>>>Shouldn't this just be:
>>>		length += strlen(kobject_name(p)) + 1;
>>>
>>
>>That is correct. But here is my concern: Some of the callers of 
>>sysfs_create_link()
>>set p->name instead of p->k_name. So for them, the length calculated 
>>using kobject_name(p) will be incorrect. Correct me if I am wrong.
> 
> 
> Well if a kobject only uses the .name field, .k_name will point to it
> (see kobject_add()), so the kobject_name() call will work in the above
> case (as it should always do.)  Actually that if (p->k_name) statement
> will always be true because of this fact :)
> 
> This lets people like the edd driver which does:
> 	 snprintf(edev->kobj.name, EDD_DEVICE_NAME_SIZE, "int13_dev%02x", edd[i].device);
> still work properly.  Ideally, callers like this should change to use
> the kobject_set_name() function, but there's no rush.
> 
> thanks,
> 
> greg k-h
> 

Thank you very much. Below is an updated patch:


diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
--- a/fs/sysfs/symlink.c        Wed Dec 17 20:08:01 2003
+++ b/fs/sysfs/symlink.c        Wed Dec 17 20:08:01 2003
@@ -42,7 +42,7 @@
         struct kobject * p = kobj;
         int length = 1;
         do {
-               length += strlen(p->name) + 1;
+               length += strlen(kobject_name(p)) + 1;
                 p = p->parent;
         } while (p);
         return length;
@@ -54,11 +54,11 @@

         --length;
         for (p = kobj; p; p = p->parent) {
-               int cur = strlen(p->name);
+               int cur = strlen(kobject_name(p));

                 /* back up enough to print this bus id with '/' */
                 length -= cur;
-               strncpy(buffer + length,p->name,cur);
+               strncpy(buffer + length,kobject_name(p),cur);
                 *(buffer + --length) = '/';
         }


Linda

Re: PATCPATCH -- add unlimited name lengths support to sysfs

[Greg KH]

On Wed, Dec 17, 2003 at 08:23:06PM -0600, Linda Xie wrote:
> Greg KH wrote:
> >On Wed, Dec 17, 2003 at 02:27:57PM -0600, Linda Xie wrote:
> >
> >>Greg KH wrote:
> >>
> >>>On Tue, Dec 16, 2003 at 05:07:22PM -0600, Linda Xie wrote:
> >>>
> >>>
> >>>>diff -Nru a/fs/sysfs/symlink.c b/fs/sysfs/symlink.c
> >>>>--- a/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
> >>>>+++ b/fs/sysfs/symlink.c	Sun Dec 14 21:19:29 2003
> >>>>@@ -42,7 +42,10 @@
> >>>>	struct kobject * p = kobj;
> >>>>	int length = 1;
> >>>>	do {
> >>>>-		length += strlen(p->name) + 1;
> >>>>+		if (p->k_name)
> >>>>+			length += strlen(p->k_name) + 1;
> >>>>+		else
> >>>>+			length += strlen(p->name) + 1;
> >>>
> >>>
> >>>Shouldn't this just be:
> >>>		length += strlen(kobject_name(p)) + 1;
> >>>
> >>
> >>That is correct. But here is my concern: Some of the callers of 
> >>sysfs_create_link()
> >>set p->name instead of p->k_name. So for them, the length calculated 
> >>using kobject_name(p) will be incorrect. Correct me if I am wrong.
> >
> >
> >Well if a kobject only uses the .name field, .k_name will point to it
> >(see kobject_add()), so the kobject_name() call will work in the above
> >case (as it should always do.)  Actually that if (p->k_name) statement
> >will always be true because of this fact :)
> >
> >This lets people like the edd driver which does:
> >	 snprintf(edev->kobj.name, EDD_DEVICE_NAME_SIZE, "int13_dev%02x", 
> >	 edd[i].device);
> >still work properly.  Ideally, callers like this should change to use
> >the kobject_set_name() function, but there's no rush.
> >
> >thanks,
> >
> >greg k-h
> >
> 
> Thank you very much. Below is an updated patch:

Ick, that patch had no tabs :(

Care to fix your email client and send it to me again?

thanks,

greg k-h