udevinfo patch

udevinfo patch

[Daniel Stekloff]

Hi Greg,

I think this is what you want for udevinfo. Patched against the latest BK
tree. I tested it and it seemed to work.

One other question, shouldn't udevinfo.c:print_all_attributes() check to
make sure attr->method is SYSFS_METHOD_SHOW along with checking to see
if attr->value != NULL or doesn't that matter?

Here's the libsysfs fix for print_device_chain():

PATCH FOLLOWS:
-------------


--- udev/udevinfo.c	2004-03-23 10:00:49.178443896 -0800
+++ udev-fix/udevinfo.c	2004-03-23 13:40:19.197296704 -0800
@@ -158,7 +158,6 @@
 	       "to match the device for which the node will be created.\n"
 	       "\n");
 	printf("device '%s' has major:minor %s", class_dev->path, attr->value);
-	sysfs_close_attribute(attr);
 
 	/* open sysfs class device directory and print all attributes */
 	printf("  looking at class device '%s':\n", class_dev->path);
@@ -170,11 +169,11 @@
 
 	/* get the device link (if parent exists look here) */
 	class_dev_parent = sysfs_get_classdev_parent(class_dev);
-	if (class_dev_parent != NULL) {
-		//sysfs_close_class_device(class_dev);
-		class_dev = class_dev_parent;
-	}
-	sysfs_dev = sysfs_get_classdev_device(class_dev);
+	if (class_dev_parent != NULL) 
+		sysfs_dev = sysfs_get_classdev_device(class_dev_parent);
+	else 
+		sysfs_dev = sysfs_get_classdev_device(class_dev);
+	
 	if (sysfs_dev != NULL)
 		printf("follow the class device's \"device\"\n");
 
@@ -191,13 +190,11 @@
 		if (sysfs_dev_parent = NULL)
 			break;
 
-		//sysfs_close_device(sysfs_dev);
 		sysfs_dev = sysfs_dev_parent;
 	}
-	sysfs_close_device(sysfs_dev);
 
 exit:
-	//sysfs_close_class_device(class_dev);
+	sysfs_close_class_device(class_dev);
 	return retval;
 }
 


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Kay Sievers]

On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> 
> 
> Hi Greg,
> 
> I think this is what you want for udevinfo. Patched against the latest BK
> tree. I tested it and it seemed to work.

Hey, thanks for fixing this. I always wanted to ask why I needed to
comment out the close() calls.
Beeing lazy sometimes seems to work well :)

> One other question, shouldn't udevinfo.c:print_all_attributes() check to
> make sure attr->method is SYSFS_METHOD_SHOW along with checking to see
> if attr->value != NULL or doesn't that matter?

Seems that the value is always NULL, if the file does not have read
permissions. The sysfs_read_attribute() which sets the value is not
called if SYSFS_METHOD_SHOW is not valid.

thanks again,
Kay


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Greg KH]

On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> 
> 
> Hi Greg,
> 
> I think this is what you want for udevinfo. Patched against the latest BK
> tree. I tested it and it seemed to work.

Thanks, I've applied this.

greg k-h


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Carl-Daniel Hailfinger]

[-- Attachment #1: Type: text/plain, Size: 1061 bytes --]

Greg KH wrote:
> On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> 
>>
>>Hi Greg,
>>
>>I think this is what you want for udevinfo. Patched against the latest BK
>>tree. I tested it and it seemed to work.

It seems to have a bug:

carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block

udevinfo starts with the device the node belongs to and then walks up the
[...]
to match the device for which the node will be created.

  looking at class device '/sys/block':
couldn't open class device directory

carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block/

udevinfo starts with the device the node belongs to and then walks up the
[...]
to match the device for which the node will be created.

device '/sys/block' has major:minor 1:9
  looking at class device '/sys/block':
couldn't open class device directory


How can /sys/block have a major/minor number which changes everytime I
recompile udevinfo?


> Thanks, I've applied this.

Could you also apply the attached patch to clean up a bit?


Thanks,
Carl-Daniel

[-- Attachment #2: udevinfo.diff --]
[-- Type: text/plain, Size: 1374 bytes --]

diff -urN udev-023-orig/udevinfo.c udev-023/udevinfo.c
--- udev-023-orig/udevinfo.c	2004-03-25 01:09:52.000000000 +0100
+++ udev-023/udevinfo.c	2004-03-26 01:16:13.122194272 +0100
@@ -133,7 +133,7 @@
 static int print_device_chain(const char *path)
 {
 	struct sysfs_class_device *class_dev;
-	struct sysfs_class_device *class_dev_parent;
+	struct sysfs_class_device *class_dev_basedev;
 	struct sysfs_attribute *attr;
 	struct sysfs_device *sysfs_dev;
 	struct sysfs_device *sysfs_dev_parent;
@@ -166,6 +166,10 @@
 			goto exit;
 		}
 		printf("device '%s' has major:minor %s", class_dev->path, attr->value);
+	} else {
+		printf("this is not a block or char device\n");
+		retval =-1;
+		goto exit;
 	}
 
 	/* open sysfs class device directory and print all attributes */
@@ -176,12 +180,11 @@
 		goto exit;
 	}
 
-	/* get the device link (if parent exists look here) */
-	class_dev_parent = sysfs_get_classdev_parent(class_dev);
-	if (class_dev_parent != NULL) 
-		sysfs_dev = sysfs_get_classdev_device(class_dev_parent);
-	else 
-		sysfs_dev = sysfs_get_classdev_device(class_dev);
+	/* if parent exists, use that instead */
+	class_dev_basedev = sysfs_get_classdev_parent(class_dev) ? : class_dev;
+
+	/* get the device link */
+	sysfs_dev = sysfs_get_classdev_device(class_dev_basedev);
 	
 	if (sysfs_dev != NULL)
 		printf("follow the class device's \"device\"\n");

Re: udevinfo patch

[Kay Sievers]

On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> Greg KH wrote:
> > On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> > 
> >>
> >>Hi Greg,
> >>
> >>I think this is what you want for udevinfo. Patched against the latest BK
> >>tree. I tested it and it seemed to work.
> 
> It seems to have a bug:
> 
> carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block/
> 
> udevinfo starts with the device the node belongs to and then walks up the
> [...]
> to match the device for which the node will be created.
> 
> device '/sys/block' has major:minor 1:9
>   looking at class device '/sys/block':
> couldn't open class device directory
> 
> 
> How can /sys/block have a major/minor number which changes everytime I
> recompile udevinfo?

It's the attribute value read from the last sysfs device.

> > Thanks, I've applied this.
> 
> Could you also apply the attached patch to clean up a bit?

No, it breaks the net device handling. I think we should change
libsysfs instead, not to return a class device for '/block', if
we want to fix it.


> diff -urN udev-023-orig/udevinfo.c udev-023/udevinfo.c
> --- udev-023-orig/udevinfo.c	2004-03-25 01:09:52.000000000 +0100
> +++ udev-023/udevinfo.c	2004-03-26 01:16:13.122194272 +0100
> @@ -133,7 +133,7 @@
>  static int print_device_chain(const char *path)
>  {
>  	struct sysfs_class_device *class_dev;
> -	struct sysfs_class_device *class_dev_parent;
> +	struct sysfs_class_device *class_dev_basedev;
>  	struct sysfs_attribute *attr;
>  	struct sysfs_device *sysfs_dev;
>  	struct sysfs_device *sysfs_dev_parent;
> @@ -166,6 +166,10 @@
>  			goto exit;
>  		}
>  		printf("device '%s' has major:minor %s", class_dev->path, attr->value);
> +	} else {
> +		printf("this is not a block or char device\n");
> +		retval =-1;
> +		goto exit;

What's this?
It doesn't fix your "bug", '/block' is still a block device and prints
the garbage.

>  	}
>  
>  	/* open sysfs class device directory and print all attributes */
> @@ -176,12 +180,11 @@
>  		goto exit;
>  	}
>  
> -	/* get the device link (if parent exists look here) */
> -	class_dev_parent = sysfs_get_classdev_parent(class_dev);
> -	if (class_dev_parent != NULL) 
> -		sysfs_dev = sysfs_get_classdev_device(class_dev_parent);
> -	else 
> -		sysfs_dev = sysfs_get_classdev_device(class_dev);
> +	/* if parent exists, use that instead */
> +	class_dev_basedev = sysfs_get_classdev_parent(class_dev) ? : class_dev;

I don't see the reason to change this? What's the problem here?

