conf/163508: [rc.subr] [patch] Add " enable" and "
disable" commands to rc.subr
Devin Teske
devin.teske at fisglobal.com
Tue Dec 27 18:20:12 UTC 2011
The following reply was made to PR conf/163508; it has been noted by GNATS.
From: Devin Teske <devin.teske at fisglobal.com>
To: <bug-followup at FreeBSD.org>, <gelraen.ua at gmail.com>
Cc: <devin.teske at fisglobal.com>
Subject: Re: conf/163508: [rc.subr] [patch] Add "enable" and "disable" commands to rc.subr
Date: Tue, 27 Dec 2011 10:17:42 -0800
------=_NextPart_000_0285_01CCC480.C5B42FC0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
I would like to submit for review a modified version of the original submitter's
patch.
I feel that the original patch takes a too-simplistic view of the problem
at-hand and am offering a much more robust solution. The replacement patch uses
my "sysrc" utility which -- if you haven't discovered it yet -- is a
peer-reviewed "nuclear reactor" approach opposed to a "bike shed" approach.
sysrc takes everything into consideration, including (but not limited to):
(listed in no particular order)
1. Environment Variable Taint
It is not possible to "confuse" or "break" the code by exporting strange things
into the environment.
2. Shell Taint Checking
If rc.conf(5) has invalid syntax prior to editing, it will refuse to edit and an
error is produced. Similarly, if editing rc.conf(5) introduces a syntax error,
the original rc.conf(5) is restored and an error is produced.
This prevents producing a situation where rc.conf itself prevents you from
booting into multi-user mode. If, for any reason at all, rc.conf(5) causes you
to drop to single-user mode, it's assuredly is NOT because of sysrc, as it
taint-checks both before and after.
3. Safety First
Use mktemp to prevent race-conditions. Use atomic actions where necessary.
4. Minimal changes
The original patch submitted with conf/163508 does more work than is necessary
w/respect to items-changed within a single-file. Case-in-point, the
"replace_var" function simply fires a sed global-replace to replace all
instances of "thing=blah" on multiple lines. That's not cool.
Sysrc will ONLY change the last [valid] assignment to the variable. Sysrc wants
to keep your rc.conf exactly the way you like it as best it can and change as
little as possible (and when it does make changes, it wants to do it in a
fashion that's going to preserve as much structure as possible; see next item).
5. Quotations, whitespace, compound statements and in-line comments
The sed command in the original patch's "replace_var" function (a) WILL preserve
leading whitespace but (b) WON'T preserve in-line comments that appear after the
assignment, (c) NOR preserve the type of quotation that was used in the
assignment(s), (d) NOR preserve compound statements.
When sysrc replaces the last [valid] assignment to a given variable, it will
actually preserve the type of quotation used (whether it be single, double or
no-quotation). It will also preserve both leading whitespace and trailing
whitespace. It will also retain in-line comments. It will also preserve trailing
commands in a compound statement (multi-variable-assignment compound separated
by whitespace or multi-command compound separated by semi-colon).
6. Leave certain things alone!
Sysrc will refuse to modify things that it knows that it couldn't have possibly
done. For example, it will refuse to touch something like this:
foogribble_enable=`echo YES`
and instead opt to add an overriding new entry.
7. Performance
Sysrc has been rewritten several times to improve performance. It heavily
leverages awk to improve performance by several orders of magnitude compared to
using straight bourne-shell built-in internals.
( end itemized list ; continue discussion )
The above list calls out major flaws in the currently-submitted patch and
highlights the fact that sysrc has no such short-comings.
NOTE: sysrc would have to be taken into the base to support this patch. (aside)
MidnightBSD has taken it into its base already (and there's another distro that
escapes my mind at the moment, which has similarly taken it into its base)
NOTE: In my submitted patch, there are two open-ended questions to reviewers:
(a) ought we to use a full pathname to sysrc and if-so (b) where might sysrc be
placed? (/bin would be nice so that it's available in single-user mode).
--
Devin
P.S. sysrc is available here: http://druidbsd.sourceforge.net/download/sysrc.txt
_____________
The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.
------=_NextPart_000_0285_01CCC480.C5B42FC0
Content-Type: text/plain; name="conf_163508_patch.new.txt"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="conf_163508_patch.new.txt"
--- /usr/src.8/etc/rc.subr 2011-09-25 10:25:06.000000000 +0300=0A=
+++ /etc/rc.subr 2011-12-27 08:33:50.000000000 -0800=0A=
@@ -443,7 +443,7 @@=0A=
#=0A=
# run_rc_command argument=0A=
# Search for argument in the list of supported commands, which is:=0A=
-# "start stop restart rcvar status poll ${extra_commands}"=0A=
+# "start stop restart rcvar status poll enable disable =
${extra_commands}"=0A=
# If there's a match, run ${argument}_cmd or the default method=0A=
# (see below).=0A=
#=0A=
@@ -579,6 +579,10 @@=0A=
#=0A=
# rcvar Display what rc.conf variable is used (if any).=0A=
#=0A=
+# enable Set ${rcvar} to YES=0A=
+#=0A=
+# disable Set ${rcvar} to NO=0A=
+#=0A=
# Variables available to methods, and after run_rc_command() has=0A=
# completed:=0A=
#=0A=
@@ -647,7 +651,7 @@=0A=
eval _override_command=3D\$${name}_program=0A=
command=3D${_override_command:-$command}=0A=
=0A=
- _keywords=3D"start stop restart rcvar $extra_commands"=0A=
+ _keywords=3D"start stop restart rcvar enable disable $extra_commands"=0A=
rc_pid=3D=0A=
_pidcmd=3D=0A=
_procname=3D${procname:-${command}}=0A=
@@ -689,12 +693,26 @@=0A=
if [ "$_elem" !=3D "$rc_arg" ]; then=0A=
continue=0A=
fi=0A=
+=0A=
+ if [ -n "${rcvar}" -a "${rc_arg}" =3D=3D "enable" ]; then=0A=
+ if checkyesno ${rcvar}; then=0A=
+ echo "Service ${name} already enabled."=0A=
+ return 0=0A=
+ fi=0A=
+ fi=0A=
+ if [ -n "${rcvar}" -a "${rc_arg}" =3D=3D "disable" ]; then=0A=
+ if ! checkyesno ${rcvar}; then=0A=
+ echo "Service ${name} not enabled."=0A=
+ return 0=0A=
+ fi=0A=
+ fi=0A=
+=0A=
# if ${rcvar} is set, $1 is not "rcvar"=0A=
# and ${rc_pid} is not set, then run=0A=
# checkyesno ${rcvar}=0A=
# and return if that failed=0A=
#=0A=
- if [ -n "${rcvar}" -a "$rc_arg" !=3D "rcvar" -a "$rc_arg" !=3D "stop" =
] ||=0A=
+ if [ -n "${rcvar}" -a "$rc_arg" !=3D "rcvar" -a "$rc_arg" !=3D "stop" =
-a "$rc_arg" !=3D "enable" ] ||=0A=
[ -n "${rcvar}" -a "$rc_arg" =3D "stop" -a -z "${rc_pid}" ]; then=0A=
if ! checkyesno ${rcvar}; then=0A=
if [ -n "${rc_quiet}" ]; then=0A=
@@ -895,6 +913,16 @@=0A=
echo ""=0A=
;;=0A=
=0A=
+ enable|disable)=0A=
+ local val=0A=
+ if [ "$rc_arg" =3D "enable" ]; then=0A=
+ val=3D"YES"=0A=
+ else=0A=
+ val=3D"NO"=0A=
+ fi=0A=
+ sysrc "$rcvar=3D$val" && sysrc -v "$rcvar"=0A=
+ ;;=0A=
+=0A=
*)=0A=
rc_usage $_keywords=0A=
;;=0A=
------=_NextPart_000_0285_01CCC480.C5B42FC0--
More information about the freebsd-rc
mailing list