mdev: fix a bug where it was not stopping on first matching rule

(testsuite entry added). Revamped line parsing while at it.

function                                             old     new   delta
next_field                                             -      36     +36
make_device                                         1104    1022     -82
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 0/1 up/down: 36/-82)            Total: -46 bytes

2 files changed, 115 insertions, 112 deletions

diff --git a/testsuite/mdev.tests b/testsuite/mdev.tests
index 4c44adcb8..1ee762828 100755
--- a/testsuite/mdev.tests
+++ b/testsuite/mdev.tests
@@ -6,8 +6,8 @@
 
 # ls -ln is showing date. Need to remove that, it's variable
 # sed: (1) "maj, min" -> "maj,min" (2) coalesce spaces
-# cut: remove user, group, and date
-FILTER_LS="sed -e 's/,  */,/g' -e 's/  */ /g' | cut -d' ' -f 1,2,5,9-"
+# cut: remove date
+FILTER_LS="sed -e 's/,  */,/g' -e 's/  */ /g' | cut -d' ' -f 1-5,9-"
 
 # testing "test name" "options" "expected result" "file input" "stdin"
 
@@ -16,6 +16,7 @@ mkdir mdev.testdir
 # We need mdev executable to be in chroot jail!
 # (will still fail with dynamically linked one, though...)
 cp ../busybox mdev.testdir/mdev
+mkdir mdev.testdir/etc
 mkdir mdev.testdir/dev
 mkdir -p mdev.testdir/sys/block/sda
 echo "8:0" >mdev.testdir/sys/block/sda/dev
@@ -25,7 +26,19 @@ testing "mdev add /block/sda" \
 	ls -ln mdev.testdir/dev | $FILTER_LS" \
 "\
 mdev: /etc/mdev.conf: No such file or directory
-brw-rw---- 1 8,0 sda
+brw-rw---- 1 0 0 8,0 sda
+" \
+	"" ""
+
+# continuing to use directory structure from prev test
+rm mdev.testdir/dev/sda
+echo ".* 1:1 666" >mdev.testdir/etc/mdev.conf
+echo "sda 2:2 444" >>mdev.testdir/etc/mdev.conf
+testing "mdev stops on first rule" \
+	"env - ACTION=add DEVPATH=/block/sda chroot mdev.testdir /mdev 2>&1;
+	ls -ln mdev.testdir/dev | $FILTER_LS" \
+"\
+brw-rw-rw- 1 1 1 8,0 sda
 " \
 	"" ""
 
