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

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

[-- 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",