2014-10-15 13:11 GMT+04:00 Craig R. Skinner <[email protected]>:
> On 2014-10-14 Tue 00:24 AM |, Antoine Jacoutot wrote:
>>
>> Makes sense yes. Not sure I'd want a function just for that one liner though.
>> I'll commit something tomorrow.
>>
>
> Nice one, using shell internals.
>
> This restricts the listing to files which are also executable:
>
>
> Index: rcctl.sh
> ===================================================================
> RCS file: /cvs/src/usr.sbin/rcctl/rcctl.sh,v
> retrieving revision 1.45
> diff -u -p -r1.45 rcctl.sh
> --- rcctl.sh    15 Oct 2014 07:38:24 -0000      1.45
> +++ rcctl.sh    15 Oct 2014 09:01:35 -0000
> @@ -39,10 +39,12 @@ needs_root()
>  ls_rcscripts() {
>         local _s
>
> -       cd /etc/rc.d && set -- *
> +       cd /etc/rc.d || exit
> +       set -- *
>         for _s; do
>                 [ "${_s}" = "rc.subr" ] && continue
> -               [ ! -d "${_s}" ] && echo "${_s}"
> +               [[ -d "${_s}" ]] && continue
> +               [[ -f "${_s}" && -x "${_s}" ]] && echo "${_s}"
>         done
>  }

It's mostly nitpicking in that particular case (I don't think
/etc/rc.d ought to be large enough), but, anyway, I usually prefer to
use "ls -f | while read" idiom instead of "set -- *; for". The first
way is more Unix-style as it's stream-oriented, i.e., doesn't keep the
whole list in memory.

For the "|| exit" part - maybe rcctl.sh wants to have "set -e" instead?

--
  WBR,
  Vadim Zhukov

Reply via email to