[PATCH] namedev.c %c{N+} capability + buglet with SYMLINK

[PATCH] namedev.c %c{N+} capability + buglet with SYMLINK

[Martin Schwenke]

I want to be able to use naming callouts that follow a convention of
providing the canonical device name as the first word, and
aliases/symlinks as subsequent words.  That is, my callout might
print:

  hdc cdrom bert ernie

so the canonical name will be "hdc", and "cdrom, "bert" and "ernie"
will be aliases/symlinks.  I can probably implement this is in
slightly gross fashion by having the result match against the number
spaces in the result, and then do the appropriate number of
substitutions.

The following patch almost let's me have the following configuration:

  PROGRAM="/sbin/aliaser %b %k %n %M %m", RESULT="?*", NAME="%c{1}", SYMLINK="%c{2+}"

allowing me to specify an arbitrary number of symlinks by saying "give
me the second and later words".  I thought of implementing something
more complex (involving arbitrary ranges of words), but I think the
implementation below provides all the functionality that is needed.

The only gotcha is that the rule won't apply when SYMLINK gets an
empty value (that is, there is only 1 word).  This seems kind-of
broken, since it stops you using a general rule to do the symlinks,
and I think the semantics of an empty SYMLINK value are pretty clear
(i.e. no symlinks please).

peace & happiness,
martin

--- udev-021.orig/namedev.c	2004-03-03 11:01:33.000000000 +1100
+++ udev-021/namedev.c	2004-03-05 12:25:55.000000000 +1100
@@ -223,6 +223,8 @@
 	int len;
 	int i;
 	char c;
+	char c2;
+	int rest;
 	struct sysfs_attribute *tmpattr;
 
 	pos = string;
@@ -276,26 +278,31 @@
 			/* get part part of the result string */
 			i = 0;
 			if (attr != NULL)
-				i = atoi(attr);
-			if (i > 0) {
-				strfieldcpy(temp1, udev->program_result);
-				pos2 = temp1;
-				while (i) {
+				i = strtol(attr, &pos2, 10);
+			if ((pos2 != attr) && (i > 0))
+				rest = (*pos2 = '+');
+			else {
+				i = rest = 1;
+			}
+			pos2 = udev->program_result;
+			while ((pos2 != NULL) && (i > 1)) {
+				pos2 = strchr(pos2, ' ');
+				if (pos2) {
 					i--;
-					pos3 = strsep(&pos2, " ");
-					if (pos3 = NULL) {
-						dbg("requested part of result string not found");
-						break;
-					}
+					pos2++;
 				}
-				if (pos3) {
-					strnfieldcat(string, pos3, maxsize);
-					dbg("substitute part of result string '%s'", pos3);
-				}
-			} else {
-				strnfieldcat(string, udev->program_result, maxsize);
-				dbg("substitute result string '%s'", udev->program_result);
 			}
+			if (pos2 != NULL) {
+				c2 = rest ? '\0' : ' ';
+				pos3 = temp1;
+				while (*pos2 != c2)
+					*pos3++ = *pos2++;
+				*pos3 = '\0';
+			} else
+				*temp1 = '\0';
+			
+			strnfieldcat(string, temp1, maxsize);
+			dbg("substitute (part of) result string '%s'", temp1);
 			break;
 		case 's':
 			if (attr != NULL) {



-------------------------------------------------------
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: [PATCH] namedev.c %c{N+} capability + buglet with SYMLINK

[Kay Sievers]

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

On Mon, Mar 08, 2004 at 12:49:44PM +1100, Martin Schwenke wrote:
> I want to be able to use naming callouts that follow a convention of
> providing the canonical device name as the first word, and
> aliases/symlinks as subsequent words.  That is, my callout might
> print:
> 
>   hdc cdrom bert ernie
> 
> so the canonical name will be "hdc", and "cdrom, "bert" and "ernie"
> will be aliases/symlinks.  I can probably implement this is in
> slightly gross fashion by having the result match against the number
> spaces in the result, and then do the appropriate number of
> substitutions.
> 
> The following patch almost let's me have the following configuration:
> 
>   PROGRAM="/sbin/aliaser %b %k %n %M %m", RESULT="?*", NAME="%c{1}", SYMLINK="%c{2+}"
> 
> allowing me to specify an arbitrary number of symlinks by saying "give
> me the second and later words".  I thought of implementing something
> more complex (involving arbitrary ranges of words), but I think the
> implementation below provides all the functionality that is needed.

Nice idea, but the code you've patched is gone:

  http://linuxusb.bkbits.net:8080/udev/anno/namedev.c@1.125?nav=index.html|src/

Hey, it was too ugly and you try to make it more ugly :)

Here is a patch for the current version, but there are more patches
pending and we need to change it again.

thanks,
Kay

[-- Attachment #2: 06-result-selector-plus.patch --]
[-- Type: text/plain, Size: 1450 bytes --]

===== namedev.c 1.125 vs edited =====
--- 1.125/namedev.c	Thu Mar  4 14:38:53 2004
+++ edited/namedev.c	Mon Mar  8 03:22:43 2004
@@ -222,6 +222,7 @@
 	int i;
 	char c;
 	char *spos;
+	char *rest;
 	int slen;
 	struct sysfs_attribute *tmpattr;
 
@@ -276,7 +277,7 @@
 			/* get part part of the result string */
 			i = 0;
 			if (attr != NULL)
-				i = atoi(attr);
+				i = strtoul(attr, &rest, 10);
 			if (i > 0) {
 				foreach_strpart(udev->program_result, " \n\r", spos, slen) {
 					i--;
@@ -287,7 +288,10 @@
 					dbg("requested part of result string not found");
 					break;
 				}
-				strnfieldcpy(temp2, spos, slen+1);
+				if (rest[0] == '+')
+					strfieldcpy(temp2, spos);
+				else
+					strnfieldcpy(temp2, spos, slen+1);
 				strnfieldcat(string, temp2, maxsize);
 				dbg("substitute part of result string '%s'", temp2);
 			} else {
===== test/udev-test.pl 1.42 vs edited =====
--- 1.42/test/udev-test.pl	Thu Mar  4 14:38:53 2004
+++ edited/test/udev-test.pl	Mon Mar  8 03:24:57 2004
@@ -289,6 +289,15 @@
 EOF
 	},
 	{
+		desc     => "program result substitution (numbered part of+)",
+		subsys   => "block",
+		devpath  => "block/sda/sda3",
+		expected => "link3" ,
+		conf     => <<EOF
+BUS="scsi", PROGRAM="/bin/echo -n node link1 link2 link3 link4", RESULT="node *", NAME="%c{1}", SYMLINK="%c{2+}"
+EOF
+	},
+	{
 		desc     => "invalid program for device with no bus",
 		subsys   => "tty",
 		devpath  => "class/tty/console",

Re: [PATCH] namedev.c %c{N+} capability + buglet with SYMLINK

[Martin Schwenke]

>>>>> "Kay" = Kay Sievers <kay.sievers@vrfy.org> writes:

    Kay> Nice idea, but the code you've patched is gone:

    Kay>   http://linuxusb.bkbits.net:8080/udev/anno/namedev.c@1.125?nav=index.html|src/

Good!  :-)

    Kay> Hey, it was too ugly and you try to make it more ugly :)

Yep, I was trying for the most compact patch, without trying to change
the style of the existing code...

    Kay> Here is a patch for the current version, but there are more
    Kay> patches pending and we need to change it again.

... and the new code is much more elegant, which is shown by the fact
that the feature I want can be slotted in so nicely...  :-)

2 questions:

* Can '\t' please be a separator too?

* Can we please merge your '+' patch?

Now, about that other bit:

    Martin>   PROGRAM="/sbin/aliaser %b %k %n %M %m", RESULT="?*", NAME="%c{1}", SYMLINK="%c{2+}"

    Martin> [...]

    Martin> The only gotcha is that the rule won't apply when SYMLINK
    Martin> gets an empty value (that is, there is only 1 word).  This
    Martin> seems kind-of broken, since it stops you using a general
    Martin> rule to do the symlinks, and I think the semantics of an
    Martin> empty SYMLINK value are pretty clear (i.e. no symlinks
    Martin> please).

Looking at namedev.c and udev-add.c, I can't see why SYMLINK
(dev->symlink, udev->symlink) being set to empty would stop the node
for NAME being created, but that does seem to be the case.

Any ideas?

Thanks...

peace & happiness,
martin



-------------------------------------------------------
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: [PATCH] namedev.c %c{N+} capability + buglet with SYMLINK

[Kay Sievers]

On Mon, 2004-03-08 at 06:02, Martin Schwenke wrote:
> >>>>> "Kay" = Kay Sievers <kay.sievers@vrfy.org> writes:

> * Can '\t' please be a separator too?

Yes, i've put this on my list.

> * Can we please merge your '+' patch?

Sure, there are 5 other patches pending, sone touching this area.
So I will wait until these are merged. It's the number 6.

> Now, about that other bit:
> 
>     Martin>   PROGRAM="/sbin/aliaser %b %k %n %M %m", RESULT="?*", NAME="%c{1}", SYMLINK="%c{2+}"
> 
>     Martin> [...]
> 
>     Martin> The only gotcha is that the rule won't apply when SYMLINK
>     Martin> gets an empty value (that is, there is only 1 word).  This
>     Martin> seems kind-of broken, since it stops you using a general
>     Martin> rule to do the symlinks, and I think the semantics of an
>     Martin> empty SYMLINK value are pretty clear (i.e. no symlinks
>     Martin> please).
> 
> Looking at namedev.c and udev-add.c, I can't see why SYMLINK
> (dev->symlink, udev->symlink) being set to empty would stop the node
> for NAME being created, but that does seem to be the case.

Hmm, this one works for me:
BUS="scsi", PROGRAM="/bin/echo -n node link1 link2", RESULT="node *", NAME="%c{1}", SYMLINK="%c{4+}"

Could you please build udev with 'make DEBUG=true'.
It will be very verbose to the syslog and we may
find the reason there.

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: [PATCH] namedev.c %c{N+} capability + buglet with SYMLINK

[Martin Schwenke]

>>>>> "Kay" = Kay Sievers <kay.sievers@vrfy.org> writes:

    Martin> PROGRAM="/sbin/aliaser %b %k %n %M %m", RESULT="?*", NAME="%c{1}", SYMLINK="%c{2+}"

    Martin> [...]

    Martin> The only gotcha is that the rule won't apply when SYMLINK
    Martin> gets an empty value [...]


    Kay> Hmm, this one works for me:
    Kay> BUS="scsi", PROGRAM="/bin/echo -n node link1 link2", RESULT="node *", NAME="%c{1}", SYMLINK="%c{4+}"

    Kay> Could you please build udev with 'make DEBUG=true'.
    Kay> It will be very verbose to the syslog and we may
    Kay> find the reason there.

I was running with DEBUG=true, but I wasn't seeing anything useful.  :-(

Now I get it: either the original ugly code or my patch was causing
stack corruption and udev was crashing on exit from
namedev.c:apply_format() (but not every time).  If I inserted an extra
debug before the end of apply_format, I would see it, but one after
the call (that processes udev->name) would not be printed.

Grabbing the version in BK and applying your '+' patch fixes the
problem...

Thanks!

peace & happiness,
martin



-------------------------------------------------------
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