Re: udevinfo patch

[Carl-Daniel Hailfinger <c-d.hailfinger.kernel.2004@gmx.net>]

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