Current-Users archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

Re: Proposed new /etc/rc.d/drvctl script



    Date:        Mon, 7 Apr 2025 23:58:13 -0700 (PDT)
    From:        Paul Goyette <paul%whooppee.com@localhost>
    Message-ID:  <Pine.NEB.4.64.2504072335130.5555%speedy.whooppee.com@localhost>

Looks good to me, though in this part:

  | 			*)
  | 				(
  | 					set -o noglob
  | 					eval set -- $args
  | 					drvctl "$@" "$name"
  | 				)

you don't really need the subshell, just (earlier) use 'local -'
to avoid the option sticking outside the script (though I believe
almost all rc.d/* scripts are already run in a subshell of their
own anyway) ... nothing in the script itself expects any filename
expansion, so the noglob option could also just be turned on once,
rather than for every drvctl command to be run.

The function does not use any positional args, so there is no issue
clobbering them either.

I assume the eval set is to allow for quoting in the args, to allow
whitespace or sh operators to appear, but is there really a need for
that here?  Plus the way you have done it would allow further
expansions in the args which is probably not desirable.

And last, while simply moving the first arg to last seems reasonable,
why?   Why not just have the config file say "-d dev" instead of "dev -d"?
If you want to keep the 'device first' model, then it would be better
to keep it always, rathar than onky when no further args to drvctl,
after the device name, are required.   That could be handled if needed
(easily if this is not intended to be pulled up, more messily otherwise)
but I won't bother with how unless it is needed.   Just making the
config file entry be ordered args to drvctl seems easier.

Then delele the eval set, use
	read args      (or 'read -r args' perhaps)
	[... still remove blank lines & comments as now]
	drvctl $args
(still with noglob set) and most issues vanish.

kre

ps: better would be to make tge config file name come from rc.conf
(with a default in defaukts/rc.conf) and certainly only put that
name in the script once, if not done that way, cfile=/etc/...
then use "${cfile}" where 8t is to be used ... use a better name
if it is to be set in defaults/rc.conf


Home | Main Index | Thread Index | Old Index