sysctl: fix file parsing, do not require -w for VAR=VAL
authorDenys Vlasenko <vda.linux@googlemail.com>
Sat, 5 Aug 2017 11:45:22 +0000 (13:45 +0200)
committerDenys Vlasenko <vda.linux@googlemail.com>
Sat, 5 Aug 2017 11:45:22 +0000 (13:45 +0200)
function                                             old     new   delta
sysctl_act_on_setting                                  -     451    +451
sysctl_main                                          222     282     +60
packed_usage                                       31744   31793     +49
config_read                                          604     639     +35
sysctl_act_recursive                                 612     163    -449
------------------------------------------------------------------------------
(add/remove: 1/0 grow/shrink: 3/1 up/down: 595/-449)          Total: 146 bytes

Signed-off-by: Denys Vlasenko <vda.linux@googlemail.com>
libbb/parse_config.c
procps/sysctl.c
testsuite/mdev.tests
testsuite/parse.tests

index 307ae2cd2b8ffc6f09c33b3dbd516eb7f6cd2496..da7482c6d3a385c402be69404f3720272e4d7a6e 100644 (file)
@@ -201,10 +201,10 @@ int FAST_FUNC config_read(parser_t *parser, char **tokens, unsigned flags, const
                /* Combine remaining arguments? */
                if ((t != (ntokens-1)) || !(flags & PARSE_GREEDY)) {
                        /* Vanilla token, find next delimiter */
-                       line += strcspn(line, delims[0] ? delims : delims + 1);
+                       line += strcspn(line, (delims[0] && (flags & PARSE_EOL_COMMENTS)) ? delims : delims + 1);
                } else {
                        /* Combining, find comment char if any */
-                       line = strchrnul(line, PARSE_EOL_COMMENTS ? delims[0] : '\0');
+                       line = strchrnul(line, (flags & PARSE_EOL_COMMENTS) ? delims[0] : '\0');
 
                        /* Trim any extra delimiters from the end */
                        if (flags & PARSE_TRIM) {
@@ -214,10 +214,10 @@ int FAST_FUNC config_read(parser_t *parser, char **tokens, unsigned flags, const
                }
 
                /* Token not terminated? */
-               if (*line == delims[0])
-                       *line = '\0';
+               if ((flags & PARSE_EOL_COMMENTS) && *line == delims[0])
+                       *line = '\0'; /* ends with comment char: this line is done */
                else if (*line != '\0')
-                       *line++ = '\0';
+                       *line++ = '\0'; /* token is done, continue parsing line */
 
 #if 0 /* unused so far */
                if (flags & PARSE_ESCAPE) {
index b17f5e896c843c28cf21ae35e399eacb20cf9f97..619f4f1e48c54faa48be9f66de26c44d137b28d0 100644 (file)
 //kbuild:lib-$(CONFIG_BB_SYSCTL) += sysctl.o
 
 //usage:#define sysctl_trivial_usage
-//usage:       "[OPTIONS] [KEY[=VALUE]]..."
+//usage:       "-p [-enq] [FILE...] / [-enqaw] [KEY[=VALUE]]..."
 //usage:#define sysctl_full_usage "\n\n"
 //usage:       "Show/set kernel parameters\n"
+//usage:     "\n       -p      Set values from FILEs (default /etc/sysctl.conf)"
 //usage:     "\n       -e      Don't warn about unknown keys"
 //usage:     "\n       -n      Don't show key names"
+//usage:     "\n       -q      Quiet"
 //usage:     "\n       -a      Show all values"
 /* Same as -a, no need to show it */
 /* //usage:     "\n    -A      Show all values in table form" */
 //usage:     "\n       -w      Set values"
-//usage:     "\n       -p FILE Set values from FILE (default /etc/sysctl.conf)"
-//usage:     "\n       -q      Set values silently"
 //usage:
 //usage:#define sysctl_example_usage
 //usage:       "sysctl [-n] [-e] variable...\n"
@@ -48,7 +48,7 @@ enum {
        FLAG_TABLE_FORMAT    = 1 << 2, /* not implemented */
        FLAG_SHOW_ALL        = 1 << 3,
        FLAG_PRELOAD_FILE    = 1 << 4,
-/* TODO: procps 3.2.8 seems to not require -w for KEY=VAL to work: */
+       /* NB: procps 3.2.8 does not require -w for KEY=VAL to work, it only rejects non-KEY=VAL form */
        FLAG_WRITE           = 1 << 5,
        FLAG_QUIET           = 1 << 6,
 };
@@ -104,6 +104,7 @@ static int sysctl_act_on_setting(char *setting)
        int fd, retval = EXIT_SUCCESS;
        char *cptr, *outname;
        char *value = value; /* for compiler */
+       bool writing = (option_mask32 & FLAG_WRITE);
 
        outname = xstrdup(setting);
 
@@ -114,8 +115,10 @@ static int sysctl_act_on_setting(char *setting)
                cptr++;
        }
 
-       if (option_mask32 & FLAG_WRITE) {
-               cptr = strchr(setting, '=');
+       cptr = strchr(setting, '=');
+       if (cptr)
+               writing = 1;
+       if (writing) {
                if (cptr == NULL) {
                        bb_error_msg("error: '%s' must be of the form name=value",
                                outname);
@@ -147,7 +150,7 @@ static int sysctl_act_on_setting(char *setting)
                        break;
                default:
                        bb_perror_msg("error %sing key '%s'",
-                                       option_mask32 & FLAG_WRITE ?
+                                       writing ?
                                                "sett" : "read",
                                        outname);
                        break;
@@ -156,7 +159,7 @@ static int sysctl_act_on_setting(char *setting)
                goto end;
        }
 
-       if (option_mask32 & FLAG_WRITE) {
+       if (writing) {
 //TODO: procps 3.2.7 writes "value\n", note trailing "\n"
                xwrite_str(fd, value);
                close(fd);
@@ -238,22 +241,27 @@ static int sysctl_handle_preload_file(const char *filename)
 {
        char *token[2];
        parser_t *parser;
+       int parse_flags;
 
        parser = config_open(filename);
        /* Must do it _after_ config_open(): */
        xchdir("/proc/sys");
-       /* xchroot("/proc/sys") - if you are paranoid */
 
 //TODO: ';' is comment char too
-//TODO: comment may be only at line start. "var=1 #abc" - "1 #abc" is the value
-// (but _whitespace_ from ends should be trimmed first (and we do it right))
-//TODO: "var==1" is mishandled (must use "=1" as a value, but uses "1")
-// can it be fixed by removing PARSE_COLLAPSE bit?
-       while (config_read(parser, token, 2, 2, "# \t=", PARSE_NORMAL)) {
+//TODO: <space><tab><space>#comment is also comment, not strictly 1st char only
+       parse_flags = 0;
+       parse_flags &= ~PARSE_COLLAPSE;   // NO (var==val is not var=val) - treat consecutive delimiters as one
+       parse_flags &= ~PARSE_TRIM;       // NO - trim leading and trailing delimiters
+       parse_flags |= PARSE_GREEDY;      // YES - last token takes entire remainder of the line
+       parse_flags &= ~PARSE_MIN_DIE;    // NO - die if < min tokens found
+       parse_flags &= ~PARSE_EOL_COMMENTS; // NO (only first char) - comments are recognized even if not first char
+       while (config_read(parser, token, 2, 2, "#=", parse_flags)) {
                char *tp;
+               trim(token[0]);
+               trim(token[1]);
                sysctl_dots_to_slashes(token[0]);
                tp = xasprintf("%s=%s", token[0], token[1]);
-               sysctl_act_recursive(tp);
+               sysctl_act_on_setting(tp);
                free(tp);
        }
        if (ENABLE_FEATURE_CLEAN_UP)
@@ -273,12 +281,19 @@ int sysctl_main(int argc UNUSED_PARAM, char **argv)
        option_mask32 = opt;
 
        if (opt & FLAG_PRELOAD_FILE) {
+               int cur_dir_fd;
                option_mask32 |= FLAG_WRITE;
-               /* xchdir("/proc/sys") is inside */
-               return sysctl_handle_preload_file(*argv ? *argv : "/etc/sysctl.conf");
+               if (!*argv)
+                       *--argv = (char*)"/etc/sysctl.conf";
+               cur_dir_fd = xopen(".", O_RDONLY | O_DIRECTORY);
+               do {
+                       /* xchdir("/proc/sys") is inside */
+                       sysctl_handle_preload_file(*argv);
+                       xfchdir(cur_dir_fd); /* files can be relative, must restore cwd */
+               } while (*++argv);
+               return 0; /* procps-ng 3.3.10 does not flag parse errors */
        }
        xchdir("/proc/sys");
-       /* xchroot("/proc/sys") - if you are paranoid */
        if (opt & (FLAG_TABLE_FORMAT | FLAG_SHOW_ALL)) {
                return sysctl_act_recursive(".");
        }
index 8515aff319d5e390bf3431db5acdee7089d8cfad..8e53ec5647b53c6de8c695f77705f7644702e0b1 100755 (executable)
@@ -168,7 +168,7 @@ SKIP=
 # continuing to use directory structure from prev test
 rm -rf mdev.testdir/dev/*
 echo "sda 0:0 644 @echo @echo TEST" >mdev.testdir/etc/mdev.conf
-optional STATIC FEATURE_MDEV_CONF FEATURE_MDEV_EXEC FEATURE_LS_RECURSIVE FEATURE_LS_TIMESTAMPS FEATURE_LS_USERNAME FEATURE_SH_IS_ASH ASH_ECHO
+optional STATIC FEATURE_MDEV_CONF FEATURE_MDEV_EXEC FEATURE_LS_RECURSIVE FEATURE_LS_TIMESTAMPS FEATURE_LS_USERNAME SH_IS_ASH ASH_ECHO
 testing "mdev command" \
        "env - PATH=$PATH ACTION=add DEVPATH=/block/sda chroot mdev.testdir /mdev 2>&1;
        ls -lnR mdev.testdir/dev | $FILTER_LS" \
@@ -183,7 +183,7 @@ SKIP=
 # continuing to use directory structure from prev test
 rm -rf mdev.testdir/dev/*
 echo "sda 0:0 644 =block/ @echo @echo TEST:\$MDEV" >mdev.testdir/etc/mdev.conf
-optional STATIC FEATURE_MDEV_CONF FEATURE_MDEV_RENAME FEATURE_MDEV_EXEC FEATURE_LS_RECURSIVE FEATURE_LS_TIMESTAMPS FEATURE_LS_USERNAME FEATURE_SH_IS_ASH
+optional STATIC FEATURE_MDEV_CONF FEATURE_MDEV_RENAME FEATURE_MDEV_EXEC FEATURE_LS_RECURSIVE FEATURE_LS_TIMESTAMPS FEATURE_LS_USERNAME SH_IS_ASH
 testing "mdev move and command" \
        "env - PATH=$PATH ACTION=add DEVPATH=/block/sda chroot mdev.testdir /mdev 2>&1;
        ls -lnR mdev.testdir/dev | $FILTER_LS2" \
index 904e1a17a2b50babeefd3f1befb53dbf7d66cfda..2cbed6f3117081e191e7a061fc7f652f0a7ff5b9 100755 (executable)
@@ -5,13 +5,13 @@
 
 . ./testing.sh
 
-COLLAPSE=$(( 0x00010000))
-TRIM=$((     0x00020000))
-GREEDY=$((   0x00040000))
-MIN_DIE=$((  0x00100000))
-KEEP_COPY=$((0x00200000))
-ESCAPE=$((   0x00400000))
-NORMAL=$((   COLLAPSE | TRIM | GREEDY))
+COLLAPSE=$((    0x00010000))
+TRIM=$((        0x00020000))
+GREEDY=$((      0x00040000))
+MIN_DIE=$((     0x00100000))
+KEEP_COPY=$((   0x00200000))
+EOL_COMMENTS=$((0x00400000))
+NORMAL=$((   COLLAPSE | TRIM | GREEDY | EOL_COMMENTS))
 
 # testing "description" "command" "result" "infile" "stdin"
 
@@ -27,6 +27,34 @@ testing "parse notrim" \
        "-" \
        " sda 0:0 644 @echo @echo TEST \n"
 
+testing "parse comments" \
+       "parse -n 4 -m 3 -f $((NORMAL - EOL_COMMENTS)) -" \
+       "[sda][0:0][644][@echo @echo TEST #this is not eaten]\n" \
+       "-" \
+       "\
+# sda 0:0 644 @echo @echo TEST - this gets eaten
+ sda 0:0 644 @echo @echo TEST #this is not eaten
+"
+
+testing "parse bad comment" \
+       "parse -n 2 -m 2 -d '#=' -f $((GREEDY)) - 2>&1" \
+       "\
+[var][val]
+parse: bad line 3: 1 tokens found, 2 needed
+[  #this][ok]
+[  #this][=ok]
+[  #this][=ok=ok=ok=]
+" \
+       "-" \
+       "\
+# this gets eaten
+var=val
+  #this causes error msg
+  #this=ok
+  #this==ok
+  #this==ok=ok=ok=
+"
+
 FILE=__parse
 cat >$FILE <<EOF
 #
@@ -96,6 +124,8 @@ cat >$FILE.res <<EOF
 [option][dns][129.219.13.81]
 [option][domain][local]
 [option][lease][864000]
+[option][msstaticroutes][10.0.0.0/8][10.127.0.1]
+[option][staticroutes][10.0.0.0/8][10.127.0.1,][10.11.12.0/24][10.11.12.1]
 [option][0x08][01020304]
 EOF