From owner-freebsd-current@FreeBSD.ORG Sat Dec 1 16:51:14 2012 Return-Path: Delivered-To: current@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [69.147.83.52]) by hub.freebsd.org (Postfix) with ESMTP id 7C20A6DF for ; Sat, 1 Dec 2012 16:51:14 +0000 (UTC) (envelope-from hrs@FreeBSD.org) Received: from mail.allbsd.org (gatekeeper.allbsd.org [IPv6:2001:2f0:104:e001::32]) by mx1.freebsd.org (Postfix) with ESMTP id 1FE328FC13 for ; Sat, 1 Dec 2012 16:51:12 +0000 (UTC) Received: from alph.allbsd.org (p1137-ipbf1505funabasi.chiba.ocn.ne.jp [118.7.212.137]) (authenticated bits=128) by mail.allbsd.org (8.14.5/8.14.5) with ESMTP id qB1GoseO048751 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO) for ; Sun, 2 Dec 2012 01:51:04 +0900 (JST) (envelope-from hrs@FreeBSD.org) Received: from localhost (localhost [127.0.0.1]) (authenticated bits=0) by alph.allbsd.org (8.14.5/8.14.5) with ESMTP id qB1GoqP4046382 for ; Sun, 2 Dec 2012 01:50:54 +0900 (JST) (envelope-from hrs@FreeBSD.org) Date: Sun, 02 Dec 2012 01:50:48 +0900 (JST) Message-Id: <20121202.015048.1122480556487090170.hrs@allbsd.org> To: current@FreeBSD.org Subject: RFC: sysctl -f filename From: Hiroki Sato X-PGPkey-fingerprint: BDB3 443F A5DD B3D0 A530 FFD7 4F2C D3D8 2793 CF2D X-Mailer: Mew version 6.5 on Emacs 23.4 / Mule 6.0 (HANACHIRUSATO) Mime-Version: 1.0 Content-Type: Multipart/Signed; protocol="application/pgp-signature"; micalg=pgp-sha1; boundary="--Security_Multipart0(Sun_Dec__2_01_50_48_2012_189)--" Content-Transfer-Encoding: 7bit X-Virus-Scanned: clamav-milter 0.97.4 at gatekeeper.allbsd.org X-Virus-Status: Clean X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.2.7 (mail.allbsd.org [133.31.130.32]); Sun, 02 Dec 2012 01:51:04 +0900 (JST) X-Spam-Status: No, score=-97.9 required=13.0 tests=CONTENT_TYPE_PRESENT, ONLY1HOPDIRECT,QENCPTR1,SAMEHELOBY2HOP,USER_IN_WHITELIST autolearn=no version=3.3.2 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on gatekeeper.allbsd.org X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sat, 01 Dec 2012 16:51:14 -0000 ----Security_Multipart0(Sun_Dec__2_01_50_48_2012_189)-- Content-Type: Multipart/Mixed; boundary="--Next_Part(Sun_Dec__2_01_50_48_2012_330)--" Content-Transfer-Encoding: 7bit ----Next_Part(Sun_Dec__2_01_50_48_2012_330)-- Content-Type: Text/Plain; charset=us-ascii Content-Transfer-Encoding: 7bit Hi, I would like comments about the attached patch for sysctl(8) to add a new option "-f filename". It supports reading of a file with key=value lines. As you probably know, we already have /etc/sysctl.conf and it is processed by rc.d/sysctl shell script in a line-by-line basis. The problem I want to fix is a confusing syntax of /etc/sysctl.conf. The file supports a typical configuration file syntax but problematic in some cases. For example: kern.coredump=1 works well in /etc/sysctl.conf, but kern.coredump="1" does not work. Similarly, it is difficult to use whitespaces and "#" in the value: OK: kern.domainname=domain\ name\ with\ spaces NG: kern.domainname="domain name with spaces" NG: kern.domainname=domain\ name\ including\ #\ character NG: kern.domainname=domain\ name\ including\ \#\ character The attached patch solves them, and in addition it displays an error message with a line number if there is something wrong in the file like this: % cat -n /etc/sysctl.conf ... 10 kern.coredump=1 11 kern.coredump2=1 ... % /etc/rc.d/sysctl start sysctl: kern.coredump at line 10: Operation not permitted sysctl: unknown oid 'kern.coredump2' at line 11 # /etc/rc.d/sysctl start kern.coredump: 1 -> 1 sysctl: unknown oid 'kern.coredump2' at line 11 Any comments are welcome. -- Hiroki ----Next_Part(Sun_Dec__2_01_50_48_2012_330)-- Content-Type: Text/X-Patch; charset=us-ascii Content-Transfer-Encoding: 7bit Content-Disposition: inline; filename="sysctl_20121201-1.diff" Index: sbin/sysctl/sysctl.8 =================================================================== --- sbin/sysctl/sysctl.8 (revision 243756) +++ sbin/sysctl/sysctl.8 (working copy) @@ -28,7 +28,7 @@ .\" From: @(#)sysctl.8 8.1 (Berkeley) 6/6/93 .\" $FreeBSD$ .\" -.Dd January 17, 2011 +.Dd November 22, 2012 .Dt SYSCTL 8 .Os .Sh NAME @@ -37,6 +37,7 @@ .Sh SYNOPSIS .Nm .Op Fl bdehiNnoqx +.Op Fl f Ar filename .Ar name Ns Op = Ns Ar value .Ar ... .Nm @@ -80,6 +81,11 @@ or .Fl n is specified, or a variable is being set. +.It Fl f Ar filename +Specify a file which contains a pair of name and value in each line. +.Nm +reads and processes the specified file first and then processes the name +and value pairs in the command line argument. .It Fl h Format output for human, rather than machine, readability. .It Fl i Index: sbin/sysctl/sysctl.c =================================================================== --- sbin/sysctl/sysctl.c (revision 243756) +++ sbin/sysctl/sysctl.c (working copy) @@ -56,13 +56,16 @@ #include #include #include +#include #include +static const char *conffile; static int aflag, bflag, dflag, eflag, hflag, iflag; -static int Nflag, nflag, oflag, qflag, xflag, warncount; +static int Nflag, nflag, oflag, qflag, xflag; static int oidfmt(int *, int, char *, u_int *); -static void parse(char *); +static int parsefile(const char *); +static int parse(char *, int); static int show_var(int *, int); static int sysctl_all(int *oid, int len); static int name2oid(char *, int *); @@ -74,7 +77,7 @@ { (void)fprintf(stderr, "%s\n%s\n", - "usage: sysctl [-bdehiNnoqx] name[=value] ...", + "usage: sysctl [-bdehiNnoqx] [-f filename] name[=value] ...", " sysctl [-bdehNnoqx] -a"); exit(1); } @@ -83,12 +86,13 @@ main(int argc, char **argv) { int ch; + int warncount = 0; setlocale(LC_NUMERIC, ""); setbuf(stdout,0); setbuf(stderr,0); - while ((ch = getopt(argc, argv, "AabdehiNnoqwxX")) != -1) { + while ((ch = getopt(argc, argv, "Aabdef:hiNnoqwxX")) != -1) { switch (ch) { case 'A': /* compatibility */ @@ -106,6 +110,9 @@ case 'e': eflag = 1; break; + case 'f': + conffile = strdup(optarg); + break; case 'h': hflag = 1; break; @@ -146,13 +153,15 @@ usage(); if (aflag && argc == 0) exit(sysctl_all(0, 0)); - if (argc == 0) + if (argc == 0 && conffile == NULL) usage(); - warncount = 0; + if (conffile != NULL) + warncount += parsefile(conffile); while (argc-- > 0) - parse(*argv++); - exit(warncount); + warncount += parse(*argv++, 0); + + return (warncount); } /* @@ -160,8 +169,8 @@ * Lookup and print out the MIB entry if it exists. * Set a new value if requested. */ -static void -parse(char *string) +static int +parse(char *string, int lineno) { int len, i, j; void *newval = 0; @@ -173,17 +182,30 @@ int64_t i64val; uint64_t u64val; int mib[CTL_MAXNAME]; - char *cp, *bufp, buf[BUFSIZ], *endptr, fmt[BUFSIZ]; + char *cp, *bufp, buf[BUFSIZ], *endptr, fmt[BUFSIZ], line[BUFSIZ]; u_int kind; + memset(line, 0, sizeof(line)); + if (lineno) + snprintf(line, sizeof(line), " at line %d", lineno); bufp = buf; - if (snprintf(buf, BUFSIZ, "%s", string) >= BUFSIZ) - errx(1, "oid too long: '%s'", string); + if (snprintf(buf, BUFSIZ, "%s", string) >= BUFSIZ) { + warn("oid too long: '%s'%s", string, line); + return (1); + } if ((cp = strchr(string, '=')) != NULL) { *strchr(buf, '=') = '\0'; *cp++ = '\0'; while (isspace(*cp)) cp++; + /* Strip a pair of " or ' if any. */ + switch (*cp) { + case '\"': + case '\'': + if (cp[strlen(cp) - 1] == *cp) + cp[strlen(cp) - 1] = '\0'; + cp++; + } newval = cp; newsize = strlen(cp); } @@ -191,15 +213,20 @@ if (len < 0) { if (iflag) - return; - if (qflag) + return (1); + if (qflag) exit(1); else - errx(1, "unknown oid '%s'", bufp); + errx(1, "unknown oid '%s'%s", bufp, line); } - if (oidfmt(mib, len, fmt, &kind)) - err(1, "couldn't find format of oid '%s'", bufp); + if (oidfmt(mib, len, fmt, &kind)) { + warn("couldn't find format of oid '%s'%s", bufp, line); + if (!iflag) + exit(1); + else + return (1); + } if (newval == NULL || dflag) { if ((kind & CTLTYPE) == CTLTYPE_NODE) { @@ -215,16 +242,20 @@ putchar('\n'); } } else { - if ((kind & CTLTYPE) == CTLTYPE_NODE) - errx(1, "oid '%s' isn't a leaf node", bufp); + if ((kind & CTLTYPE) == CTLTYPE_NODE) { + warn("oid '%s' isn't a leaf node%s", bufp, line); + return (1); + } if (!(kind & CTLFLAG_WR)) { if (kind & CTLFLAG_TUN) { - warnx("oid '%s' is a read only tunable", bufp); - errx(1, "Tunable values are set in /boot/loader.conf"); - } else { - errx(1, "oid '%s' is read only", bufp); - } + warnx("oid '%s' is a read only tunable%s", + bufp, line); + warn("Tunable values are set in /boot/loader.conf"); + } else + warn("oid '%s' is read only%s", bufp, line); + + return (1); } if ((kind & CTLTYPE) == CTLTYPE_INT || @@ -233,22 +264,28 @@ (kind & CTLTYPE) == CTLTYPE_ULONG || (kind & CTLTYPE) == CTLTYPE_S64 || (kind & CTLTYPE) == CTLTYPE_U64) { - if (strlen(newval) == 0) - errx(1, "empty numeric value"); + if (strlen(newval) == 0) { + warn("empty numeric value%s", line); + return (1); + } } switch (kind & CTLTYPE) { case CTLTYPE_INT: if (strcmp(fmt, "IK") == 0) { - if (!set_IK(newval, &intval)) - errx(1, "invalid value '%s'", - (char *)newval); + if (!set_IK(newval, &intval)) { + warn("invalid value '%s'%s", + (char *)newval, line); + return (1); + } } else { intval = (int)strtol(newval, &endptr, 0); - if (endptr == newval || *endptr != '\0') - errx(1, "invalid integer '%s'", - (char *)newval); + if (endptr == newval || *endptr != '\0') { + warn("invalid integer '%s'%s", + (char *)newval, line); + return (1); + } } newval = &intval; newsize = sizeof(intval); @@ -256,16 +293,16 @@ case CTLTYPE_UINT: uintval = (int) strtoul(newval, &endptr, 0); if (endptr == newval || *endptr != '\0') - errx(1, "invalid unsigned integer '%s'", - (char *)newval); + errx(1, "invalid unsigned integer " + "'%s'%s", (char *)newval, line); newval = &uintval; newsize = sizeof(uintval); break; case CTLTYPE_LONG: longval = strtol(newval, &endptr, 0); if (endptr == newval || *endptr != '\0') - errx(1, "invalid long integer '%s'", - (char *)newval); + errx(1, "invalid long integer '%s'%s", + (char *)newval, line); newval = &longval; newsize = sizeof(longval); break; @@ -273,7 +310,7 @@ ulongval = strtoul(newval, &endptr, 0); if (endptr == newval || *endptr != '\0') errx(1, "invalid unsigned long integer" - " '%s'", (char *)newval); + " '%s'%s", (char *)newval, line); newval = &ulongval; newsize = sizeof(ulongval); break; @@ -282,16 +319,16 @@ case CTLTYPE_S64: i64val = strtoimax(newval, &endptr, 0); if (endptr == newval || *endptr != '\0') - errx(1, "invalid int64_t '%s'", - (char *)newval); + errx(1, "invalid int64_t '%s'%s", + (char *)newval, line); newval = &i64val; newsize = sizeof(i64val); break; case CTLTYPE_U64: u64val = strtoumax(newval, &endptr, 0); if (endptr == newval || *endptr != '\0') - errx(1, "invalid uint64_t '%s'", - (char *)newval); + errx(1, "invalid uint64_t '%s'%s", + (char *)newval, line); newval = &u64val; newsize = sizeof(u64val); break; @@ -299,8 +336,8 @@ /* FALLTHROUGH */ default: errx(1, "oid '%s' is type %d," - " cannot set that", bufp, - kind & CTLTYPE); + " cannot set that%s", bufp, + kind & CTLTYPE, line); } i = show_var(mib, len); @@ -309,18 +346,20 @@ putchar('\n'); switch (errno) { case EOPNOTSUPP: - errx(1, "%s: value is not available", - string); + warn("%s: value is not available%s", string, + line); + return (1); case ENOTDIR: - errx(1, "%s: specification is incomplete", - string); + warn("%s: specification is incomplete%s", + string, line); + return (1); case ENOMEM: - errx(1, "%s: type is unknown to this program", - string); + warn("%s: type is unknown to this program%s", + string, line); + return (1); default: - warn("%s", string); - warncount++; - return; + warn("%s%s", string, line); + return (1); } } if (!bflag) @@ -332,8 +371,46 @@ putchar('\n'); nflag = i; } + + return (0); } +static int +parsefile(const char *filename) +{ + FILE *file; + char line[BUFSIZ], *p; + int warncount = 0, lineno = 0; + + file = fopen(filename, "r"); + if (file == NULL) + err(EX_NOINPUT, "%s", filename); + while (fgets(line, sizeof(line), file) != NULL) { + lineno++; + p = line; + /* Replace the first # with \0. */ + while((p = strchr(p, '#')) != NULL) { + if (p == line || *(p-1) != '\\') { + *p = '\0'; + break; + } + p++; + } + p = line; + while (isspace((int)p[strlen(p) - 1])) + p[strlen(p) - 1] = '\0'; + while (isspace((int)*p)) + p++; + if (*p == '\0') + continue; + else + warncount += parse(p, lineno); + } + fclose(file); + + return (warncount); +} + /* These functions will dump out various interesting structures. */ static int Index: etc/rc.d/sysctl =================================================================== --- etc/rc.d/sysctl (revision 243756) +++ etc/rc.d/sysctl (working copy) @@ -8,51 +8,27 @@ . /etc/rc.subr name="sysctl" +command=/sbin/sysctl stop_cmd=":" start_cmd="sysctl_start" reload_cmd="sysctl_start" lastload_cmd="sysctl_start last" extra_commands="reload lastload" -# -# Read in a file containing sysctl settings and set things accordingly. -# -parse_file() -{ - if [ -f $1 ]; then - while read var comments - do - case ${var} in - \#*|'') - ;; - *) - mib=${var%=*} - val=${var#*=} - - if current_value=`${SYSCTL} -n ${mib} 2>/dev/null`; then - case ${current_value} in - ${val}) - ;; - *) - if ! sysctl "${var}" >/dev/null 2>&1; then - warn "unable to set ${var}" - fi - ;; - esac - elif [ "$2" = "last" ]; then - warn "sysctl ${mib} does not exist." - fi - ;; - esac - done < $1 - fi -} - sysctl_start() { + case $1 in + last) + command_args="-i -f" + ;; + *) + command_args="-f" + ;; + esac - parse_file /etc/sysctl.conf $1 - parse_file /etc/sysctl.conf.local $1 + for _f in /etc/sysctl.conf /etc/sysctl.conf.local; do + [ -r $_f ] && $command $command_args $_f + done } load_rc_config $name ----Next_Part(Sun_Dec__2_01_50_48_2012_330)---- ----Security_Multipart0(Sun_Dec__2_01_50_48_2012_189)-- Content-Type: application/pgp-signature Content-Transfer-Encoding: 7bit -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.11 (FreeBSD) iEYEABECAAYFAlC6NWgACgkQTyzT2CeTzy0S9QCeMTjv42ScGn2UDpIkhacW38Xa byYAoNYcjnPOLWtoLwnt6TsGNo+RkRNa =19UY -----END PGP SIGNATURE----- ----Security_Multipart0(Sun_Dec__2_01_50_48_2012_189)----