On Sun, Jul 03, 2022 at 01:55:44PM +0200, Alexander Hall wrote:
> On Sun, Jul 03, 2022 at 09:59:21AM +0000, Klemens Nanni wrote:
> > ...
> > _ifs seems like idiomatic way in our shell code, i.e. assign function
> > arguments to local variables, but $@ is a bit special.
> >
> > In fact, "$@" is `set -u' clean whereas "${_ifs[@]}" is not.
> > netstart does not use `set -u', but it's still an argument for the $@:
> >
> > $ set -u
> > $ set -A _ifs -- "$@"
> > $ echo "$@"
> >
> > $ echo "${_ifs[@]}"
> > ksh: _ifs[@]: parameter not set
>
> FWIW, this would work if $@ had any parameters. IIRC,
> `set -A <varname> --` with no additional parameters is essentially the
> same as unset.
>
> $ set -u
> $ set -A a -- 1 2 3
> $ echo ${a[@]}
> 1 2 3
> $ set -A a --
> $ echo ${a[@]}
> ksh: a[@]: parameter not set
>
> >
> >
> > This works like a charm for me and behaviour only differs if I actually
> > pass interfaces:
> >
> > # sh /etc/netstart -n > old
> > # sh /usr/src/etc/netstart -n > new
> > # diff -u old new ; echo $?
> > 0
> >
> > # sh /etc/netstart -n pair1 pair2 > old-ifs
> > # sh /usr/src/etc/netstart -n pair1 pair2 > new-ifs
> > # diff -u old-ifs new-ifs
> > --- old-ifs Sun Jul 3 13:54:51 2022
> > +++ new-ifs Sun Jul 3 13:54:45 2022
> > @@ -1,4 +1,6 @@
> > { ifconfig pair1 || ifconfig pair1 create; }
> > +{ ifconfig pair2 || ifconfig pair2 create; }
> > +{ ifconfig pair1 || ifconfig pair1 create; }
> > ifconfig pair1 inet 192.0.0.4/29
> > ifconfig pair1 patch pair2
> > { ifconfig pair2 || ifconfig pair2 create; }
> >
> >
> > Feedback? Objection? OK?
>
> OK halex@, with two minor nits, free to ignore, below.
Thanks.
> > Index: netstart
> > ===================================================================
> > RCS file: /cvs/src/etc/netstart,v
> > retrieving revision 1.218
> > diff -u -p -r1.218 netstart
> > --- netstart 26 Jun 2022 09:36:13 -0000 1.218
> > +++ netstart 3 Jul 2022 09:46:05 -0000
> > @@ -11,6 +11,17 @@ usage() {
> > exit 1
> > }
> >
> > +# Test the first argument against the remaining ones, return success on a
> > match.
> > +isin() {
> > + local _a=$1 _b
> > +
> > + shift
> > + for _b; do
> > + [[ $_a == "$_b" ]] && return 0
> ^ Superfluous but, well... :)
I won't change the function here as it is shared and synced code.
If at all, all copies should be adapted.
> > + done
> > + return 1
> > +}
> > +
> > # Echo file $1 to stdout. Skip comment lines. Strip leading and trailing
> > # whitespace if IFS is set.
> > # Usage: stripcom /path/to/file
> > @@ -94,7 +105,8 @@ ifcreate() {
> > }
> >
> > # Create interfaces for network pseudo-devices referred to by hostname.if
> > files.
> > -# Usage: vifscreate
> > +# Optionally, limit creation to given interfaces only.
> > +# Usage: vifscreate [if ...]
> > vifscreate() {
> > local _vif _hn _if
> >
> > @@ -106,6 +118,10 @@ vifscreate() {
> > # loopback for routing domain is created by kernel
> > [[ -n ${_if##lo[1-9]*} ]] || continue
> >
> > + if (($# > 0)) && ! isin $_if "$@"; then
> > + continue
> > + fi
>
> A simple && chain would follow the loopback exception syntax above.
>
> (($# > 0)) && ! isin $_if "$@" && continue
Which is a bit too close to
(($# > 0)) && isin $_if "$@" || continue
which reads the same but will also `continue' if $# is zero, a common
pitfall with &&/||, hence why I opted for the explicit if/then around
combined conditions.
> > +
> > if ! ifcreate $_if; then
> > print -u2 "${0##*/}: create for '$_if' failed."
> > fi
> > @@ -313,7 +329,11 @@ $PRINT_ONLY || [[ ! -f /etc/soii.key ]]
> >
> > # If we were invoked with a list of interface names, just reconfigure these
> > # interfaces (or bridges), add default routes and return.
> > +# Create virtual interfaces upfront to make ifconfig commands depending on
> > +# other interfaces, e.g. "patch", work regardless of in which order
> > interface
> > +# names were specified.
> > if (($# > 0)); then
> > + vifscreate "$@"
> > for _if; do ifstart $_if; done
> > defaultroute
> > return
>
> /Alexander
>
