From mboxrd@z Thu Jan 1 00:00:00 1970 From: Kay Sievers Date: Mon, 16 Feb 2004 00:54:52 +0000 Subject: Re: [PATCH] Adding '%s' format specifier to NAME and SYMLINK Message-Id: <20040216005452.GA3301@vrfy.org> MIME-Version: 1 Content-Type: multipart/mixed; boundary="FL5UXtIhxfXey3p5" List-Id: References: <402892DC.4000003@suse.de> In-Reply-To: <402892DC.4000003@suse.de> To: linux-hotplug@vger.kernel.org --FL5UXtIhxfXey3p5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline 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{}', 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 --FL5UXtIhxfXey3p5 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline; filename="02-attributes_for_keys.patch" 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 => < "sustitution of sysfs value (%s{file})", subsys => "block", devpath => "block/sda", --FL5UXtIhxfXey3p5-- ------------------------------------------------------- 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=1356&alloc_id=3438&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