From 9cf89cdf84fb20154088145980b676d2b28fc55d Mon Sep 17 00:00:00 2001 From: Denys Vlasenko Date: Sat, 5 Aug 2017 13:45:22 +0200 Subject: [PATCH] sysctl: fix file parsing, do not require -w for VAR=VAL 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 --- libbb/parse_config.c | 10 ++++----- procps/sysctl.c | 51 ++++++++++++++++++++++++++++--------------- testsuite/mdev.tests | 4 ++-- testsuite/parse.tests | 44 +++++++++++++++++++++++++++++++------ 4 files changed, 77 insertions(+), 32 deletions(-) diff --git a/libbb/parse_config.c b/libbb/parse_config.c index 307ae2cd2..da7482c6d 100644 --- a/libbb/parse_config.c +++ b/libbb/parse_config.c @@ -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) { diff --git a/procps/sysctl.c b/procps/sysctl.c index b17f5e896..619f4f1e4 100644 --- a/procps/sysctl.c +++ b/procps/sysctl.c @@ -21,17 +21,17 @@ //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: #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("."); } diff --git a/testsuite/mdev.tests b/testsuite/mdev.tests index 8515aff31..8e53ec564 100755 --- a/testsuite/mdev.tests +++ b/testsuite/mdev.tests @@ -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" \ diff --git a/testsuite/parse.tests b/testsuite/parse.tests index 904e1a17a..2cbed6f31 100755 --- a/testsuite/parse.tests +++ b/testsuite/parse.tests @@ -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 <$FILE.res <