> +
> +	/* get the device link */
> +	sysfs_dev = sysfs_get_classdev_device(class_dev_basedev);
>  	
>  	if (sysfs_dev != NULL)
>  		printf("follow the class device's \"device\"\n");


thanks,
Kay


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Carl-Daniel Hailfinger]

Kay Sievers wrote:
> On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> 
>>Greg KH wrote:
>>
>>>On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
>>>
>>>
>>>>Hi Greg,
>>>>
>>>>I think this is what you want for udevinfo. Patched against the latest BK
>>>>tree. I tested it and it seemed to work.
>>
>>It seems to have a bug:
>>
>>carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block/
>>
>>udevinfo starts with the device the node belongs to and then walks up the
>>[...]
>>to match the device for which the node will be created.
>>
>>device '/sys/block' has major:minor 1:9
>>  looking at class device '/sys/block':
>>couldn't open class device directory
>>
>>
>>How can /sys/block have a major/minor number which changes everytime I
>>recompile udevinfo?
> 
> 
> It's the attribute value read from the last sysfs device.

And why does it only happen with /sys/block/ and not with /sys/block
(Note the bug depends on the trailing slash).


>>>Thanks, I've applied this.
>>
>>Could you also apply the attached patch to clean up a bit?
> 
> 
> No, it breaks the net device handling. I think we should change

Sorry for that. My patch should not have changed change the behaviour of
udevinfo at all (except one warning). Feel free to delete the offending hunk.


> libsysfs instead, not to return a class device for '/block', if
> we want to fix it.

I agree. Daniel, can this be fixed in libsysfs?


>>diff -urN udev-023-orig/udevinfo.c udev-023/udevinfo.c
>>--- udev-023-orig/udevinfo.c	2004-03-25 01:09:52.000000000 +0100
>>+++ udev-023/udevinfo.c	2004-03-26 01:16:13.122194272 +0100
>>@@ -133,7 +133,7 @@
>> static int print_device_chain(const char *path)
>> {
>> 	struct sysfs_class_device *class_dev;
>>-	struct sysfs_class_device *class_dev_parent;
>>+	struct sysfs_class_device *class_dev_basedev;
>> 	struct sysfs_attribute *attr;
>> 	struct sysfs_device *sysfs_dev;
>> 	struct sysfs_device *sysfs_dev_parent;


>>@@ -166,6 +166,10 @@
>> 			goto exit;
>> 		}
>> 		printf("device '%s' has major:minor %s", class_dev->path, attr->value);
>>+	} else {
>>+		printf("this is not a block or char device\n");
>>+		retval =-1;
>>+		goto exit;
> 
> 
> What's this?
> It doesn't fix your "bug", '/block' is still a block device and prints
> the garbage.

Sorry, the above hunk is bogus. I was working with udev-022 and udev-023
at the same time and got confused.


> 
>> 	}
>> 
>> 	/* open sysfs class device directory and print all attributes */
>>@@ -176,12 +180,11 @@
>> 		goto exit;
>> 	}
>> 
>>-	/* get the device link (if parent exists look here) */
>>-	class_dev_parent = sysfs_get_classdev_parent(class_dev);
>>-	if (class_dev_parent != NULL) 
>>-		sysfs_dev = sysfs_get_classdev_device(class_dev_parent);
>>-	else 
>>-		sysfs_dev = sysfs_get_classdev_device(class_dev);
>>+	/* if parent exists, use that instead */
>>+	class_dev_basedev = sysfs_get_classdev_parent(class_dev) ? : class_dev;
> 
> 
> I don't see the reason to change this? What's the problem here?

As I said, this is a cleanup patch. It makes the code more readable (at
least for me).


>>+
>>+	/* get the device link */
>>+	sysfs_dev = sysfs_get_classdev_device(class_dev_basedev);
>> 	
>> 	if (sysfs_dev != NULL)
>> 		printf("follow the class device's \"device\"\n");


Regards,
Carl-Daniel



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Ananth N Mavinakayanahalli]

