Re: RFC: sysctl -f filename

On Sat, Dec 1, 2012 at 2:10 PM, Garrett Cooper <yanegomi_at_gmail.com> wrote:
> On Sat, Dec 1, 2012 at 8:50 AM, Hiroki Sato <hrs_at_freebsd.org> wrote:
>> 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.
>
>     Why change the tool when we can change the rc script to do the
> right thing? I have a patch I'm working on to resolve this (you hit an
> itch I've been meaning to scratch for a little while).

    This should work. I also refactored the script to get it down to
80 columns. I've attached the debug output and the diff for the debug
version of the script.
Cheers,
-Garrett

# diff -u etc/rc.d/sysctl etc/rc.d/sysctl.debug  | sed -e 's,\.debug,,'
--- etc/rc.d/sysctl     2012-12-01 14:29:31.948502097 -0800
+++ etc/rc.d/sysctl     2012-12-01 14:29:21.694501765 -0800
_at__at_ -38,6 +38,10 _at__at_
                val="${val#\"*}"
                val="${val%%\"*}"

+               debug "line -> '$line'"
+               debug "mib -> '$mib'"
+               debug "val -> '$val'"
+
                if current_value=`${SYSCTL} -n "${mib}" 2>/dev/null`; then
                        if [ "${current_value}" = "${val}" ]; then
                                continue

./etc/rc.d/sysctl: DEBUG: run_rc_command: doit: sysctl_start
./etc/rc.d/sysctl: DEBUG: line -> 'net.inet.tcp.recvspace=262144'
./etc/rc.d/sysctl: DEBUG: mib -> 'net.inet.tcp.recvspace'
./etc/rc.d/sysctl: DEBUG: val -> '262144'
./etc/rc.d/sysctl: DEBUG: line -> 'net.inet.tcp.sendspace="231072'
./etc/rc.d/sysctl: DEBUG: mib -> 'net.inet.tcp.sendspace'
./etc/rc.d/sysctl: DEBUG: val -> '231072'
./etc/rc.d/sysctl: DEBUG: line -> 'net.inet.tcp.sendspace="131074"'
./etc/rc.d/sysctl: DEBUG: mib -> 'net.inet.tcp.sendspace'
./etc/rc.d/sysctl: DEBUG: val -> '131074'
./etc/rc.d/sysctl: DEBUG: line -> 'net.inet.udp.recvspace=131072'
./etc/rc.d/sysctl: DEBUG: mib -> 'net.inet.udp.recvspace'
./etc/rc.d/sysctl: DEBUG: val -> '131072'
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile='
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> ''
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="#'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '#'
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="# abcd'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '# abcd'
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="# abcd" #'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '# abcd'
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="# abcd"       #'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '# abcd'
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="# abcd " #'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '# abcd '
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="# abcd "      #'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '# abcd '
./etc/rc.d/sysctl: DEBUG: line -> 'kern.corefile="%N.core"'
./etc/rc.d/sysctl: DEBUG: mib -> 'kern.corefile'
./etc/rc.d/sysctl: DEBUG: val -> '%N.core'