Date: Sun, 02 Dec 2012 01:50:48 +0900 (JST) From: Hiroki Sato <hrs@FreeBSD.org> To: current@FreeBSD.org Subject: RFC: sysctl -f filename Message-ID: <20121202.015048.1122480556487090170.hrs@allbsd.org>
next in thread | raw e-mail | index | archive | help
----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 <stdio.h>
#include <stdlib.h>
#include <string.h>
+#include <sysexits.h>
#include <unistd.h>
+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)----
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20121202.015048.1122480556487090170.hrs>