linux-hotplug.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [udev] don't rely on field order in namedev_parse
@ 2003-12-12 19:36 Kay Sievers
  0 siblings, 0 replies; 10+ messages in thread
From: Kay Sievers @ 2003-12-12 19:36 UTC (permalink / raw)
  To: linux-hotplug

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

Hi,
here is a patch for the namedev-parser not to rely on the order of the
fields given in the rule. The error handling now is relaxed, if a field is given
twice, the last one wins. If a key is unknown we use it as a sysfs attribute.

It is not as efficient as the current version,
cause we need to loop over the list of known fields for every given field,
but we are free to sort the fields in udev.rules.
What do you think?

thanks,
Kay

[-- Attachment #2: 01-remove-field-ordering.diff --]
[-- Type: text/plain, Size: 8741 bytes --]

diff -Nru a/namedev.h b/namedev.h
--- a/namedev.h	Fri Dec 12 20:21:01 2003
+++ b/namedev.h	Fri Dec 12 20:21:01 2003
@@ -44,11 +44,6 @@
 #define ID_SIZE		50
 #define PLACE_SIZE	50
 
-#define TYPE_LABEL	"LABEL"
-#define TYPE_NUMBER	"NUMBER"
-#define TYPE_TOPOLOGY	"TOPOLOGY"
-#define TYPE_REPLACE	"REPLACE"
-#define TYPE_CALLOUT	"CALLOUT"
 #define CALLOUT_MAXARG	8
 
 struct config_device {
diff -Nru a/namedev_parse.c b/namedev_parse.c
--- a/namedev_parse.c	Fri Dec 12 20:21:01 2003
+++ b/namedev_parse.c	Fri Dec 12 20:21:01 2003
@@ -45,7 +45,7 @@
 		return -ENODEV;
 
 	/* eat any whitespace */
-	while (isspace(*string))
+	while (isspace(*string) || *string == ',')
 		++string;
 
 	/* split based on '=' */
@@ -71,19 +71,6 @@
 	return 0;
 }
 
-static int get_value(const char *left, char **orig_string, char **ret_string)
-{
-	int retval;
-	char *left_string;
-
-	retval = get_pair(orig_string, &left_string, ret_string);
-	if (retval)
-		return retval;
-	if (strcasecmp(left_string, left) != 0)
-		return -ENODEV;
-	return 0;
-}
-
 void dump_config_dev(struct config_device *dev)
 {
 	switch (dev->type) {
@@ -169,13 +156,8 @@
 		if (temp == NULL)
 			goto exit;
 		lineno++;
-
 		dbg_parse("read '%s'", temp);
 
-		/* eat the whitespace at the beginning of the line */
-		while (isspace(*temp))
-			++temp;
-
 		/* empty line? */
 		if (*temp == 0x00)
 			continue;
@@ -184,199 +166,155 @@
 		if (*temp == COMMENT_CHARACTER)
 			continue;
 
+		/* eat the whitespace */
+		while (isspace(*temp))
+			++temp;
+
 		memset(&dev, 0x00, sizeof(struct config_device));
 
-		/* parse the line */
+		/* get the method */
 		temp2 = strsep(&temp, ",");
-		if (strcasecmp(temp2, TYPE_LABEL) == 0) {
-			/* label type */
+
+		if (strcasecmp(temp2, "LABEL") == 0) {
 			dev.type = LABEL;
+			goto keys;
+		}
 
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
+		if (strcasecmp(temp2, "NUMBER") == 0) {
+			dev.type = NUMBER;
+			goto keys;
+		}
+
+		if (strcasecmp(temp2, "TOPOLOGY") == 0) {
+			dev.type = TOPOLOGY;
+			goto keys;
+		}
+
+		if (strcasecmp(temp2, "REPLACE") == 0) {
+			dev.type = REPLACE;
+			goto keys;
+		}
 
-			/* file="value" */
-			temp2 = strsep(&temp, ",");
+		if (strcasecmp(temp2, "CALLOUT") == 0) {
+			dev.type = CALLOUT;
+			goto keys;
+		}
+
+		dbg_parse("unknown type of method '%s'", temp2);
+		goto error;
+keys:
+		/* get all known keys */
+		while (1) {
 			retval = get_pair(&temp, &temp2, &temp3);
 			if (retval)
 				break;
-			strfieldcpy(dev.sysfs_file, temp2);
-			strfieldcpy(dev.sysfs_value, temp3);
 
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
+			if (strcasecmp(temp2, "BUS") == 0) {
+				strfieldcpy(dev.bus, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, "ID") == 0) {
+				strfieldcpy(dev.id, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, "PLACE") == 0) {
+				strfieldcpy(dev.place, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, "KERNEL") == 0) {
+				strfieldcpy(dev.kernel_name, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, "PROGRAM") == 0) {
+				strfieldcpy(dev.exec_program, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, "NAME") == 0) {
+				strfieldcpy(dev.name, temp3);
+				continue;
+			}
 
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
+			if (strcasecmp(temp2, "SYMLINK") == 0) {
 				strfieldcpy(dev.symlink, temp3);
+				continue;
+			}
 
+			/* unknown key taken as sysfs attribute */
+			strfieldcpy(dev.sysfs_file, temp2);
+			strfieldcpy(dev.sysfs_value, temp3);
+		}
+
+		/* check presence of keys according to method type */
+		switch (dev.type) {
+		case LABEL:
 			dbg_parse("LABEL name='%s', bus='%s', "
 				  "sysfs_file='%s', sysfs_value='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.sysfs_file,
 				  dev.sysfs_value, dev.symlink);
-		}
-
-		if (strcasecmp(temp2, TYPE_NUMBER) == 0) {
-			/* number type */
-			dev.type = NUMBER;
-
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* ID="id" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("ID", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.id, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.sysfs_file == '\0') ||
+			    (*dev.sysfs_value == '\0'))
+				goto error;
+			break;
+		case NUMBER:
 			dbg_parse("NUMBER name='%s', bus='%s', id='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.id, dev.symlink);
-		}
-
-		if (strcasecmp(temp2, TYPE_TOPOLOGY) == 0) {
-			/* number type */
-			dev.type = TOPOLOGY;
-
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* PLACE="place" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("PLACE", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.place, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.id == '\0'))
+				goto error;
+			break;
+		case TOPOLOGY:
 			dbg_parse("TOPOLOGY name='%s', bus='%s', "
 				  "place='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.place, dev.symlink);
-		}
-
-		if (strcasecmp(temp2, TYPE_REPLACE) == 0) {
-			/* number type */
-			dev.type = REPLACE;
-
-			/* KERNEL="kernel_name" */
-			retval = get_value("KERNEL", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.kernel_name, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.place == '\0'))
+				goto error;
+			break;
+		case REPLACE:
 			dbg_parse("REPLACE name='%s', kernel_name='%s', symlink='%s'",
 				  dev.name, dev.kernel_name, dev.symlink);
-		}
-
-		if (strcasecmp(temp2, TYPE_CALLOUT) == 0) {
-			/* number type */
-			dev.type = CALLOUT;
-
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* PROGRAM="executable" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("PROGRAM", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.exec_program, temp3);
-
-			/* ID="id" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("ID", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.id, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
+			if ((*dev.name == '\0') ||
+			    (*dev.kernel_name == '\0'))
+				goto error;
+			break;
+		case CALLOUT:
 			dbg_parse("CALLOUT name='%s', bus='%s', program='%s', "
 				  "id='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.exec_program,
 				  dev.id, dev.symlink);
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.id == '\0') ||
+			    (*dev.exec_program == '\0'))
+				goto error;
+			break;
+		default:
+			dbg_parse("xxx default method");
+			goto error;
 		}
 
 		retval = add_config_dev(&dev);
 		if (retval) {
 			dbg("add_config_dev returned with error %d", retval);
-			goto exit;
+			continue;
 		}
 	}
-	dbg_parse("%s:%d:%Zd: error parsing '%s'", udev_rules_filename,
+error:
+	dbg_parse("%s:%d:%Zd: field missing or parse error", udev_rules_filename,
 		  lineno, temp - line, temp);
 exit:
 	fclose(fd);
 	return retval;
-}	
-
+}
 
 int namedev_init_permissions(void)
 {

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
@ 2003-12-15 22:36 Greg KH
  2003-12-16  0:21 ` Kay Sievers
                   ` (7 more replies)
  0 siblings, 8 replies; 10+ messages in thread
From: Greg KH @ 2003-12-15 22:36 UTC (permalink / raw)
  To: linux-hotplug

On Fri, Dec 12, 2003 at 08:36:03PM +0100, Kay Sievers wrote:
> Hi,
> here is a patch for the namedev-parser not to rely on the order of the
> fields given in the rule. The error handling now is relaxed, if a field is given
> twice, the last one wins. If a key is unknown we use it as a sysfs attribute.
> 
> It is not as efficient as the current version,
> cause we need to loop over the list of known fields for every given field,
> but we are free to sort the fields in udev.rules.
> What do you think?

I like the idea, it really cleans up the code a lot.  But it looks like
it breaks the fact that we might want to test for the sysfs file "name"
in a rule, right? 

> diff -Nru a/namedev.h b/namedev.h
> --- a/namedev.h	Fri Dec 12 20:21:01 2003
> +++ b/namedev.h	Fri Dec 12 20:21:01 2003
> @@ -44,11 +44,6 @@
>  #define ID_SIZE		50
>  #define PLACE_SIZE	50
>  
> -#define TYPE_LABEL	"LABEL"
> -#define TYPE_NUMBER	"NUMBER"
> -#define TYPE_TOPOLOGY	"TOPOLOGY"
> -#define TYPE_REPLACE	"REPLACE"
> -#define TYPE_CALLOUT	"CALLOUT"

Why remove these?

thanks,

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
@ 2003-12-16  0:21 ` Kay Sievers
  2003-12-16  0:27 ` Greg KH
                   ` (6 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Kay Sievers @ 2003-12-16  0:21 UTC (permalink / raw)
  To: linux-hotplug

On Mon, Dec 15, 2003 at 02:36:04PM -0800, Greg KH wrote:
> On Fri, Dec 12, 2003 at 08:36:03PM +0100, Kay Sievers wrote:
> > Hi,
> > here is a patch for the namedev-parser not to rely on the order of the
> > fields given in the rule. The error handling now is relaxed, if a field is given
> > twice, the last one wins. If a key is unknown we use it as a sysfs attribute.
> > 
> > It is not as efficient as the current version,
> > cause we need to loop over the list of known fields for every given field,
> > but we are free to sort the fields in udev.rules.
> > What do you think?
> 
> I like the idea, it really cleans up the code a lot.  But it looks like
> it breaks the fact that we might want to test for the sysfs file "name"
> in a rule, right?

Yes, that's not nice. Two options come to my mind:

o prepend SYSFS_ to the match, like:
  LABEL, BUS="usb", SYSFS_model="Creative Labs WebCam*", NAME="video%n"
  This is my favorite. It's close to the current and says clearly what it does.

o switch to case sensitive field name match
  and hope nobody puts uppercase 'ID', 'BUS', 'NAME' in the kernel
  It's more like a text adventure :)

What do you think?


> > -#define TYPE_LABEL	"LABEL"
> > -#define TYPE_NUMBER	"NUMBER"
> > -#define TYPE_TOPOLOGY	"TOPOLOGY"
> > -#define TYPE_REPLACE	"REPLACE"
> > -#define TYPE_CALLOUT	"CALLOUT"
> 
> Why remove these?

Oh I just wrote the code without it in mind and later simply removed it (being lazy).
The field name strings are also embedded in the code,
we may use #define's for the names of the fields too, then I'm happy :)

thanks,
Kay


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
  2003-12-16  0:21 ` Kay Sievers
@ 2003-12-16  0:27 ` Greg KH
  2003-12-16  1:36 ` Kay Sievers
                   ` (5 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2003-12-16  0:27 UTC (permalink / raw)
  To: linux-hotplug

On Tue, Dec 16, 2003 at 01:21:58AM +0100, Kay Sievers wrote:
> Yes, that's not nice. Two options come to my mind:
> 
> o prepend SYSFS_ to the match, like:
>   LABEL, BUS="usb", SYSFS_model="Creative Labs WebCam*", NAME="video%n"
>   This is my favorite. It's close to the current and says clearly what it does.

Yeah, I like this one too.

> > > -#define TYPE_LABEL	"LABEL"
> > > -#define TYPE_NUMBER	"NUMBER"
> > > -#define TYPE_TOPOLOGY	"TOPOLOGY"
> > > -#define TYPE_REPLACE	"REPLACE"
> > > -#define TYPE_CALLOUT	"CALLOUT"
> > 
> > Why remove these?
> 
> Oh I just wrote the code without it in mind and later simply removed it (being lazy).
> The field name strings are also embedded in the code,
> we may use #define's for the names of the fields too, then I'm happy :)

Ok, that sounds good.

thanks,

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
  2003-12-16  0:21 ` Kay Sievers
  2003-12-16  0:27 ` Greg KH
@ 2003-12-16  1:36 ` Kay Sievers
  2003-12-16 23:40 ` Greg KH
                   ` (4 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Kay Sievers @ 2003-12-16  1:36 UTC (permalink / raw)
  To: linux-hotplug

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

On Mon, Dec 15, 2003 at 04:27:59PM -0800, Greg KH wrote:
> On Tue, Dec 16, 2003 at 01:21:58AM +0100, Kay Sievers wrote:
> > o prepend SYSFS_ to the match, like:
> >   LABEL, BUS="usb", SYSFS_model="Creative Labs WebCam*", NAME="video%n"
> >   This is my favorite. It's close to the current and says clearly what it does.
> 
> Yeah, I like this one too.

Here is a try.
Please have a look at it.

My motto of the day: Try to attach the patch before hitting the 'y'-key :)

Kay


01-remove-field-ordering.diff
  o change the parsing to get a key from the rule and sort it
    into our list of known keys instead of expecting a special order
  o the key to match a sysfs file must be prependend by 'SYSFS_' now
    to match with the new parsing.
    (The config must be changed, but it's a bit more descriptive too.)
  o put names of fields in define's, like the name of the methods
  o update all tests and the man page


[-- Attachment #2: 01-remove-field-ordering.diff --]
[-- Type: text/plain, Size: 14381 bytes --]

diff -Nru a/namedev.h b/namedev.h
--- a/namedev.h	Tue Dec 16 02:26:49 2003
+++ b/namedev.h	Tue Dec 16 02:26:49 2003
@@ -49,6 +49,16 @@
 #define TYPE_TOPOLOGY	"TOPOLOGY"
 #define TYPE_REPLACE	"REPLACE"
 #define TYPE_CALLOUT	"CALLOUT"
+
+#define FIELD_BUS	"BUS"
+#define FIELD_ID	"ID"
+#define FIELD_SYSFS	"SYSFS_"
+#define FIELD_PLACE	"PLACE"
+#define FIELD_PROGRAM	"PROGRAM"
+#define FIELD_KERNEL	"KERNEL"
+#define FIELD_NAME	"NAME"
+#define FIELD_SYMLINK	"SYMLINK"
+
 #define CALLOUT_MAXARG	8
 
 struct config_device {
diff -Nru a/namedev_parse.c b/namedev_parse.c
--- a/namedev_parse.c	Tue Dec 16 02:26:49 2003
+++ b/namedev_parse.c	Tue Dec 16 02:26:49 2003
@@ -45,7 +45,7 @@
 		return -ENODEV;
 
 	/* eat any whitespace */
-	while (isspace(*string))
+	while (isspace(*string) || *string == ',')
 		++string;
 
 	/* split based on '=' */
@@ -71,19 +71,6 @@
 	return 0;
 }
 
-static int get_value(const char *left, char **orig_string, char **ret_string)
-{
-	int retval;
-	char *left_string;
-
-	retval = get_pair(orig_string, &left_string, ret_string);
-	if (retval)
-		return retval;
-	if (strcasecmp(left_string, left) != 0)
-		return -ENODEV;
-	return 0;
-}
-
 void dump_config_dev(struct config_device *dev)
 {
 	switch (dev->type) {
@@ -169,13 +156,8 @@
 		if (temp == NULL)
 			goto exit;
 		lineno++;
-
 		dbg_parse("read '%s'", temp);
 
-		/* eat the whitespace at the beginning of the line */
-		while (isspace(*temp))
-			++temp;
-
 		/* empty line? */
 		if (*temp == 0x00)
 			continue;
@@ -184,199 +166,160 @@
 		if (*temp == COMMENT_CHARACTER)
 			continue;
 
+		/* eat the whitespace */
+		while (isspace(*temp))
+			++temp;
+
 		memset(&dev, 0x00, sizeof(struct config_device));
 
-		/* parse the line */
+		/* get the method */
 		temp2 = strsep(&temp, ",");
+
 		if (strcasecmp(temp2, TYPE_LABEL) == 0) {
-			/* label type */
 			dev.type = LABEL;
-
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* file="value" */
-			temp2 = strsep(&temp, ",");
-			retval = get_pair(&temp, &temp2, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.sysfs_file, temp2);
-			strfieldcpy(dev.sysfs_value, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
-			dbg_parse("LABEL name='%s', bus='%s', "
-				  "sysfs_file='%s', sysfs_value='%s', symlink='%s'",
-				  dev.name, dev.bus, dev.sysfs_file,
-				  dev.sysfs_value, dev.symlink);
+			goto keys;
 		}
 
 		if (strcasecmp(temp2, TYPE_NUMBER) == 0) {
-			/* number type */
 			dev.type = NUMBER;
-
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* ID="id" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("ID", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.id, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
-			dbg_parse("NUMBER name='%s', bus='%s', id='%s', symlink='%s'",
-				  dev.name, dev.bus, dev.id, dev.symlink);
+			goto keys;
 		}
 
 		if (strcasecmp(temp2, TYPE_TOPOLOGY) == 0) {
-			/* number type */
 			dev.type = TOPOLOGY;
-
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* PLACE="place" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("PLACE", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.place, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
-			dbg_parse("TOPOLOGY name='%s', bus='%s', "
-				  "place='%s', symlink='%s'",
-				  dev.name, dev.bus, dev.place, dev.symlink);
+			goto keys;
 		}
 
 		if (strcasecmp(temp2, TYPE_REPLACE) == 0) {
-			/* number type */
 			dev.type = REPLACE;
-
-			/* KERNEL="kernel_name" */
-			retval = get_value("KERNEL", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.kernel_name, temp3);
-
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
-
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
-				strfieldcpy(dev.symlink, temp3);
-
-			dbg_parse("REPLACE name='%s', kernel_name='%s', symlink='%s'",
-				  dev.name, dev.kernel_name, dev.symlink);
+			goto keys;
 		}
 
 		if (strcasecmp(temp2, TYPE_CALLOUT) == 0) {
-			/* number type */
 			dev.type = CALLOUT;
+			goto keys;
+		}
 
-			/* BUS="bus" */
-			retval = get_value("BUS", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.bus, temp3);
-
-			/* PROGRAM="executable" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("PROGRAM", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.exec_program, temp3);
-
-			/* ID="id" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("ID", &temp, &temp3);
+		dbg_parse("unknown type of method '%s'", temp2);
+		goto error;
+keys:
+		/* get all known keys */
+		while (1) {
+			retval = get_pair(&temp, &temp2, &temp3);
 			if (retval)
 				break;
-			strfieldcpy(dev.id, temp3);
 
-			/* NAME="new_name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("NAME", &temp, &temp3);
-			if (retval)
-				break;
-			strfieldcpy(dev.name, temp3);
+			if (strcasecmp(temp2, FIELD_BUS) == 0) {
+				strfieldcpy(dev.bus, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, FIELD_ID) == 0) {
+				strfieldcpy(dev.id, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, FIELD_PLACE) == 0) {
+				strfieldcpy(dev.place, temp3);
+				continue;
+			}
+
+			if (strncasecmp(temp2, FIELD_SYSFS, sizeof(FIELD_SYSFS)-1) == 0) {
+				/* remove prepended 'SYSFS_' */
+				strfieldcpy(dev.sysfs_file, temp2 + sizeof(FIELD_SYSFS)-1);
+				strfieldcpy(dev.sysfs_value, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, FIELD_KERNEL) == 0) {
+				strfieldcpy(dev.kernel_name, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, FIELD_PROGRAM) == 0) {
+				strfieldcpy(dev.exec_program, temp3);
+				continue;
+			}
+
+			if (strcasecmp(temp2, FIELD_NAME) == 0) {
+				strfieldcpy(dev.name, temp3);
+				continue;
+			}
 
-			/* SYMLINK="name" */
-			temp2 = strsep(&temp, ",");
-			retval = get_value("SYMLINK", &temp, &temp3);
-			if (retval == 0)
+			if (strcasecmp(temp2, FIELD_SYMLINK) == 0) {
 				strfieldcpy(dev.symlink, temp3);
+				continue;
+			}
+
+			dbg_parse("unknown type of field '%s'", temp2);
+		}
 
+		/* check presence of keys according to method type */
+		switch (dev.type) {
+		case LABEL:
+			dbg_parse("LABEL name='%s', bus='%s', "
+				  "sysfs_file='%s', sysfs_value='%s', symlink='%s'",
+				  dev.name, dev.bus, dev.sysfs_file,
+				  dev.sysfs_value, dev.symlink);
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.sysfs_file == '\0') ||
+			    (*dev.sysfs_value == '\0'))
+				goto error;
+			break;
+		case NUMBER:
+			dbg_parse("NUMBER name='%s', bus='%s', id='%s', symlink='%s'",
+				  dev.name, dev.bus, dev.id, dev.symlink);
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.id == '\0'))
+				goto error;
+			break;
+		case TOPOLOGY:
+			dbg_parse("TOPOLOGY name='%s', bus='%s', "
+				  "place='%s', symlink='%s'",
+				  dev.name, dev.bus, dev.place, dev.symlink);
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.place == '\0'))
+				goto error;
+			break;
+		case REPLACE:
+			dbg_parse("REPLACE name='%s', kernel_name='%s', symlink='%s'",
+				  dev.name, dev.kernel_name, dev.symlink);
+			if ((*dev.name == '\0') ||
+			    (*dev.kernel_name == '\0'))
+				goto error;
+			break;
+		case CALLOUT:
 			dbg_parse("CALLOUT name='%s', bus='%s', program='%s', "
 				  "id='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.exec_program,
 				  dev.id, dev.symlink);
+			if ((*dev.name == '\0') ||
+			    (*dev.bus == '\0') ||
+			    (*dev.id == '\0') ||
+			    (*dev.exec_program == '\0'))
+				goto error;
+			break;
+		default:
+			dbg_parse("xxx default method");
+			goto error;
 		}
 
 		retval = add_config_dev(&dev);
 		if (retval) {
 			dbg("add_config_dev returned with error %d", retval);
-			goto exit;
+			continue;
 		}
 	}