On Fri, Mar 26, 2004 at 04:47:35AM +0100, Kay Sievers wrote:
> On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> > Greg KH wrote:
> > > On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> > > 
> > >>
> > >>Hi Greg,
> > >>
> > >>I think this is what you want for udevinfo. Patched against the latest BK
> > >>tree. I tested it and it seemed to work.
> > 
> > It seems to have a bug:
> > 
> > carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block/
> > 
> > udevinfo starts with the device the node belongs to and then walks up the
> > [...]
> > to match the device for which the node will be created.
> > 
> > device '/sys/block' has major:minor 1:9
> >   looking at class device '/sys/block':
> > couldn't open class device directory
> > 
> > 
> > How can /sys/block have a major/minor number which changes everytime I
> > recompile udevinfo?
> 
> It's the attribute value read from the last sysfs device.
> 
> > > Thanks, I've applied this.
> > 
> > Could you also apply the attached patch to clean up a bit?
> 
> No, it breaks the net device handling. I think we should change
> libsysfs instead, not to return a class device for '/block', if
> we want to fix it.

/sys/block is considered a sysfs "class" and not a class_device. So, 
going by udevinfo's help, -p expects path to a class_device and _not_ 
a class itself and hence option /sys/block with -p is not a valid query. 

Kay?

Thanks,
Ananth


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Kay Sievers]

On Fri, Mar 26, 2004 at 06:25:36AM +0100, Carl-Daniel Hailfinger wrote:
> Kay Sievers wrote:
> > On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> > 
> >>Greg KH wrote:
> >>
> >>>On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> >>>
> >>>
> >>>>Hi Greg,
> >>>>
> >>>>I think this is what you want for udevinfo. Patched against the latest BK
> >>>>tree. I tested it and it seemed to work.
> >>
> >>It seems to have a bug:
> >>
> >>carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block/
> >>
> >>udevinfo starts with the device the node belongs to and then walks up the
> >>[...]
> >>to match the device for which the node will be created.
> >>
> >>device '/sys/block' has major:minor 1:9
> >>  looking at class device '/sys/block':
> >>couldn't open class device directory
> >>
> >>
> >>How can /sys/block have a major/minor number which changes everytime I
> >>recompile udevinfo?
> > 
> > 
> > It's the attribute value read from the last sysfs device.
> 
> And why does it only happen with /sys/block/ and not with /sys/block
> (Note the bug depends on the trailing slash).

Cause '/block' isn't recognized as a block device.

  strstr(path, "/block/") != NULL)

Kay


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Kay Sievers]

On Fri, Mar 26, 2004 at 11:21:29AM +0500, Ananth N Mavinakayanahalli wrote:
> On Fri, Mar 26, 2004 at 04:47:35AM +0100, Kay Sievers wrote:
> > On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> > > Greg KH wrote:
> > > > On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> > > > 
> > > >>
> > > >>Hi Greg,
> > > >>
> > > >>I think this is what you want for udevinfo. Patched against the latest BK
> > > >>tree. I tested it and it seemed to work.
> > > 
> > > It seems to have a bug:
> > > 
> > > carldani@4100xcdt:~/work/udev-023-orig> ./udevinfo -a -p /sys/block/
> > > 
> > > udevinfo starts with the device the node belongs to and then walks up the
> > > [...]
> > > to match the device for which the node will be created.
> > > 
> > > device '/sys/block' has major:minor 1:9
> > >   looking at class device '/sys/block':
> > > couldn't open class device directory
> > > 
> > > 
> > > How can /sys/block have a major/minor number which changes everytime I
> > > recompile udevinfo?
> > 
> > It's the attribute value read from the last sysfs device.
> > 
> > > > Thanks, I've applied this.
> > > 
> > > Could you also apply the attached patch to clean up a bit?
> > 
> > No, it breaks the net device handling. I think we should change
> > libsysfs instead, not to return a class device for '/block', if
> > we want to fix it.
> 
> /sys/block is considered a sysfs "class" and not a class_device. So, 
> going by udevinfo's help, -p expects path to a class_device and _not_ 
> a class itself and hence option /sys/block with -p is not a valid query. 
> 
> Kay?

Yes, it's invalid, but we shouldn't print major minor for a invalid
path. sysfs_open_class_device_path("/block") returns a device. If this is
the right behavior for libsysfs, I will change the get_device_type("/block")
not to return a 'b'-type.

thanks,
Kay


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Ananth N Mavinakayanahalli]

