Bug 192697 - /etc/rc.subr "find_local_scripts_old()" is broken for all versions of freebsd
Summary: /etc/rc.subr "find_local_scripts_old()" is broken for all versions of freebsd
Status: Open
Alias: None
Product: Base System
Classification: Unclassified
Component: conf (show other bugs)
Version: 9.2-STABLE
Hardware: Any Any
: --- Affects Many People
Assignee: Jilles Tjoelker
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-16 01:21 UTC by hostmaster
Modified: 2023-02-01 07:44 UTC (History)
1 user (show)

See Also:


Attachments
patch for /etc/rc.subr "find_local_scripts_old()" (693 bytes, patch)
2014-08-16 01:21 UTC, hostmaster
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description hostmaster 2014-08-16 01:21:59 UTC
Created attachment 145853 [details]
patch for /etc/rc.subr "find_local_scripts_old()"

If there are no /usr/local/etc/rc.d/*.sh scripts "$zlist" & "$slist" are non-empty upon return:

zlist=' /usr/local/etc/rc.d/[0-9]*.sh'
slist=' /usr/local/etc/rc.d/[^0-9]*.sh'

A patch is attached.
Comment 1 Fernando Apesteguía freebsd_committer 2023-01-24 12:47:56 UTC
$ ls /usr/local/etc/rc.d/
avahi-daemon*   avahi-dnsconfd* cupsd*          dbus*           git_daemon*     poudriered*     slim*           svnserve*

$ source /etc/rc.subr 
$ find_local_scripts_old

Checking variables (which are recognized and autocompleted by the shell)

$ echo "+++"$zlist"+++"
++++++
$ echo "+++"$slist"+++"
++++++

Look empty.

Closing.
Comment 2 Bruce Becker 2023-01-24 15:16:25 UTC
the test you used to test does not include $local_startup

in bourne sh:
> cd 
> mkdir x
> local_startup=/root/x
> . /etc/rc.subr
> set +x
> find_local_scripts_old
+ find_local_scripts_old
+ zlist=''
+ slist=''
+ [ -d /root/x ]
+ grep '^# PROVIDE:' '/root/x/[0-9]*.sh'
+ zlist=' /root/x/[0-9]*.sh'
+ grep '^# PROVIDE:' '/root/x/[!0-9]*.sh'
+ slist=' /root/x/[!0-9]*.sh'

This was done on FreeBSD 12.x
Comment 3 Fernando Apesteguía freebsd_committer 2023-01-24 16:04:35 UTC
You're right, I can confirm.

Assigning jilles@ based on git history to see if he can have a look at this.
Comment 4 Jilles Tjoelker freebsd_committer 2023-01-24 20:20:49 UTC
This "bug" in find_local_scripts_old has existed since it was added in SVN r153027 / git 0f3ce2b32c1dd5e82c838030e87f1f868cf58af9. It may not really be a bug since its caller libexec/rc/rc.subr (installed as /etc/rc.subr) compensates for it.

As for a fix, the proposed  [ "`echo $f`" != "$f" ]  works in this particular case, but I'd say a check  [ -f "$file" ] || continue  in the loop would be more idiomatic (since it is more common and also works for patterns that match themselves or contain quoted characters).
Comment 5 Fernando Apesteguía freebsd_committer 2023-01-25 17:35:51 UTC
(In reply to Jilles Tjoelker from comment #4)
Does that mean that the patch is good if we use [ -f "$file" ] || continue ? :-)
Comment 6 Jilles Tjoelker freebsd_committer 2023-01-29 16:28:47 UTC
Looking at it again, I think the original version with  [ "`echo $f`" != "$f" ]  is better. The code is a bit strangely factored, and /etc/rc.d/localpkg in fact depends on non-executable files and dangling symlinks being included for the " (skipping ${script}, not executable)" message. Only the pattern itself (if it does not match any files) is safe to exclude.

The more idiomatic pattern with  [ -f "$file" ] || continue  (or a similar condition) would apply to loops that perform an actual action rather than just collecting a list to be processed later.
Comment 7 Fernando Apesteguía freebsd_committer 2023-01-29 17:39:21 UTC
(In reply to Jilles Tjoelker from comment #6)
Thanks Jills for looking into this.
Will you proceed with the fix? I'm not a src committer, but if you want me to do it, just let me know.
Comment 8 Jilles Tjoelker freebsd_committer 2023-01-31 23:05:05 UTC
(In reply to Fernando Apesteguía from comment #7)
I will take care of it.
Comment 9 Fernando Apesteguía freebsd_committer 2023-02-01 07:44:01 UTC
(In reply to Jilles Tjoelker from comment #8)
Thanks!