-	dbg_parse("%s:%d:%Zd: error parsing '%s'", udev_rules_filename,
-		  lineno, temp - line, temp);
+error:
+	dbg_parse("%s:%d:%Zd: field missing or parse error", udev_rules_filename,
+		  lineno, temp - line);
 exit:
 	fclose(fd);
 	return retval;
-}	
-
+}
 
 int namedev_init_permissions(void)
 {
diff -Nru a/test/label_test b/test/label_test
--- a/test/label_test	Tue Dec 16 02:26:49 2003
+++ b/test/label_test	Tue Dec 16 02:26:49 2003
@@ -8,7 +8,7 @@
 export UDEV_CONFIG_FILE=$PWD/$CONFIG
 
 cat > $RULES << EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="boot_disk%n"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="boot_disk%n"
 EOF
 
 cat > $CONFIG << EOF
diff -Nru a/test/udev-test.pl b/test/udev-test.pl
--- a/test/udev-test.pl	Tue Dec 16 02:26:49 2003
+++ b/test/udev-test.pl	Tue Dec 16 02:26:49 2003
@@ -38,7 +38,7 @@
 		devpath  => "block/sda",
 		expected => "boot_disk" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="boot_disk%n"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="boot_disk%n"
 REPLACE, KERNEL="ttyUSB0", NAME="visor"
 EOF
 	},
@@ -48,7 +48,7 @@
 		devpath  => "block/sda/sda1",
 		expected => "boot_disk1" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="boot_disk%n"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="boot_disk%n"
 EOF
 	},
 	{
@@ -57,10 +57,10 @@
 		devpath  => "block/sda/sda1",
 		expected => "boot_disk1" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="?IBM-ESXS", NAME="boot_disk%n-1"
-LABEL, BUS="scsi", vendor="IBM-ESXS?", NAME="boot_disk%n-2"
-LABEL, BUS="scsi", vendor="IBM-ES??", NAME="boot_disk%n"
-LABEL, BUS="scsi", vendor="IBM-ESXSS", NAME="boot_disk%n-3"
+LABEL, BUS="scsi", SYSFS_vendor="?IBM-ESXS", NAME="boot_disk%n-1"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS?", NAME="boot_disk%n-2"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ES??", NAME="boot_disk%n"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXSS", NAME="boot_disk%n-3"
 EOF
 	},
 	{
@@ -167,7 +167,7 @@
 		devpath  => "block/sda",
 		expected => "lun0/disc" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="lun0/%D"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="lun0/%D"
 EOF
 	},
 	{
@@ -176,7 +176,7 @@
 		devpath  => "block/sda/sda2",
 		expected => "lun0/part2" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="lun0/%D"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="lun0/%D"
 EOF
 	},
 	{
@@ -205,7 +205,7 @@
 		devpath  => "block/sda/sda2",
 		expected => "1/2/a/b/symlink" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="1/2/node", SYMLINK="1/2/a/b/symlink"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="1/2/node", SYMLINK="1/2/a/b/symlink"
 EOF
 	},
 	{
@@ -214,7 +214,7 @@
 		devpath  => "block/sda/sda2",
 		expected => "1/2/symlink" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="1/2/a/b/node", SYMLINK="1/2/symlink"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="1/2/a/b/node", SYMLINK="1/2/symlink"
 EOF
 	},
 	{
@@ -223,7 +223,7 @@
 		devpath  => "block/sda/sda2",
 		expected => "1/2/c/d/symlink" ,
 		conf     => <<EOF
-LABEL, BUS="scsi", vendor="IBM-ESXS", NAME="1/2/a/b/node", SYMLINK="1/2/c/d/symlink"
+LABEL, BUS="scsi", SYSFS_vendor="IBM-ESXS", NAME="1/2/a/b/node", SYMLINK="1/2/c/d/symlink"
 EOF
 	},
 	{
diff -Nru a/udev.8 b/udev.8
--- a/udev.8	Tue Dec 16 02:26:49 2003
+++ b/udev.8	Tue Dec 16 02:26:49 2003
@@ -18,7 +18,7 @@
 .B udev
 reads the sysfs directory of the given device to collect device attributes
 like label, serial number or bus device number.
-These attributes are treated as a key 
+These attributes are treated as a key
 to determine a unique name for device file creation.
 .B udev
 maintains a database for devices present on the system.
@@ -87,7 +87,7 @@
 .I /etc/udev/udev.rules
 or specified by the
 .I udev_rules
-value in the 
+value in the
 .I /etc/udev/udev.conf
 file.
 .P
@@ -114,8 +114,7 @@
 device label or serial number, like USB serial number, SCSI UUID or
 file system label
 .br
-.RB "keys: " BUS ", "
-.I sysfs_attribute
+.RB "keys: " BUS ", " SYSFS_
 .TP
 .B NUMBER
 device number on the bus, like PCI bus id
@@ -130,7 +129,7 @@
 .B REPLACE
 string replacement of the kernel device name
 .br
-.RB "key: " KERNEL_NAME
+.RB "key: " KERNEL
 .P
 The methods are applied in the following order:
 .BR CALLOUT ", " LABEL ", " NUMBER ", " TOPOLOGY ", " REPLACE "."
@@ -167,7 +166,7 @@
 CALLOUT, BUS="scsi", PROGRAM="/sbin/scsi_id", ID="OEM 0815", NAME="disk1"
 
 # USB printer to be called lp_color
-LABEL, BUS="usb", serial="W09090207101241330", NAME="lp_color"
+LABEL, BUS="usb", SYSFS_serial="W09090207101241330", NAME="lp_color"
 
 # sound card with PCI bus id 00:0b.0 to be called dsp
 NUMBER, BUS="pci", ID="00:0b.0", NAME="dsp"
diff -Nru a/udev.rules.demo b/udev.rules.demo
--- a/udev.rules.demo	Tue Dec 16 02:26:49 2003
+++ b/udev.rules.demo	Tue Dec 16 02:26:49 2003
@@ -1,8 +1,8 @@
 # USB camera from Fuji to be named "camera"
-LABEL, BUS="usb", vendor="FUJIFILM", NAME="camera%n"
+LABEL, BUS="usb", SYSFS_vendor="FUJIFILM", NAME="camera%n"
 
 # USB device plugged into the fourth port of the second hub to be called gps_device
-TOPOLOGY, BUS="usb", place="2.4", NAME="gps_device"
+TOPOLOGY, BUS="usb", PLACE="2.4", NAME="gps_device"
 
 # ttyUSB1 should always be called visor
 REPLACE, KERNEL="ttyUSB1", NAME="visor"

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
                   ` (2 preceding siblings ...)
  2003-12-16  1:36 ` Kay Sievers
@ 2003-12-16 23:40 ` Greg KH
  2003-12-17  0:50 ` Kay Sievers
                   ` (3 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2003-12-16 23:40 UTC (permalink / raw)
  To: linux-hotplug

On Tue, Dec 16, 2003 at 02:36:25AM +0100, Kay Sievers wrote:
> On Mon, Dec 15, 2003 at 04:27:59PM -0800, Greg KH wrote:
> > On Tue, Dec 16, 2003 at 01:21:58AM +0100, Kay Sievers wrote:
> > > o prepend SYSFS_ to the match, like:
> > >   LABEL, BUS="usb", SYSFS_model="Creative Labs WebCam*", NAME="video%n"
> > >   This is my favorite. It's close to the current and says clearly what it does.
> > 
> > Yeah, I like this one too.
> 
> Here is a try.
> Please have a look at it.

Looks great, it reduced the ammount of code by a bit, making things
easier to read.  Nice job.

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
                   ` (3 preceding siblings ...)
  2003-12-16 23:40 ` Greg KH
@ 2003-12-17  0:50 ` Kay Sievers
  2003-12-17  0:59 ` Greg KH
                   ` (2 subsequent siblings)
  7 siblings, 0 replies; 10+ messages in thread
From: Kay Sievers @ 2003-12-17  0:50 UTC (permalink / raw)
  To: linux-hotplug

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

On Tue, Dec 16, 2003 at 03:40:09PM -0800, Greg KH wrote:
> On Tue, Dec 16, 2003 at 02:36:25AM +0100, Kay Sievers wrote:
> > On Mon, Dec 15, 2003 at 04:27:59PM -0800, Greg KH wrote:
> > > On Tue, Dec 16, 2003 at 01:21:58AM +0100, Kay Sievers wrote:
> > > > o prepend SYSFS_ to the match, like:
> > > >   LABEL, BUS="usb", SYSFS_model="Creative Labs WebCam*", NAME="video%n"
> > > >   This is my favorite. It's close to the current and says clearly what it does.
> > > 
> > > Yeah, I like this one too.
> > 
> > Here is a try.
> > Please have a look at it.
> 
> Looks great, it reduced the ammount of code by a bit, making things
> easier to read.  Nice job.

Fine, thanks.
Here is a small trivial cleanup of the recent changes.

Kay



02-trivial-cleanup-parser-changes.diff
  o use defines in debug strings
  o replace my 'xxx' debug :)
  o shorten line in man page example to not to exceed 80 chars when printed