diff --git a/util-linux/mdev.c b/util-linux/mdev.c
index f86ce14ce..a2866054c 100644
--- a/util-linux/mdev.c
+++ b/util-linux/mdev.c
@@ -24,6 +24,16 @@ struct globals {
 /* We use additional 64+ bytes in make_device() */
 #define SCRATCH_SIZE 80
 
+static char *next_field(char *s)
+{
+	char *end = skip_non_whitespace(s);
+	s = skip_whitespace(end);
+	*end = '\0';
+	if (*s == '\0')
+		s = NULL;
+	return s;
+}
+
 /* mknod in /dev based on a path like "/sys/block/hda/hda1" */
 /* NB: "mdev -s" may call us many times, do not leak memory/fds! */
 static void make_device(char *path, int delete)
@@ -63,135 +73,115 @@ static void make_device(char *path, int delete)
 
 	if (ENABLE_FEATURE_MDEV_CONF) {
 		FILE *fp;
-		char *line, *vline;
+		char *line, *val, *next;
 		unsigned lineno = 0;
 
-		/* If we have a config file, look up the user settings */
+		/* If we have config file, look up user settings */
 		fp = fopen_or_warn("/etc/mdev.conf", "r");
 		if (!fp)
 			goto end_parse;
 
-		while ((vline = line = xmalloc_fgetline(fp)) != NULL) {
-			int field;
-			char *orig_line;
-
-			if (ENABLE_FEATURE_MDEV_EXEC)
-				orig_line = xstrdup(line); /* pristine copy for command execution. */
-
+		while ((line = xmalloc_fgetline(fp)) != NULL) {
 			++lineno;
-
-// TODO: get rid of loop,
-// just linear skip_whitespace() style code will do!
-// (will also get rid of orig_line).
+			trim(line);
+			if (!line[0])
+				goto next_line;
 
 			/* Fields: regex uid:gid mode [alias] [cmd] */
-			for (field = 0; field < (3 + ENABLE_FEATURE_MDEV_RENAME + ENABLE_FEATURE_MDEV_EXEC); ++field) {
-
-				/* Find a non-empty field */
-				char *val;
-				do {
-					val = strtok(vline, " \t");
-					vline = NULL;
-				} while (val && !*val);
-				if (!val) {
-					if (field)
-						break;
-					else
-						goto next_line;
-				}
-
-				if (field == 0) {
-
-					/* Regex to match this device */
-					regex_t match;
-					regmatch_t off;
-					int result;
-
-					/* Is this it? */
-					xregcomp(&match, val, REG_EXTENDED);
-					result = regexec(&match, device_name, 1, &off, 0);
-					regfree(&match);
-
-					/* If not this device, skip rest of line */
-					if (result || off.rm_so || off.rm_eo != strlen(device_name))
-						goto next_line;
-
-				} else if (field == 1) {
-
-					/* uid:gid device ownership */
-					struct passwd *pass;
-					struct group *grp;
 
-					char *str_uid = val;
-					char *str_gid = strchrnul(val, ':');
-					if (*str_gid)
-						*str_gid++ = '\0';
-
-					/* Parse UID */
-					pass = getpwnam(str_uid);
-					if (pass)
-						uid = pass->pw_uid;
-					else
-						uid = strtoul(str_uid, NULL, 10);
-
-					/* Parse GID */
-					grp = getgrnam(str_gid);
-					if (grp)
-						gid = grp->gr_gid;
-					else
-						gid = strtoul(str_gid, NULL, 10);
-
-				} else if (field == 2) {
-
-					/* Mode device permissions */
-					mode = strtoul(val, NULL, 8);
+			/* 1st field: regex to match this device */
+			next = next_field(line);
+			{
+				regex_t match;
+				regmatch_t off;
+				int result;
+
+				/* Is this it? */
+				xregcomp(&match, line, REG_EXTENDED);
+				result = regexec(&match, device_name, 1, &off, 0);
+				regfree(&match);
+
+				/* If not this device, skip rest of line */
+				if (result || off.rm_so || off.rm_eo != strlen(device_name))
+					goto next_line;
+			}
 
-				} else if (ENABLE_FEATURE_MDEV_RENAME && field == 3) {
+			/* This line matches: stop parsing the file
+			 * after parsing the rest of fields */
 
-					if (*val != '>')
-						++field;
-					else {
-						free(alias); /* don't leak in case we matched it on prev line */
-						alias = xstrdup(val + 1);
-					}
+			/* 2nd field: uid:gid - device ownership */
+			if (!next) /* field must exist */
+				bb_error_msg_and_die("bad line %u", lineno);
+			val = next;
+			next = next_field(val);
+			{
+				struct passwd *pass;
+				struct group *grp;
+				char *str_uid = val;
+				char *str_gid = strchrnul(val, ':');
+
+				if (*str_gid)
+					*str_gid++ = '\0';
+				/* Parse UID */
+				pass = getpwnam(str_uid);
+				if (pass)
+					uid = pass->pw_uid;
+				else
+					uid = strtoul(str_uid, NULL, 10);
+				/* Parse GID */
+				grp = getgrnam(str_gid);
+				if (grp)
+					gid = grp->gr_gid;
+				else
+					gid = strtoul(str_gid, NULL, 10);
+			}
 
+			/* 3rd field: mode - device permissions */
+			if (!next) /* field must exist */
+				bb_error_msg_and_die("bad line %u", lineno);
+			val = next;
+			next = next_field(val);
+			mode = strtoul(val, NULL, 8);
+
+			/* 4th field (opt): >alias */
+			if (ENABLE_FEATURE_MDEV_RENAME) {
+				if (!next)
+					break;
+				val = next;
+				next = next_field(val);
+				if (*val == '>') {
+					alias = xstrdup(val + 1);
 				}
+			}
 
-				if (ENABLE_FEATURE_MDEV_EXEC && field == 3 + ENABLE_FEATURE_MDEV_RENAME) {
-
-					/* Optional command to run */
-					const char *s = "@$*";
-					const char *s2 = strchr(s, *val);
-
-					if (!s2) {
-						/* Force error */
-						field = 1;
-						break;
-					}
-
-					/* Correlate the position in the "@$*" with the delete
-					 * step so that we get the proper behavior:
-					 * @cmd: run on create
-					 * $cmd: run on delete
-					 * *cmd: run on both
-					 */
-					if ((s2 - s + 1) /*1/2/3*/ & /*1/2*/ (1 + delete)) {
-						free(command); /* don't leak in case we matched it on prev line */
-						command = xstrdup(orig_line + (val + 1 - line));
-					}
+			/* The rest (opt): command to run */
+			if (!next)
+				break;
+			val = next;
+			if (ENABLE_FEATURE_MDEV_EXEC) {
+				const char *s = "@$*";
+				const char *s2 = strchr(s, *val);
+
+				if (!s2)
+					bb_error_msg_and_die("bad line %u", lineno);
+
+				/* Correlate the position in the "@$*" with the delete
+				 * step so that we get the proper behavior:
+				 * @cmd: run on create
+				 * $cmd: run on delete
+				 * *cmd: run on both
+				 */
+				if ((s2 - s + 1) /*1/2/3*/ & /*1/2*/ (1 + delete)) {
+					command = xstrdup(val + 1);
 				}
-			} /* end of "for every field" */
-
-			/* Did everything parse happily? */
-			if (field <= 2)
-				bb_error_msg_and_die("bad line %u", lineno);
-
+			}
+			/* end of field parsing */
+			break; /* we found matching line, stop */
  next_line:
 			free(line);
-			if (ENABLE_FEATURE_MDEV_EXEC)
-				free(orig_line);
 		} /* end of "while line is read from /etc/mdev.conf" */
 
+		free(line); /* in case we used "break" to get here */
 		fclose(fp);
 	}
  end_parse: