On Sat, Nov 26, 2022 at 10:44:05AM -0800, Andrew Hewus Fresh wrote:
> I think this is the complete diff, with the man page bits mostly from
> Martijn. Now that the ifconfig -M bits are in, this should be easier
> to test. I was able to test the installer bits, and they worked for me.
This reads straight forward, but see inline.
>
> Index: etc/netstart
> ===================================================================
> RCS file: /cvs/src/etc/netstart,v
> retrieving revision 1.229
> diff -u -p -r1.229 netstart
> --- etc/netstart 5 Nov 2022 12:06:05 -0000 1.229
> +++ etc/netstart 26 Nov 2022 17:21:38 -0000
> @@ -135,6 +135,23 @@ ifstart() {
> local _if=$1 _hn=/etc/hostname.$1 _cmds _i=0 _line _stat
> set -A _cmds
>
> + # If the _if looks like an lladdr and look it up.
That sentence is not proper english.
"Look up the interface if _if seems like an lladdr."
or
"Obtain the interface if _if is a MAC"
?
> + if [[ $_if = ??:??:??:??:??:?? ]]; then
Please use == for comparison; I know = works but it reads off.
Base should use == consistently, imho.
> + if ! _line="$( ifconfig -M "$_if" )"; then
Here you quote the subshell and the variable...
> + print -u2 "${0##*/}: unique if for lladdr $_if not
> found."
Maybe just "$_if is not unique"?
> + return
> + fi
> +
> + [[ -z "$_line" ]] && return
> + _if=$_line
> + _line=
> +
> + if [[ -e /etc/hostname.$_if ]]; then
> + print -u2 "${0##*/}: $_hn: /etc/hostname.$_if overrides"
> + return
> + fi
> + fi
> +
> # Interface names must be alphanumeric only. We check to avoid
> # configuring backup or temp files, and to catch the "*" case.
> [[ $_if != +([[:alpha:]])+([[:digit:]]) ]] && return
> @@ -183,14 +200,16 @@ ifmstart() {
> local _sifs=$1 _xifs=$2 _hn _if _sif _xif
>
> for _sif in ${_sifs:-ALL}; do
> - for _hn in /etc/hostname.+([[:alpha:]])+([[:digit:]]); do
> + for _hn in
> /etc/hostname.@(+([[:alpha:]])+([[:digit:]])|??:??:??:??:??:??); do
> [[ -f $_hn ]] || continue
> _if=${_hn#/etc/hostname.}
>
> - # Skip unwanted ifs.
> - for _xif in $_xifs; do
> - [[ $_xif == ${_if%%[0-9]*} ]] && continue 2
> - done
> + if [[ $_if = +([[:alpha:]])+([[:digit:]]) ]]; then
> + # Skip unwanted ifs.
> + for _xif in $_xifs; do
> + [[ $_xif == ${_if%%[0-9]*} ]] &&
> continue 2
> + done
> + fi
>
> # Start wanted ifs.
> [[ $_sif == @(ALL|${_if%%[0-9]*}) ]] && ifstart $_if
> Index: distrib/miniroot/install.sub
> ===================================================================
> RCS file: /cvs/src/distrib/miniroot/install.sub,v
> retrieving revision 1.1214
> diff -u -p -r1.1214 install.sub
> --- distrib/miniroot/install.sub 6 Nov 2022 21:32:54 -0000 1.1214
> +++ distrib/miniroot/install.sub 26 Nov 2022 17:21:38 -0000
> @@ -2431,6 +2431,12 @@ ifstart() {
> local _if=$1 _hn=/mnt/etc/hostname.$1 _cmds _i=0 _line
> set -A _cmds
>
> + if [[ $_if = ??:??:??:??:??:?? ]]; then
> + _if=$(ifconfig -M $_if)
... here you quote neither subshell nor variable.
I think that's fine, both install.sub and netstart already use unquoted
$_if and the subshell does not have to be quoted, anyway, since word
spliting does not occur:
$ v=$(echo 1 2 3)
$ echo $v
1 2 3
> + [[ -z $_if ]] && return # invalid interface
> + [[ -e /mnt/etc/hostname.$_if ]] && return # duplicate config
> + fi
> +
> # Create interface if it does not yet exist.
> { ifconfig $_if || ifconfig $_if create; } >/dev/null 2>&1 || return
>
> @@ -2472,6 +2478,9 @@ enable_ifs() {
> svlan) _svlans="$_svlans $_if";;
> vlan) _vlans="$_vlans $_if";;
> esac
> + elif [[ $_if = ??:??:??:??:??:?? ]]; then
==, please.
> + # start by lladdr
> + ifstart $_if
> else
> # 'Real' interfaces (if available) are done now.
> ifconfig $_if >/dev/null 2>&1 && ifstart $_if
> Index: share/man/man5/hostname.if.5
> ===================================================================
> RCS file: /cvs/src/share/man/man5/hostname.if.5,v
> retrieving revision 1.79
> diff -u -p -r1.79 hostname.if.5
> --- share/man/man5/hostname.if.5 26 Jul 2022 00:36:54 -0000 1.79
> +++ share/man/man5/hostname.if.5 26 Nov 2022 17:21:38 -0000
> @@ -41,9 +41,14 @@ The
> .Nm hostname.*\&
> files contain information regarding the configuration of each network
> interface.
> One file should exist for each interface that is to be configured, such as
> -.Pa hostname.fxp0
> +.Pa hostname.fxp0 ,
> +.Pa hostname.12:34:56:78:ab ,
> or
> .Pa hostname.bridge0 .
> +Additionally the interface name can be replaced with the interface's lladdr,
> +given that the lladdr is unique across the system and no
> +.Nm
> +exists for the interface name.
Isn't that too vague?
.Nm is hostname.if but you meant to say that .fxp0 should not exist when .MAC
already exists, no?
> A configuration file is not needed for lo0.
> .Pp
> The configuration information is expressed in a line-by-line packed format
>
