[PATCH] Adding '%s' format specifier to NAME and SYMLINK

[PATCH] Adding '%s' format specifier to NAME and SYMLINK

[Hannes Reinecke]

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

Hi all,

this patch makes the format for NAME and SYMLINK a bit more flexible: 
I've added a new format specifier '%s{<SYSFS_var>}', which allows for 
the value of any sysfs entry found for this device to be inserted.
Example (for our S/390 fcp adapter):

BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"

I know this could also be done with an external program, but having this 
incorporated into udev makes life easier, especially if run from 
initramfs. Plus it makes the rules easier to follow, as the result is 
directly visible and need not to be looked up in some external program.

Comments etc. welcome.

Cheers,

Hannes
-- 
Dr. Hannes Reinecke			hare@suse.de
SuSE Linux AG				S390 & zSeries
Maxfeldstraße 5				+49 911 74053 688
90409 Nürnberg				http://www.suse.de

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

===== namedev.c 1.98 vs edited =====
--- 1.98/namedev.c	Tue Jan 27 22:40:29 2004
+++ edited/namedev.c	Mon Feb  9 16:59:35 2004
@@ -168,7 +168,7 @@
 	return default_group_str;
 }
 
-static void apply_format(struct udevice *udev, unsigned char *string)
+static void apply_format(struct udevice *udev, unsigned char *string, struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device)
 {
 	char temp[NAME_SIZE];
 	char temp1[NAME_SIZE];
@@ -176,50 +176,75 @@
 	char *pos;
 	char *pos2;
 	char *pos3;
-	int num;
+	int num, len;
+	struct sysfs_attribute *tmpattr;
 
 	pos = string;
+	strcpy(temp, string);
+	tail = temp;
 	while (1) {
 		num = 0;
-		pos = strchr(pos, '%');
+		dbg("string is '%s' (index '%c')\n",string, *pos);
+#if 1
+		if (tail) {
+			pos2 = strchr(tail, '%');
+			if (!pos2) {
+				strcat(pos,tail);
+			} else {
+				if ((len = pos2 - tail) > 0) {
+					strncat(pos,tail,len);
+					pos += len;
+				}
+				*pos = '\0';
+			}
+			tail = pos2;
+		}
+		dbg("string '%s', tail '%s'\n", string, tail);
+#else
+		pos = strchr(pos,'%');
+#endif
 
-		if (pos) {
-			pos[0] = '\0';
-			tail = pos+1;
+		if (tail) {
+			tail ++;
 			if (isdigit(tail[0])) {
-				num = (int) strtoul(&pos[1], &tail, 10);
-				if (tail == NULL) {
-					dbg("format parsing error '%s'", pos+1);
+				num = (int) strtoul(tail, &pos2, 10);
+				if (pos2 == NULL) {
+					dbg("format parsing error '%s'", tail);
 					break;
 				}
+				tail = pos2;
 			}
-			strfieldcpy(temp, tail+1);
 
 			switch (tail[0]) {
 			case 'b':
 				if (strlen(udev->bus_id) == 0)
 					break;
 				strcat(pos, udev->bus_id);
+				tail++;
 				dbg("substitute bus_id '%s'", udev->bus_id);
 				break;
 			case 'k':
 				if (strlen(udev->kernel_name) == 0)
 					break;
 				strcat(pos, udev->kernel_name);
+				tail++;
 				dbg("substitute kernel name '%s'", udev->kernel_name);
 				break;
 			case 'n':
 				if (strlen(udev->kernel_number) == 0)
 					break;
 				strcat(pos, udev->kernel_number);
+				tail++;
 				dbg("substitute kernel number '%s'", udev->kernel_number);
 				break;
 			case 'm':
 				sprintf(pos, "%u", udev->minor);
+				tail++;
 				dbg("substitute minor number '%u'", udev->minor);
 				break;
 			case 'M':
 				sprintf(pos, "%u", udev->major);
+				tail++;
 				dbg("substitute major number '%u'", udev->major);
 				break;
 			case 'c':
@@ -245,16 +270,66 @@
 					strcat(pos, udev->program_result);
 					dbg("substitute result string '%s'", udev->program_result);
 				}
+				tail++;
 				break;
+			case 's':
+				if (tail[1] != '{') {
+					dbg("missing open braces for format");
+					break;
+				}
+				pos2 = tail + 2;
+				if (!(pos3 = strchr(pos2,'}'))) {
+					dbg("missing close brace for format");
+					break;
+				}
+				dbg("Tail is '%s'\n", temp);
+				*pos3 = '\0';
+				tail = pos3 + 1;
+				dbg("look for class attribute '%s'", pos2);
+				/* try to find the attribute in the class device directory */
+				tmpattr = sysfs_get_classdev_attr(class_dev, 
+								  pos2);
+				if (tmpattr) {
+				    pos3 = tmpattr->value + 
+					strlen(tmpattr->value)-1;
+				    if (*pos3 == '\n')
+					*pos3 = 0x00;
+				    strcpy(pos, tmpattr->value);
+				    pos += strlen(tmpattr->value);
+				    dbg("substitute sysfs value '%s'", 
+					tmpattr->value);
+				    break;
+				}
+				
+				/* look in the class device directory 
+				   if present */
+				if (!sysfs_device) {
+				    break;
+				}
+				dbg("look for device attribute '%s'", pos2);
+				tmpattr = sysfs_get_device_attr(sysfs_device, 
+								pos2);
+				if (tmpattr) {
+				    pos3 = tmpattr->value +
+					strlen(tmpattr->value)-1;
+				    if (*pos3 == '\n')
+					*pos3 = 0x00;
+				    strcpy(pos, tmpattr->value);
+				    pos += strlen(tmpattr->value);
+				    dbg("substitute sysfs value '%s'", 
+					tmpattr->value);
+				}				
+			        break;
+				
 			case '%':
 				strcat(pos, "%");
 				pos++;
 				break;
 			default:
-				dbg("unknown substitution type '%%%c'", pos[1]);
+				dbg("unknown substitution type '%%%c'", tail[1]);
 				break;
 			}
-			strcat(string, temp);
+/*			strcat(string, temp); */
 		} else
 			break;
 	}
@@ -647,7 +722,7 @@
 		/* execute external program */
 		if (dev->program[0] != '\0') {
 			dbg("check " FIELD_PROGRAM);
-			apply_format(udev, dev->program);
+			apply_format(udev, dev->program, class_dev, sysfs_device);
 			if (execute_program(dev->program, udev->program_result, NAME_SIZE) != 0) {
 				dbg(FIELD_PROGRAM " returned nozero");
 				goto no_good;
@@ -739,8 +814,8 @@
 
 found:
 	/* substitute placeholder */
-	apply_format(udev, udev->name);
-	apply_format(udev, udev->symlink);
+	apply_format(udev, udev->name, class_dev, sysfs_device);
+	apply_format(udev, udev->symlink, class_dev, sysfs_device);
 
 done:
 	perm = find_perm(udev->name);

Re: [PATCH] Adding '%s' format specifier to NAME and SYMLINK

[Greg KH]

On Tue, Feb 10, 2004 at 09:14:20AM +0100, Hannes Reinecke wrote:
> Hi all,
> 
> this patch makes the format for NAME and SYMLINK a bit more flexible: 
> I've added a new format specifier '%s{<SYSFS_var>}', which allows for 
> the value of any sysfs entry found for this device to be inserted.
> Example (for our S/390 fcp adapter):
> 
> BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
> SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
> 
> I know this could also be done with an external program, but having this 
> incorporated into udev makes life easier, especially if run from 
> initramfs. Plus it makes the rules easier to follow, as the result is 
> directly visible and need not to be looked up in some external program.
> 
> Comments etc. welcome.

Oops, sorry I missed this for the 017 release.  I'll look at it tomorrow
and get back to you.  At first glance it looks like a good thing.

Oh, you forgot to update the documentation, that's important to do if
you want this change to make it in :)

thanks,

greg k-h


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&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] Adding '%s' format specifier to NAME and SYMLINK

[Kay Sievers]

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

On Thu, Feb 12, 2004 at 05:34:57PM -0800, Greg KH wrote:
> On Tue, Feb 10, 2004 at 09:14:20AM +0100, Hannes Reinecke wrote:
> > Hi all,
> > 
> > this patch makes the format for NAME and SYMLINK a bit more flexible: 
> > I've added a new format specifier '%s{<SYSFS_var>}', which allows for 
> > the value of any sysfs entry found for this device to be inserted.
> > Example (for our S/390 fcp adapter):
> > 
> > BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
> > SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
> > 
> > I know this could also be done with an external program, but having this 
> > incorporated into udev makes life easier, especially if run from 
> > initramfs. Plus it makes the rules easier to follow, as the result is 
> > directly visible and need not to be looked up in some external program.
> > 
> > Comments etc. welcome.
> 
> Oops, sorry I missed this for the 017 release.  I'll look at it tomorrow
> and get back to you.  At first glance it looks like a good thing.
> 
> Oh, you forgot to update the documentation, that's important to do if
> you want this change to make it in :)

I took a part of the code and made a version that uses already implemented
attribute finding logic.

The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now,
maybe there are more possible users in the future.

I've also added the test to udev-test.pl.


thanks,
Kay

[-- Attachment #2: 01-s-format-char.patch --]
[-- Type: text/plain, Size: 8932 bytes --]

===== namedev.c 1.104 vs edited =====
--- 1.104/namedev.c	Thu Feb 12 23:42:13 2004
+++ edited/namedev.c	Sat Feb 14 19:14:42 2004
@@ -41,6 +41,8 @@
 #include "libsysfs/libsysfs.h"
 #include "klibc_fixups.h"
 
+static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr);
+
 LIST_HEAD(config_device_list);
 LIST_HEAD(perm_device_list);
 
@@ -168,7 +170,46 @@
 	return default_group_str;
 }
 
