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.

> 
> 
> 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... :)

> +     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

> +
>                       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

Reply via email to