[-- Attachment #2: 02-trivial-cleanup-parser-changes.diff --]
[-- Type: text/plain, Size: 2338 bytes --]

diff -Nru a/namedev_parse.c b/namedev_parse.c
--- a/namedev_parse.c	Wed Dec 17 01:38:27 2003
+++ b/namedev_parse.c	Wed Dec 17 01:38:27 2003
@@ -257,7 +257,7 @@
 		/* check presence of keys according to method type */
 		switch (dev.type) {
 		case LABEL:
-			dbg_parse("LABEL name='%s', bus='%s', "
+			dbg_parse(TYPE_LABEL " name='%s', bus='%s', "
 				  "sysfs_file='%s', sysfs_value='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.sysfs_file,
 				  dev.sysfs_value, dev.symlink);
@@ -268,7 +268,7 @@
 				goto error;
 			break;
 		case NUMBER:
-			dbg_parse("NUMBER name='%s', bus='%s', id='%s', symlink='%s'",
+			dbg_parse(TYPE_NUMBER "name='%s', bus='%s', id='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.id, dev.symlink);
 			if ((*dev.name == '\0') ||
 			    (*dev.bus == '\0') ||
@@ -276,7 +276,7 @@
 				goto error;
 			break;
 		case TOPOLOGY:
-			dbg_parse("TOPOLOGY name='%s', bus='%s', "
+			dbg_parse(TYPE_TOPOLOGY "name='%s', bus='%s', "
 				  "place='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.place, dev.symlink);
 			if ((*dev.name == '\0') ||
@@ -285,14 +285,14 @@
 				goto error;
 			break;
 		case REPLACE:
-			dbg_parse("REPLACE name='%s', kernel_name='%s', symlink='%s'",
+			dbg_parse(TYPE_REPLACE "name='%s', kernel_name='%s', symlink='%s'",
 				  dev.name, dev.kernel_name, dev.symlink);
 			if ((*dev.name == '\0') ||
 			    (*dev.kernel_name == '\0'))
 				goto error;
 			break;
 		case CALLOUT:
-			dbg_parse("CALLOUT name='%s', bus='%s', program='%s', "
+			dbg_parse(TYPE_CALLOUT "name='%s', bus='%s', program='%s', "
 				  "id='%s', symlink='%s'",
 				  dev.name, dev.bus, dev.exec_program,
 				  dev.id, dev.symlink);
@@ -303,7 +303,7 @@
 				goto error;
 			break;
 		default:
-			dbg_parse("xxx default method");
+			dbg_parse("unknown type of method");
 			goto error;
 		}
 
diff -Nru a/udev.8 b/udev.8
--- a/udev.8	Wed Dec 17 01:38:27 2003
+++ b/udev.8	Wed Dec 17 01:38:27 2003
@@ -178,7 +178,7 @@
 REPLACE, KERNEL="ttyUSB1", NAME="pda", SYMLINK="palmtop handheld"
 
 # multiple USB webcams with symlinks to be called webcam0, webcam1, ...
-LABEL, BUS="usb", SYSFS_model="WebCam V3", NAME="video%n", SYMLINK="webcam%n"
+LABEL, BUS="usb", SYSFS_model="XV3", NAME="video%n", SYMLINK="webcam%n"
 .fi
 .P
 Permissions and ownership for the created device files may specified at

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
                   ` (4 preceding siblings ...)
  2003-12-17  0:50 ` Kay Sievers
@ 2003-12-17  0:59 ` Greg KH
  2003-12-17 17:04 ` Roman Kagan
  2003-12-17 18:26 ` Greg KH
  7 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2003-12-17  0:59 UTC (permalink / raw)
  To: linux-hotplug

On Wed, Dec 17, 2003 at 01:50:20AM +0100, Kay Sievers wrote:
> 
> Fine, thanks.
> Here is a small trivial cleanup of the recent changes.

Applied, thanks.  I released 009 just a few minutes too early :(

thanks,

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
                   ` (5 preceding siblings ...)
  2003-12-17  0:59 ` Greg KH
@ 2003-12-17 17:04 ` Roman Kagan
  2003-12-17 18:26 ` Greg KH
  7 siblings, 0 replies; 10+ messages in thread
From: Roman Kagan @ 2003-12-17 17:04 UTC (permalink / raw)
  To: linux-hotplug

  Hi,

On Tue, Dec 16, 2003 at 02:36:25AM +0100, Kay Sievers wrote:
> 01-remove-field-ordering.diff
> [...]
> diff -Nru a/namedev_parse.c b/namedev_parse.c
> --- a/namedev_parse.c	Tue Dec 16 02:26:49 2003
> +++ b/namedev_parse.c	Tue Dec 16 02:26:49 2003
> @@ -169,13 +156,8 @@
>  		if (temp = NULL)
>  			goto exit;
>  		lineno++;
> -
>  		dbg_parse("read '%s'", temp);
>  
> -		/* eat the whitespace at the beginning of the line */
> -		while (isspace(*temp))
> -			++temp;
> -
>  		/* empty line? */
>  		if (*temp = 0x00)
>  			continue;
> @@ -184,199 +166,160 @@
>  		if (*temp = COMMENT_CHARACTER)
>  			continue;
>  
> +		/* eat the whitespace */
> +		while (isspace(*temp))
> +			++temp;
> +
>  		memset(&dev, 0x00, sizeof(struct config_device));
>  

This chunk broke parsing of blank lines and comments with blanks before
'#'.  Please revert it with the patch below.

  Roman.

--- udev-009/namedev_parse.c~	2003-12-16 05:26:49.000000000 +0300
+++ udev-009/namedev_parse.c	2003-12-17 20:00:21.000000000 +0300
@@ -158,6 +158,10 @@
 		lineno++;
 		dbg_parse("read '%s'", temp);
 
+		/* eat the whitespace */
+		while (isspace(*temp))
+			++temp;
+
 		/* empty line? */
 		if (*temp = 0x00)
 			continue;
@@ -166,10 +170,6 @@
 		if (*temp = COMMENT_CHARACTER)
 			continue;
 
-		/* eat the whitespace */
-		while (isspace(*temp))
-			++temp;
-
 		memset(&dev, 0x00, sizeof(struct config_device));
 
 		/* get the method */


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [udev] don't rely on field order in namedev_parse
  2003-12-15 22:36 Greg KH
                   ` (6 preceding siblings ...)
  2003-12-17 17:04 ` Roman Kagan
@ 2003-12-17 18:26 ` Greg KH
  7 siblings, 0 replies; 10+ messages in thread
From: Greg KH @ 2003-12-17 18:26 UTC (permalink / raw)
  To: linux-hotplug

On Wed, Dec 17, 2003 at 08:04:29PM +0300, Roman Kagan wrote:
> 
> This chunk broke parsing of blank lines and comments with blanks before
> '#'.  Please revert it with the patch below.

Thanks I've applied this.  I've also added some tests to the
udev-test.pl script to catch this kind of error in the future.

greg k-h


-------------------------------------------------------
This SF.net email is sponsored by: IBM Linux Tutorials.
Become an expert in LINUX or just sharpen your skills.  Sign up for IBM's
Free Linux Tutorials.  Learn everything from the bash shell to sys admin.
Click now! http://ads.osdn.com/?ad_id\x1278&alloc_id371&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

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2003-12-17 18:26 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2003-12-12 19:36 [udev] don't rely on field order in namedev_parse Kay Sievers
  -- strict thread matches above, loose matches on Subject: below --
2003-12-15 22:36 Greg KH
2003-12-16  0:21 ` Kay Sievers
2003-12-16  0:27 ` Greg KH
2003-12-16  1:36 ` Kay Sievers
2003-12-16 23:40 ` Greg KH
2003-12-17  0:50 ` Kay Sievers
2003-12-17  0:59 ` Greg KH
2003-12-17 17:04 ` Roman Kagan
2003-12-17 18:26 ` Greg KH

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).