On Fri, Mar 26, 2004 at 11:24:39AM +0100, Kay Sievers wrote:
> On Fri, Mar 26, 2004 at 11:21:29AM +0500, Ananth N Mavinakayanahalli wrote:
> > On Fri, Mar 26, 2004 at 04:47:35AM +0100, Kay Sievers wrote:
> > > On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> > > > Greg KH wrote:
> > > > > On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> > > 
> > > No, it breaks the net device handling. I think we should change
> > > libsysfs instead, not to return a class device for '/block', if
> > > we want to fix it.
> > 
> > /sys/block is considered a sysfs "class" and not a class_device. So, 
> > going by udevinfo's help, -p expects path to a class_device and _not_ 
> > a class itself and hence option /sys/block with -p is not a valid query. 
> > 
> > Kay?
> 
> Yes, it's invalid, but we shouldn't print major minor for a invalid
> path. sysfs_open_class_device_path("/block") returns a device. If this is
> the right behavior for libsysfs, I will change the get_device_type("/block")
> not to return a 'b'-type.

Libsysfs validates the path given to it for opening a class_device to be
a valid directory; it does not however validate if the path is a valid
class_device path. So, in the case of udevinfo, a 'b' type should not 
be returned if the path is just /sys/block or /sys/block/


Thanks,
Ananth



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Carl-Daniel Hailfinger]

Ananth N Mavinakayanahalli wrote:
> On Fri, Mar 26, 2004 at 11:24:39AM +0100, Kay Sievers wrote:
> 
>>On Fri, Mar 26, 2004 at 11:21:29AM +0500, Ananth N Mavinakayanahalli wrote:
>>
>>>On Fri, Mar 26, 2004 at 04:47:35AM +0100, Kay Sievers wrote:
>>>
>>>>On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
>>>>
>>>>>Greg KH wrote:
>>>>>
>>>>>>On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
>>>>
>>>>No, it breaks the net device handling. I think we should change
>>>>libsysfs instead, not to return a class device for '/block', if
>>>>we want to fix it.
>>>
>>>/sys/block is considered a sysfs "class" and not a class_device. So, 
>>>going by udevinfo's help, -p expects path to a class_device and _not_ 
>>>a class itself and hence option /sys/block with -p is not a valid query. 
>>>
>>>Kay?
>>
>>Yes, it's invalid, but we shouldn't print major minor for a invalid
>>path. sysfs_open_class_device_path("/block") returns a device. If this is
>>the right behavior for libsysfs, I will change the get_device_type("/block")
>>not to return a 'b'-type.
> 
> 
> Libsysfs validates the path given to it for opening a class_device to be
> a valid directory; it does not however validate if the path is a valid
> class_device path. So, in the case of udevinfo, a 'b' type should not 
> be returned if the path is just /sys/block or /sys/block/

Regardless of that, libsysfs should not claim something has a major/minor
if it has not.


Regards,
Carl-Daniel



-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel

Re: udevinfo patch

[Kay Sievers]

