[udev] don't rely on field order in namedev_parse

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

[-- 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)
 {