Re: udevinfo patch

[Kay Sievers <kay.sievers@vrfy.org>]

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