Date: Mon, 17 Aug 2026 21:46:53 -0700
From: Jason Thorpe <[email protected]>
Message-ID: <[email protected]>
| The proposed changes, here:
| https://www.netbsd.org/~thorpej/rcorder-cache-diff-v2.txt
One obvious trivial change, in rc.subr:_rc_order_cache_is_valid()
the test
if [ -f /etc/rcorder.cache ] && ...
should clearly be using -s instead of -f ... there's no point wasting
time checking mod times on everything if the later "Verify there is at
least one valid file..." is guaranteed to fail because the cache (while
existing) is empty.
Second, the uses of $f throughout that function should be quoted. It might
not seem like it should be needed, but it is (on the other hand, the assignment
_rc_ordered_script_list="$allf"
and other similar ones don't need the quotes, but they are harmless.)
More importantly, nothing there (I can see) is checking that the
rc.d/* files themselves have not been changed, just that the directory
hasn't. Using a sub-standard editor to change the files might change
the directory as a side effect of updating a file in it, but something
simple like cp new-rc-file /etc/rc.d/script will not (assuming the script
existed already) and nor would editing the file with a good editor.
If the change to the file were, for example, to alter the default setting
of the rcvar (so instead of requiring rcvar=YES in rc.conf to enable the
script, it now requires rcvar=NO in rc.conf (or defaults) to disable it),
then if done using "cp" or a good editor, would totally break the proposed
scheme. That kind of change is perhaps unlikely in NetBSD standard rc.d
scripts, but is entirely possible in pkgsrc supplied scripts, which generally
do not go about altering /etc/defaults/rc.conf to modify the default yes/no
state, it is in the script itself.
The script needs to check every single rc.d script to see if it has been
modified, and at the very least, include that file in the list to be
evaluated (which would not be trivial to do alone, and put it in the
correct location in the sequence with the current scheme, so probably
any single changed file needs to mean to rebuild the cache).
Because of that, its fallback (if use of this mechanism is disabled)
probably should be to leave the status quo alone (no point doing all
that checking if it is not
Of course test's -nt operator only compares modified times (and there's
nothing which checks the st_ctime values currently in test(1), it would
need a new operator (operators probably) or use of "stat(1)" (which can't
be done at boot time, as it is /usr/bin/stat) to check the better time
field (none of the rc.d scripts or dirs will normally have either st_mtime or
st_ctime altered, so testing ctime rather than mtime, were a test available,
would not make a practical difference, except in the odd case that ctime
does get altered, but mtime doesn't ... the converse never happens.)
(This one though is the more minor of the fixes needed.)
Last, few of the "echo" commands in the changes, except for the ones in
the new rcorder_cache script, the two in_rc_order_save_cache(), and the
two in _rc_order_build_list() are intended to write anything to the console.
All the other uses of "echo" should be "command echo" instead, to avoid
using the echo() function defined in rc.subr which deals with ensuring
partial output lines to the console are properly formatted (not needed
when the echo is just writing to a file, or to the output of a command
substitution.)
They should really be using printf instead of echo (avoid issues with
script names starting with a '-' or containing \ chars for example) but
that would still want "command printf" to avoid getting rc.subr()'s
printf() function, which exists for the same reason as its echo().
Note using rc.subr()'s echo() function does use printf, not echo, so the
problems with using echo are suppressed in that case, but using "command echo"
would defeat that protection, so use "command printf" instead (just do it
correctly.)
kre