-static void apply_format(struct udevice *udev, unsigned char *string)
+/* extract possible {attr} and move str behind it */
+static char *get_format_attribute(char **str)
+{
+	char *pos;
+	char *attr = NULL;
+
+	if (*str[0] == '{') {
+		pos = strchr(*str, '}');
+		if (pos == NULL) {
+			dbg("missing closing brace for format");
+			return NULL;
+		}
+		pos[0] = '\0';
+		attr = *str+1;
+		*str = pos+1;
+		dbg("attribute='%s', str='%s'", attr, *str);
+	}
+	return attr;
+}
+
+/* extract possible format length and move str behind it*/
+static int get_format_len(char **str)
+{
+	int num;
+	char *tail;
+
+	if (isdigit(*str[0])) {
+		num = (int) strtoul(*str, &tail, 10);
+		if (tail != NULL) {
+			*str = tail;
+			dbg("format length=%i", num);
+			return num;
+		} else {
+			dbg("format parsing error '%s'", *str);
+		}
+	}
+	return -1;
+}
+
+static void apply_format(struct udevice *udev, unsigned char *string, struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device)
 {
 	char temp[NAME_SIZE];
 	char temp1[NAME_SIZE];
@@ -176,87 +217,100 @@
 	char *pos;
 	char *pos2;
 	char *pos3;
+	char *attr;
 	int num;
+	char c;
+	struct sysfs_attribute *tmpattr;
 
 	pos = string;
+
 	while (1) {
-		num = 0;
 		pos = strchr(pos, '%');
-
-		if (pos) {
+		if (pos != NULL) {
 			pos[0] = '\0';
 			tail = pos+1;
-			if (isdigit(tail[0])) {
-				num = (int) strtoul(&pos[1], &tail, 10);
-				if (tail == NULL) {
-					dbg("format parsing error '%s'", pos+1);
-					break;
-				}
-			}
+			num = get_format_len(&tail);
+			c = tail[0];
 			strfieldcpy(temp, tail+1);
+			tail = temp;
+		} else {
+			break;
+		}
+		dbg("format=%c, string='%s', tail='%s'",c , string, tail);
 
-			switch (tail[0]) {
-			case 'b':
-				if (strlen(udev->bus_id) == 0)
-					break;
-				strcat(pos, udev->bus_id);
-				dbg("substitute bus_id '%s'", udev->bus_id);
+		attr = get_format_attribute(&tail);
+
+		switch (c) {
+		case 'b':
+			if (strlen(udev->bus_id) == 0)
 				break;
-			case 'k':
-				if (strlen(udev->kernel_name) == 0)
-					break;
-				strcat(pos, udev->kernel_name);
-				dbg("substitute kernel name '%s'", udev->kernel_name);
+			strcat(pos, udev->bus_id);
+			dbg("substitute bus_id '%s'", udev->bus_id);
+			break;
+		case 'k':
+			if (strlen(udev->kernel_name) == 0)
 				break;
-			case 'n':
-				if (strlen(udev->kernel_number) == 0)
-					break;
-				strcat(pos, udev->kernel_number);
-				dbg("substitute kernel number '%s'", udev->kernel_number);
+			strcat(pos, udev->kernel_name);
+			dbg("substitute kernel name '%s'", udev->kernel_name);
+			break;
+		case 'n':
+			if (strlen(udev->kernel_number) == 0)
 				break;
-			case 'm':
-				sprintf(pos, "%u", udev->minor);
-				dbg("substitute minor number '%u'", udev->minor);
+			strcat(pos, udev->kernel_number);
+			dbg("substitute kernel number '%s'", udev->kernel_number);
 				break;
+		case 'm':
+			sprintf(pos, "%u", udev->minor);
+			dbg("substitute minor number '%u'", udev->minor);
+			break;
 			case 'M':
-				sprintf(pos, "%u", udev->major);
-				dbg("substitute major number '%u'", udev->major);
+			sprintf(pos, "%u", udev->major);
+			dbg("substitute major number '%u'", udev->major);
+			break;
+		case 'c':
+			if (strlen(udev->program_result) == 0)
 				break;
-			case 'c':
-				if (strlen(udev->program_result) == 0)
-					break;
-				if (num) {
-					/* get part of return string */
-					strncpy(temp1, udev->program_result, sizeof(temp1));
-					pos2 = temp1;
-					while (num) {
-						num--;
-						pos3 = strsep(&pos2, " ");
-						if (pos3 == NULL) {
-							dbg("requested part of result string not found");
-							break;
-						}
-					}
-					if (pos3) {
-						strcat(pos, pos3);
-						dbg("substitute part of result string '%s'", pos3);
+			if (num > 0) {
+				strncpy(temp1, udev->program_result, sizeof(temp1));
+				pos2 = temp1;
+				while (num) {
+					num--;
+					pos3 = strsep(&pos2, " ");
+					if (pos3 == NULL) {
+						dbg("requested part of result string not found");
+						break;
 					}
-				} else {
-					strcat(pos, udev->program_result);
-					dbg("substitute result string '%s'", udev->program_result);
 				}
-				break;
-			case '%':
-				strcat(pos, "%");
-				pos++;
-				break;
-			default:
-				dbg("unknown substitution type '%%%c'", pos[1]);
-				break;
+				if (pos3) {
+					strcat(pos, pos3);
+					dbg("substitute part of result string '%s'", pos3);
+				}
+			} else {
+				strcat(pos, udev->program_result);
+				dbg("substitute result string '%s'", udev->program_result);
 			}
-			strcat(string, temp);
-		} else
 			break;
+		case 's':
+			if (attr != NULL) {
+				tmpattr = find_sysfs_attribute(class_dev, sysfs_device, attr);
+				if (tmpattr == NULL) {
+					dbg("sysfa attribute '%s' not found", attr);
+					break;
+				}
+				strcpy(pos, tmpattr->value);
+				dbg("substitute sysfs value '%s'", tmpattr->value);
+			} else {
+				dbg("missing attribute");
+			}
+			break;
+		case '%':
+			strcat(pos, "%");
+			break;
+		default:
+			dbg("unknown substitution type '%%%c'", c);
+			break;
+		}
+		strcat(pos, tail);
 	}
 }
 
@@ -422,32 +476,46 @@
 	return retval;
 }
 
-static int compare_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, struct sysfs_pair *pair)
+static struct sysfs_attribute *find_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, char *attr)
 {
 	struct sysfs_attribute *tmpattr = NULL;
 	char *c;
 
-	if ((pair == NULL) || (pair->file[0] == '\0') || (pair->value == '\0'))
-		return -ENODEV;
-
-	dbg("look for device attribute '%s'", pair->file);
+	dbg("look for device attribute '%s'", attr);
 	/* try to find the attribute in the class device directory */
-	tmpattr = sysfs_get_classdev_attr(class_dev, pair->file);
+	tmpattr = sysfs_get_classdev_attr(class_dev, attr);
 	if (tmpattr)
-		goto label_found;
+		goto attr_found;
 
 	/* look in the class device directory if present */
 	if (sysfs_device) {
-		tmpattr = sysfs_get_device_attr(sysfs_device, pair->file);
+		tmpattr = sysfs_get_device_attr(sysfs_device, attr);
 		if (tmpattr)
-			goto label_found;
+			goto attr_found;
 	}
-	return -ENODEV;
 
-label_found:
-	c = tmpattr->value + strlen(tmpattr->value)-1;
-	if (*c == '\n')
-		*c = 0x00;
+	return NULL;
+
+attr_found:
+	c = strchr(tmpattr->value, '\n');
+	if (c != NULL)
+		c[0] = '\0';
+
+	dbg("found attribute '%s'", tmpattr->path);
+	return tmpattr;
+}
+
+static int compare_sysfs_attribute(struct sysfs_class_device *class_dev, struct sysfs_device *sysfs_device, struct sysfs_pair *pair)
+{
+	struct sysfs_attribute *tmpattr;
+
+	if ((pair == NULL) || (pair->file[0] == '\0') || (pair->value == '\0'))
+		return -ENODEV;
+
+	tmpattr = find_sysfs_attribute(class_dev, sysfs_device, pair->file);
+	if (tmpattr == NULL)
+		return -ENODEV;
+
 	dbg("compare attribute '%s' value '%s' with '%s'",
 		  pair->file, tmpattr->value, pair->value);
 	if (strcmp_pattern(pair->value, tmpattr->value) != 0)
@@ -658,7 +726,7 @@
 		/* execute external program */
 		if (dev->program[0] != '\0') {
 			dbg("check " FIELD_PROGRAM);
-			apply_format(udev, dev->program);
+			apply_format(udev, dev->program, class_dev, sysfs_device);
 			if (execute_program(dev->program, udev->program_result, NAME_SIZE) != 0) {
 				dbg(FIELD_PROGRAM " returned nozero");
 				goto try_parent;
@@ -750,8 +818,8 @@
 
 found:
 	/* substitute placeholder */
-	apply_format(udev, udev->name);
-	apply_format(udev, udev->symlink);
+	apply_format(udev, udev->name, class_dev, sysfs_device);
+	apply_format(udev, udev->symlink, class_dev, sysfs_device);
 
 done:
 	perm = find_perm(udev->name);
===== test/udev-test.pl 1.30 vs edited =====
--- 1.30/test/udev-test.pl	Thu Feb 12 01:49:51 2004
+++ edited/test/udev-test.pl	Sat Feb 14 18:45:28 2004
@@ -194,6 +194,16 @@
 EOF
 	},
 	{
+		desc     => "sustitution of sysfs value (%s{file})",
+		subsys   => "block",
+		devpath  => "block/sda",
+		expected => "disk-IBM-ESXS-sda" ,
+		conf     => <<EOF
+BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="disk-%s{vendor}-%k"
+KERNEL="ttyUSB0", NAME="visor"
+EOF
+	},
+	{
 		desc     => "program result substitution",
 		subsys   => "block",
 		devpath  => "block/sda/sda3",
@@ -418,7 +428,7 @@
 	$test_num = $ARGV[0];
 	print "udev-test will run test number $test_num only\n";
 
-	run_test($tests[$test_num], $test_num);
+	run_test($tests[$test_num-1], $test_num-1);
 } else {
 	# test all
 	print "\nudev-test will run ".($#tests + 1)." tests:\n\n";

Re: [PATCH] Adding '%s' format specifier to NAME and SYMLINK

[Kay Sievers]

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

On Sat, Feb 14, 2004 at 07:32:58PM +0100, Kay Sievers wrote:
> On Thu, Feb 12, 2004 at 05:34:57PM -0800, Greg KH wrote:
> > On Tue, Feb 10, 2004 at 09:14:20AM +0100, Hannes Reinecke wrote:
> > > Hi all,
> > > 
> > > this patch makes the format for NAME and SYMLINK a bit more flexible: 
> > > I've added a new format specifier '%s{<SYSFS_var>}', which allows for 
> > > the value of any sysfs entry found for this device to be inserted.
> > > Example (for our S/390 fcp adapter):
> > > 
> > > BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
> > > SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
> > > 
> > > I know this could also be done with an external program, but having this 
> > > incorporated into udev makes life easier, especially if run from 
> > > initramfs. Plus it makes the rules easier to follow, as the result is 
> > > directly visible and need not to be looked up in some external program.
> > > 
> > > Comments etc. welcome.
> > 
> > Oops, sorry I missed this for the 017 release.  I'll look at it tomorrow
> > and get back to you.  At first glance it looks like a good thing.
> > 
> > Oh, you forgot to update the documentation, that's important to do if
> > you want this change to make it in :)
> 
> I took a part of the code and made a version that uses already implemented
> attribute finding logic.
> 
> The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now,
> maybe there are more possible users in the future.
> 
> I've also added the test to udev-test.pl.

Since we have %s{file} it may be nice to allow SYSFS{file}.
This patch allows:

  BUS="usb", SYSFS{idProduct}="a511", NAME="video%n"

compared to the current:

  BUS="usb", SYSFS_idProduct="a511", NAME="video%n"

The curent syntax is still supported.
Looks a bit nicer and less hackish, I think.


thanks,
Kay


[-- Attachment #2: 02-brackets_for_parser.patch --]
[-- Type: text/plain, Size: 1468 bytes --]

diff -Nru a/namedev_parse.c b/namedev_parse.c
--- a/namedev_parse.c	Sun Feb 15 03:18:44 2004
+++ b/namedev_parse.c	Sun Feb 15 03:18:44 2004
@@ -92,6 +92,7 @@
 	char *temp;
 	char *temp2;
 	char *temp3;
+	char *pos;
 	FILE *fd;
 	int program_given = 0;
 	int retval = 0;
@@ -150,7 +151,7 @@
 				continue;
 			}
 
-			if (strncasecmp(temp2, FIELD_SYSFS, sizeof(FIELD_SYSFS)-1) == 0) {
+			if (strncasecmp(temp2, FIELD_SYSFS, sizeof(FIELD_SYSFS)-2) == 0) {
 				struct sysfs_pair *pair = &dev.sysfs_pair[0];
 				int sysfs_pair_num = 0;
 
@@ -164,8 +165,11 @@
 					++pair;
 				}
 				if (pair) {
-					/* remove prepended 'SYSFS_' */
+					/* extract file from 'SYSFS{file}' or 'SYSFS_file' */
 					strfieldcpy(pair->file, temp2 + sizeof(FIELD_SYSFS)-1);
+					pos = strchr(pair->file, '}');
+					if (pos != NULL)
+						pos[0] = '\0';
 					strfieldcpy(pair->value, temp3);
 				}
 				continue;
diff -Nru a/test/udev-test.pl b/test/udev-test.pl
--- a/test/udev-test.pl	Sun Feb 15 03:18:44 2004
+++ b/test/udev-test.pl	Sun Feb 15 03:18:44 2004
@@ -194,6 +194,16 @@
 EOF
 	},
 	{
+		desc     => "select sysfs attribute by SYFS{vendor}",
+		subsys   => "block",
+		devpath  => "block/sda",
+		expected => "disk-IBM-ESXS-sda" ,
+		conf     => <<EOF
+BUS="scsi", SYSFS{vendor}="IBM-ESXS", NAME="disk-%s{vendor}-%k"
+KERNEL="ttyUSB0", NAME="visor"
+EOF
+	},
+	{
 		desc     => "sustitution of sysfs value (%s{file})",
 		subsys   => "block",
 		devpath  => "block/sda",

Re: [PATCH] Adding '%s' format specifier to NAME and SYMLINK

[Kay Sievers]

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

On Sun, Feb 15, 2004 at 03:36:00AM +0100, Kay Sievers wrote:
> On Sat, Feb 14, 2004 at 07:32:58PM +0100, Kay Sievers wrote:
> > On Thu, Feb 12, 2004 at 05:34:57PM -0800, Greg KH wrote:
> > > On Tue, Feb 10, 2004 at 09:14:20AM +0100, Hannes Reinecke wrote:
> > > > Hi all,
> > > > 
> > > > this patch makes the format for NAME and SYMLINK a bit more flexible: 
> > > > I've added a new format specifier '%s{<SYSFS_var>}', which allows for 
> > > > the value of any sysfs entry found for this device to be inserted.
> > > > Example (for our S/390 fcp adapter):
> > > > 
> > > > BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
> > > > SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
> > > > 
> > > > I know this could also be done with an external program, but having this 
> > > > incorporated into udev makes life easier, especially if run from 
> > > > initramfs. Plus it makes the rules easier to follow, as the result is 
> > > > directly visible and need not to be looked up in some external program.
> > > > 
> > > > Comments etc. welcome.
> > > 
> > > Oops, sorry I missed this for the 017 release.  I'll look at it tomorrow
> > > and get back to you.  At first glance it looks like a good thing.
> > > 
> > > Oh, you forgot to update the documentation, that's important to do if
> > > you want this change to make it in :)
> > 
> > I took a part of the code and made a version that uses already implemented
> > attribute finding logic.
> > 
> > The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now,
> > maybe there are more possible users in the future.
> > 
> > I've also added the test to udev-test.pl.
> 
> Since we have %s{file} it may be nice to allow SYSFS{file}.
> This patch allows:
> 
>   BUS="usb", SYSFS{idProduct}="a511", NAME="video%n"
> 
> compared to the current:
> 
>   BUS="usb", SYSFS_idProduct="a511", NAME="video%n"
> 
> The curent syntax is still supported.
> Looks a bit nicer and less hackish, I think.

Better patch with infrastructure to easily implement KEY{attribute}
for every other key. The first user is the SYSFS{file} key.
Both versions, brackets or underscore is supported for the attribute.


thanks,
Kay

[-- Attachment #2: 02-attributes_for_keys.patch --]
[-- Type: text/plain, Size: 2732 bytes --]

diff -Nru a/namedev.h b/namedev.h
--- a/namedev.h	Mon Feb 16 01:42:03 2004
+++ b/namedev.h	Mon Feb 16 01:42:03 2004
@@ -36,7 +36,7 @@
 #define PROGRAM_SIZE	100
 
 #define FIELD_BUS	"BUS"
-#define FIELD_SYSFS	"SYSFS_"
+#define FIELD_SYSFS	"SYSFS"
 #define FIELD_ID	"ID"
 #define FIELD_PLACE	"PLACE"
 #define FIELD_PROGRAM	"PROGRAM"
diff -Nru a/namedev_parse.c b/namedev_parse.c
--- a/namedev_parse.c	Mon Feb 16 01:42:03 2004
+++ b/namedev_parse.c	Mon Feb 16 01:42:03 2004
@@ -85,6 +85,35 @@
 		dump_perm_dev(dev);
 }
 
+/* extract possible KEY{attr} or KEY_attr */
+static char *get_key_attribute(char *str)
+{
+	char *pos;
+	char *attr;
+
+	attr = strchr(str, '_');
+	if (attr != NULL) {
+		attr++;
+		dbg("attribute='%s'", attr);
+		return attr;
+	}
+
+	attr = strchr(str, '{');
+	if (attr != NULL) {
+		attr++;
+		pos = strchr(attr, '}');
+		if (pos == NULL) {
+			dbg("missing closing brace for format");
+			return NULL;
+		}
+		pos[0] = '\0';
+		dbg("attribute='%s'", attr);
+		return attr;
+	}
+
+	return NULL;
+}
+
 int namedev_init_rules(void)
 {
 	char line[255];
@@ -92,6 +121,7 @@
 	char *temp;
 	char *temp2;
 	char *temp3;
+	char *attr;
 	FILE *fd;
 	int program_given = 0;
 	int retval = 0;
@@ -164,8 +194,12 @@
 					++pair;
 				}
 				if (pair) {
-					/* remove prepended 'SYSFS_' */
-					strfieldcpy(pair->file, temp2 + sizeof(FIELD_SYSFS)-1);
+					attr = get_key_attribute(temp2 + sizeof(FIELD_SYSFS)-1);
+					if (attr == NULL) {
+						dbg("error parsing " FIELD_SYSFS " attribute");
+						continue;
+					}
+					strfieldcpy(pair->file, attr);
 					strfieldcpy(pair->value, temp3);
 				}
 				continue;
@@ -205,12 +239,13 @@
 		/* simple plausibility check for given keys */
 		if ((dev.sysfs_pair[0].file[0] == '\0') ^
 		    (dev.sysfs_pair[0].value[0] == '\0')) {
-			dbg("inconsistency in SYSFS_ key");
+			dbg("inconsistency in " FIELD_SYSFS " key");
 			goto error;
 		}
 
 		if ((dev.result[0] != '\0') && (program_given == 0)) {
-			dbg("RESULT is only useful when PROGRAM called in any rule before");
+			dbg(FIELD_RESULT " is only useful when "
+			    FIELD_PROGRAM " is called in any rule before");
 			goto error;
 		}
 
diff -Nru a/test/udev-test.pl b/test/udev-test.pl
--- a/test/udev-test.pl	Mon Feb 16 01:42:03 2004
+++ b/test/udev-test.pl	Mon Feb 16 01:42:03 2004
@@ -194,6 +194,16 @@
 EOF
 	},
 	{
+		desc     => "select sysfs attribute by SYSFS{vendor}",
+		subsys   => "block",
+		devpath  => "block/sda",
+		expected => "disk-IBM-ESXS-sda" ,
+		conf     => <<EOF
+BUS="scsi", SYSFS{vendor}="IBM-ESXS", NAME="disk-%s{vendor}-%k"
+KERNEL="ttyUSB0", NAME="visor"
+EOF
+	},
+	{
 		desc     => "sustitution of sysfs value (%s{file})",
 		subsys   => "block",
 		devpath  => "block/sda",

Re: [PATCH] Adding '%s' format specifier to NAME and SYMLINK

[Greg KH]

On Sat, Feb 14, 2004 at 07:32:58PM +0100, Kay Sievers wrote:
> On Thu, Feb 12, 2004 at 05:34:57PM -0800, Greg KH wrote:
> > On Tue, Feb 10, 2004 at 09:14:20AM +0100, Hannes Reinecke wrote:
> > > Hi all,
> > > 
> > > this patch makes the format for NAME and SYMLINK a bit more flexible: 
> > > I've added a new format specifier '%s{<SYSFS_var>}', which allows for 
> > > the value of any sysfs entry found for this device to be inserted.
> > > Example (for our S/390 fcp adapter):
> > > 
> > > BUS="ccw", SYSFS_devtype="1732/03", NAME="%k" \
> > > SYMLINK="zfcp-%s{hba_id}-%s{wwpn}:%s{fcp_lun}"
> > > 
> > > I know this could also be done with an external program, but having this 
> > > incorporated into udev makes life easier, especially if run from 
> > > initramfs. Plus it makes the rules easier to follow, as the result is 
> > > directly visible and need not to be looked up in some external program.
> > > 
> > > Comments etc. welcome.
> > 
> > Oops, sorry I missed this for the 017 release.  I'll look at it tomorrow
> > and get back to you.  At first glance it looks like a good thing.
> > 
> > Oh, you forgot to update the documentation, that's important to do if
> > you want this change to make it in :)
> 
> I took a part of the code and made a version that uses already implemented
> attribute finding logic.
> 
> The parsing of the format length '%3x' and the '%x{attribute}' is a fuction now,
> maybe there are more possible users in the future.
> 
> I've also added the test to udev-test.pl.

Nice cleanup.  I've applied this.

thanks,

greg k-h


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&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] Adding '%s' format specifier to NAME and SYMLINK

[Greg KH]

On Mon, Feb 16, 2004 at 01:54:52AM +0100, Kay Sievers wrote:
> On Sun, Feb 15, 2004 at 03:36:00AM +0100, Kay Sievers wrote:
> > Since we have %s{file} it may be nice to allow SYSFS{file}.
> > This patch allows:
> > 
> >   BUS="usb", SYSFS{idProduct}="a511", NAME="video%n"
> > 
> > compared to the current:
> > 
> >   BUS="usb", SYSFS_idProduct="a511", NAME="video%n"
> > 
> > The curent syntax is still supported.
> > Looks a bit nicer and less hackish, I think.
> 
> Better patch with infrastructure to easily implement KEY{attribute}
> for every other key. The first user is the SYSFS{file} key.
> Both versions, brackets or underscore is supported for the attribute.

Aplied, thanks.

greg k-h


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id\x1356&alloc_id438&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