[-- Attachment #1: Type: text/plain, Size: 1491 bytes --]

On Fri, Mar 26, 2004 at 06:36:32PM +0500, Ananth N Mavinakayanahalli wrote:
> On Fri, Mar 26, 2004 at 11:24:39AM +0100, Kay Sievers wrote:
> > On Fri, Mar 26, 2004 at 11:21:29AM +0500, Ananth N Mavinakayanahalli wrote:
> > > On Fri, Mar 26, 2004 at 04:47:35AM +0100, Kay Sievers wrote:
> > > > On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> > > > > Greg KH wrote:
> > > > > > On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> > > > 
> > > > No, it breaks the net device handling. I think we should change
> > > > libsysfs instead, not to return a class device for '/block', if
> > > > we want to fix it.
> > > 
> > > /sys/block is considered a sysfs "class" and not a class_device. So, 
> > > going by udevinfo's help, -p expects path to a class_device and _not_ 
> > > a class itself and hence option /sys/block with -p is not a valid query. 
> > > 
> > > Kay?
> > 
> > Yes, it's invalid, but we shouldn't print major minor for a invalid
> > path. sysfs_open_class_device_path("/block") returns a device. If this is
> > the right behavior for libsysfs, I will change the get_device_type("/block")
> > not to return a 'b'-type.
> 
> Libsysfs validates the path given to it for opening a class_device to be
> a valid directory; it does not however validate if the path is a valid
> class_device path. So, in the case of udevinfo, a 'b' type should not 
> be returned if the path is just /sys/block or /sys/block/

This may prevent it.

thanks,
Kay

[-- Attachment #2: 03-get_dev.patch --]
[-- Type: text/plain, Size: 1010 bytes --]

===== udev_lib.c 1.3 vs edited =====
--- 1.3/udev_lib.c	Thu Mar 25 00:50:34 2004
+++ edited/udev_lib.c	Fri Mar 26 18:18:34 2004
@@ -81,17 +81,28 @@
 	return subsystem;
 }
 
+#define BLOCK_PATH		"/block/"
+#define CLASS_PATH		"/class/"
+#define NET_PATH		"/class/net/"
+
 char get_device_type(const char *path, const char *subsystem)
 {
-	if (strcmp(subsystem, "block") == 0 ||
-	    strstr(path, "/block/") != NULL)
+	if (strcmp(subsystem, "block") == 0)
+		return 'b';
+
+	if (strcmp(subsystem, "net") == 0)
+		return 'n';
+
+	if (strncmp(path, BLOCK_PATH, strlen(BLOCK_PATH)) == 0 &&
+	    strlen(path) > strlen(BLOCK_PATH))
 		return 'b';
 
-	if (strcmp(subsystem, "net") == 0 ||
-	    strstr(path, "/class/net/") != NULL)
+	if (strncmp(path, NET_PATH, strlen(NET_PATH)) == 0 &&
+	    strlen(path) > strlen(NET_PATH))
 		return 'n';
 
-	if (strstr(path, "/class/") != NULL)
+	if (strncmp(path, CLASS_PATH, strlen(CLASS_PATH)) == 0 &&
+	    strlen(path) > strlen(CLASS_PATH))
 		return 'c';
 
 	return '\0';

Re: udevinfo patch

[Greg KH]

On Fri, Mar 26, 2004 at 06:19:47PM +0100, Kay Sievers wrote:
> On Fri, Mar 26, 2004 at 06:36:32PM +0500, Ananth N Mavinakayanahalli wrote:
> > On Fri, Mar 26, 2004 at 11:24:39AM +0100, Kay Sievers wrote:
> > > On Fri, Mar 26, 2004 at 11:21:29AM +0500, Ananth N Mavinakayanahalli wrote:
> > > > On Fri, Mar 26, 2004 at 04:47:35AM +0100, Kay Sievers wrote:
> > > > > On Fri, Mar 26, 2004 at 01:26:46AM +0100, Carl-Daniel Hailfinger wrote:
> > > > > > Greg KH wrote:
> > > > > > > On Tue, Mar 23, 2004 at 01:51:01PM -0800, Daniel Stekloff wrote:
> > > > > 
> > > > > No, it breaks the net device handling. I think we should change
> > > > > libsysfs instead, not to return a class device for '/block', if
> > > > > we want to fix it.
> > > > 
> > > > /sys/block is considered a sysfs "class" and not a class_device. So, 
> > > > going by udevinfo's help, -p expects path to a class_device and _not_ 
> > > > a class itself and hence option /sys/block with -p is not a valid query. 
> > > > 
> > > > Kay?
> > > 
> > > Yes, it's invalid, but we shouldn't print major minor for a invalid
> > > path. sysfs_open_class_device_path("/block") returns a device. If this is
> > > the right behavior for libsysfs, I will change the get_device_type("/block")
> > > not to return a 'b'-type.
> > 
> > Libsysfs validates the path given to it for opening a class_device to be
> > a valid directory; it does not however validate if the path is a valid
> > class_device path. So, in the case of udevinfo, a 'b' type should not 
> > be returned if the path is just /sys/block or /sys/block/
> 
> This may prevent it.

Applied, thanks.

greg k-h


-------------------------------------------------------
This SF.Net email is sponsored by: IBM Linux Tutorials
Free Linux tutorial presented by Daniel Robbins, President and CEO of
GenToo technologies. Learn everything from fundamentals to system
administration.http://ads.osdn.com/?ad_id\x1470&alloc_id638&op=click
_______________________________________________
Linux-hotplug-devel mailing list  http://linux-hotplug.sourceforge.net
Linux-hotplug-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-hotplug-devel