Re: Problem parsing %s in udev rules

[Andrew Patterson <andrew.patterson@hp.com>]

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

On Tue, 2004-10-12 at 16:58 -0700, Greg KH wrote:
> On Tue, Oct 12, 2004 at 05:04:21PM -0600, Andrew Patterson wrote:
> > Kay,
> > 
> > On Thu, 2004-10-07 at 02:30 +0200, Kay Sievers wrote:
> > > On Wed, Oct 06, 2004 at 06:23:05PM -0600, Andrew Patterson wrote:
> > > > On Thu, 2004-10-07 at 01:50 +0200, Kay Sievers wrote:
> > > > > On Wed, Oct 06, 2004 at 05:33:04PM -0600, Andrew Patterson wrote:
> > > > > > Udev seems to be chopping off anything seen in a rule after a %s.  For
> > > > > > example, if I have the following rule:
> > > > > > 
> > > > > > BUS="scsi", KERNEL="sd*", PROGRAM="/sbin/scsi_id", NAME="%k",
> > > > > > SYMLINK="scsi_disks/some-proceeding-stuff-%s{model}-some-following-
> > > > > > stuff"
> > > > > > 
> > > > > > Everything after the %s{model} is ignored.
> > > > > > 
> > > > > > Furthermore, everything in model following whitespace is ignored as
> > > > > > well.  I.e, if model contains "HP 34.7G", the resulting device file
> > > > > > created is /dev/scsi_disks/some-proceeding-stuff-HP.
> > > > > > 
> > > > > > I used the following command to run this test:
> > > > > > 
> > > > > > # DEVPATH=/block/sda ACTION=add ./udev block
> > > > > > 
> > > > > > I also get the same behavior when using udevstart (patched to fix
> > > > > > parameter passing problems).
> > > > > 
> > > > > If your sysfs value contains spaces, which is the delimiter for multiple
> > > > > symlinks, it will not work.
> > > > > 
> > > > > udev can't do this now, you need to wrap the call with an external script,
> > > > > which ensures sane values.
> > > > > 
> > > > 
> > > > That explains the spaces.  What about stuff trailing %s, if %s does not
> > > > contain spaces.  I.e, in the above example, model is ST336753LC and the
> > > > resulting device file is /dev/scsi_disks/some-proceding-stuff-
> > > > ST336753LC.
> > > 
> > > I expect the model value has trailing spaces.
> > > 
> > > You may look with:
> > >   udevinfo -a -p /block/sdX
> > 
> > Yes it does, and it seems for most SCSI devices, vendor and model will
> > have trailing spaces.
> 
> It all depends on the vendor and model :)
> 
> > I have included a patch to udev-036 to deal with
> > this issue.  It trims off trailing whitespace for all sysfs attributes.
> > It might be better to trim off leading whitespace as well.  
> 
> We already trim it off when matching, but we also allow matching if you
> do put the spaces in there.  This patch breaks that, right?

Correct, I have a new patch that trims after the comparison, so it
should work in both cases. 

> 
> > I also wonder if it might be a good idea to do some sort of
> > transformation for characters in an attribute that are illegal in a file
> > system, rather than just chopping everything off. e.g, spaces that are
> > not leading or trailing would be changed to underscores.
> 
> ick, no.

Really?  Care to elaborate? 

Do you not like any transformation (besides the "let's chop off
everything after anything illegal") or just the underscores for spaces
one?  Perhaps a configuration value to make it optional? This sort of
thing might relieve a lot of the script writing that must be done to
handle cases that may be common.

Andrew

diff -Nur udev-036.orig/namedev.c udev-036/namedev.c
--- udev-036.orig/namedev.c	2004-10-12 14:52:18.000000000 -0600
+++ udev-036/namedev.c	2004-10-13 17:04:51.000000000 -0600
@@ -307,6 +307,17 @@
 					dbg("sysfa attribute '%s' not found", attr);
 					break;
 				}
+				/* strip trailing whitespace of matching value */
+				if (isspace(tmpattr->value[strlen(tmpattr->value)-1])) {
+					i = len = strlen(tmpattr->value);
+					while (i > 0 &&  isspace(tmpattr->value[i-1]))
+						i--;
+					if (i < len) {
+						tmpattr->value[i] = '\0';
+						dbg("remove %i trailing whitespace chars from '%s'",
+							 len - i, tmpattr->value);
+					}
+				}
 				strfieldcatmax(string, tmpattr->value, maxsize);
 				dbg("substitute sysfs value '%s'", tmpattr->value);
 			} else {


> thanks,
> 
> greg k-h
> 
-- 
Andrew Patterson                
Hewlett-Packard

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]