mdev: fix a bug where it was not stopping on first matching rule
authorDenis Vlasenko <vda.linux@googlemail.com>
Sat, 29 Mar 2008 13:10:57 +0000 (13:10 -0000)
committerDenis Vlasenko <vda.linux@googlemail.com>
Sat, 29 Mar 2008 13:10:57 +0000 (13:10 -0000)
(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

testsuite/mdev.tests
util-linux/mdev.c

index 4c44adcb8ee56af7fdc80dec471567b5be75ef8d..1ee762828a77a48d355c9a9ef28cbfd6726c9505 100755 (executable)
@@ -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
 " \
        "" ""
 
index f86ce14ceba61cb141949d7a730377150e85bd74..a2866054c2e7dff1da69524198b770ea378355cc 100644 (file)
@@ -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: