Hi Craig,
Craig Skinner wrote on Sat, Jun 27, 2015 at 02:24:18PM +0100:
> Here's a diff for the ntpd rc script pre start, to check the config.
I don't like that.
It's not the job of the "start" action to check the configuration.
Doing so is needless. When you edit any configuration file,
you will test the change anyway.
Doing so is even more needless. If i artificially break my
configuration file, i get this:
schwarze@isnote $ sudo rcctl stop ntpd
ntpd(ok)
schwarze@isnote $ sudoedit /etc/ntpd.conf
schwarze@isnote $ sudo rcctl start ntpd
ntpd(failed)
schwarze@isnote $ sudo rcctl -d start ntpd
doing _rc_parse_conf
doing _rc_quirks
ntpd_flags empty, using default ><
doing _rc_read_runfile
doing rc_check
ntpd
doing rc_start
/etc/ntpd.conf:14: syntax error
doing _rc_rm_runfile
(failed)
So, the current state of affairs already properly reports the
problem, and the current state of affairs already provides an
easy way to debug it.
Doing so is harmful. It wastes time, in particular during boot.
Doing so is even more harmful. One of the main assets of our rc.d(8)
system is simplicity, in particular when comparing it to overengineered,
bloated, new-fangled alternatives like SysV init. You are more
than doubling the size of /etc/rc.d/ntpd for almost no benefit.
The additional code and processing may adversely impact reliability
and could introduce bugs.
Besides, your code doesn't work (no need to fix it, though, as the
feature does not seem useful). The following would run the test
with the wrong configuration file: ntpd_flags="-f/other/file"
Yours,
Ingo
> Index: ntpd
> ===================================================================
> RCS file: /cvs/src/etc/rc.d/ntpd,v
> retrieving revision 1.2
> diff -u -p -r1.2 ntpd
> --- ntpd 14 Sep 2011 02:36:09 -0000 1.2
> +++ ntpd 27 Jun 2015 13:01:36 -0000
> @@ -9,4 +9,21 @@ daemon="/usr/sbin/ntpd"
> pexp="ntpd: \[priv\]"
> rc_reload=NO
>
> +
> +rc_pre()
> +{
> + # ntpd [-dnSsv] [-f file]
> + unset ntpd_conf
> + [[ ${daemon_flags} == -*f* ]] &&
> + {
> + for daemon_flag in ${daemon_flags}
> + do
> + [[ ${daemon_flag} == -* ]] && continue
> + ntpd_conf="-f ${daemon_flag}"
> + break
> + done
> + }
> + _rc_do ${daemon} -n ${ntpd_conf}
> +}
> +
> rc_cmd $1
