Skip site navigation (1)Skip section navigation (2)
Date:      Mon, 31 Mar 2014 12:04:35 -0700
From:      <dteske@FreeBSD.org>
To:        "'Hiroki Sato'" <hrs@FreeBSD.org>, <dteske@FreeBSD.org>
Cc:        jhellenthal@dataix.net, rc@FreeBSD.org
Subject:   RE: network.subr _aliasN handling
Message-ID:  <069f01cf4d14$0ff03980$2fd0ac80$@FreeBSD.org>
In-Reply-To: <20140331.121757.1100840815853109946.hrs@allbsd.org>
References:  <04c901cf4c5d$a6a4a030$f3ede090$@FreeBSD.org>	<0EBE3981-DC85-414D-85B8-7638F172040A@dataix.net>	<04f701cf4c85$d1929680$74b7c380$@FreeBSD.org> <20140331.121757.1100840815853109946.hrs@allbsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
------=_NextPart_000_06A0_01CF4CD9.6391AFA0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit



> -----Original Message-----
> From: Hiroki Sato [mailto:hrs@FreeBSD.org]
> Sent: Sunday, March 30, 2014 8:18 PM
> To: dteske@FreeBSD.org
> Cc: jhellenthal@dataix.net; lists@jnielsen.net; rc@FreeBSD.org
> Subject: Re: network.subr _aliasN handling
> 
> <dteske@FreeBSD.org> wrote
>   in <04f701cf4c85$d1929680$74b7c380$@FreeBSD.org>:
> 
> dt> But that wouldn't have deterred me. 30+ days of silence is
> dt> equivalent to acceptance -- just that I had noticed that the patch
> dt> could be expanded to include mdconfig{,2} scripts. Was going to wait
> dt> a full day to see if anyone balked at the expansion to include
> dt> mdconfig{,2} and then move forward.
> 
>  I like the direction in general, but there are two more comments:
> 
>  1. sort(1) cannot be used [snip]

Problem solved. I just wrote a mini-sort(1) in shell that uses only
shell built-ins (now it doesn't matter what's mounted). See attached
patch.txt (added sort_lite() function to rc.subr).

> 
>  2. Please put the normalization part into a function and use it in
>     get_if_var(), too.  Adding another code for the same functionality
>     makes maintenance difficult.

There is already a function for doing the normalization ... ltr()
and it is already used by get_if_var.

However, I am not using ltr() because it itself degrades performance.
Because ltr() produces its result on stdout, it requires the use of a
sub-shell to capture the result.

NB: I think a good approach would be to fix ltr() and use it instead of
creating some new function and changing get_if_var() to use it instead
of ltr().

>  It degrades the performance a bit
>     but I think maintainability is more important for that.
> 

Negative. The 3-lines of code you're talking about operate about 1000
times faster than calling ltr() in a sub-shell to perform the same action.

That being said... let's do this... let's give ltr() a new argument that can
be used to set a variable in the caller's name-space rather than requiring
me to call it in a sub-shell to cull the answer.

See attached patch wherein I also modify ltr() to fit our needs.

However, while we're here, let's not point a finger at this
glaringly obvious problem with ltr():

dteske@scribe9.vicor.com etc $ sh -c '. /etc/rc.subr; echo "IFS=[$IFS]"; ltr
foo.bar . _; echo "IFS=[$IFS]"'
IFS=[
]
foo_bar
IFS=[.]

It modifies IFS and doesn't restore it. Let me fix that while I'm here. Oh,
and let's quote those positional arguments too so that *maybe* someone
could pass whitespace into the IFS (among other things).

Oh look, while we're here let's make get_if_var() more efficient. Because
the following is just plain silly...

	        _punct=". - / +"
	        for _punct_c in $_punct; do
	                _if=`ltr ${_if} ${_punct_c} '_'`
	        done

Should instead be...

	_punct=".-/+"
	_if=`ltr ${_if} "${_punct}" '_'`

But of course, with the new ltr() syntax, we could get rid of all the
sub-shells:

	_punct=".-/+"
	ltr "$_if" "$_punct" '_' "_if"

Please see attached patch.
-- 
Devin

_____________
The information contained in this message is proprietary and/or confidential. If you are not the intended recipient, please: (i) delete the message and all copies; (ii) do not disclose, distribute or use the message in any manner; and (iii) notify the sender immediately. In addition, please be aware that any message addressed to our domain is subject to archiving and review by persons other than the intended recipient. Thank you.

------=_NextPart_000_06A0_01CF4CD9.6391AFA0
Content-Type: text/plain; name="patch.txt"
Content-Transfer-Encoding: quoted-printable
Content-Disposition: attachment; filename="patch.txt"

Index: head/etc/network.subr=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
--- etc/network.subr	(revision 263957)=0A=
+++ etc/network.subr	(working copy)=0A=
@@ -283,10 +283,8 @@ get_if_var()=0A=
 	fi=0A=
 =0A=
 	_if=3D$1=0A=
-	_punct=3D". - / +"=0A=
-	for _punct_c in $_punct; do=0A=
-		_if=3D`ltr ${_if} ${_punct_c} '_'`=0A=
-	done=0A=
+	_punct=3D".-/+"=0A=
+	ltr ${_if} "${_punct}" '_' _if=0A=
 	_var=3D$2=0A=
 	_default=3D$3=0A=
 =0A=
@@ -1076,6 +1074,7 @@ ifalias_af_common_handler()=0A=
 ifalias_af_common()=0A=
 {=0A=
 	local _ret _if _af _action alias ifconfig_args _aliasn _c _tmpargs _iaf=0A=
+	local _punct=3D".-/+"=0A=
 =0A=
 	_ret=3D1=0A=
 	_aliasn=3D=0A=
@@ -1083,10 +1082,14 @@ ifalias_af_common()=0A=
 	_af=3D$2=0A=
 	_action=3D$3=0A=
 =0A=
+	# Normalize $_if before using it in a pattern to list_vars()=0A=
+	ltr "$_if" "$_punct" "_" _if=0A=
+=0A=
 	# ifconfig_IF_aliasN which starts with $_af=0A=
-	alias=3D0=0A=
-	while : ; do=0A=
-		ifconfig_args=3D`get_if_var $_if ifconfig_IF_alias${alias}`=0A=
+	for alias in `list_vars ifconfig_${_if}_alias[0-9]\* |=0A=
+		sort_lite -nk1.$((9+${#_if}+7))`=0A=
+	do=0A=
+		eval ifconfig_args=3D\"\$$alias\"=0A=
 		_iaf=3D=0A=
 		case $ifconfig_args in=0A=
 		inet\ *)	_iaf=3Dinet ;;=0A=
@@ -1107,15 +1110,15 @@ ifalias_af_common()=0A=
 			warn "\$ifconfig_${_if}_alias${alias} needs " \=0A=
 			    "\"inet\" keyword for an IPv4 address."=0A=
 		esac=0A=
-		alias=3D$(($alias + 1))=0A=
 	done=0A=
 =0A=
 	# backward compatibility: ipv6_ifconfig_IF_aliasN.=0A=
 	case $_af in=0A=
 	inet6)=0A=
-		alias=3D0=0A=
-		while : ; do=0A=
-			ifconfig_args=3D`get_if_var $_if ipv6_ifconfig_IF_alias${alias}`=0A=
+		for alias in `list_vars ipv6_ifconfig_${_if}_alias[0-9]\* |=0A=
+			sort_lite -nk1.$((14+${#_if}+7))`=0A=
+		do=0A=
+			eval ifconfig_args=3D\"\$$alias\"=0A=
 			case ${_action}:"${ifconfig_args}" in=0A=
 			*:"")=0A=
 				break=0A=
@@ -1127,7 +1130,6 @@ ifalias_af_common()=0A=
 				    "instead."=0A=
 			;;=0A=
 			esac=0A=
-			alias=3D$(($alias + 1))=0A=
 		done=0A=
 	esac=0A=
 =0A=
Index: head/etc/rc.d/mdconfig=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
--- etc/rc.d/mdconfig	(revision 263957)=0A=
+++ etc/rc.d/mdconfig	(working copy)=0A=
@@ -181,17 +181,14 @@ fi=0A=
 =0A=
 load_rc_config $name=0A=
 =0A=
-_mdconfig_unit=3D0=0A=
 if [ -z "${_mdconfig_list}" ]; then=0A=
-	while :; do=0A=
-		eval _mdconfig_config=3D\$mdconfig_md${_mdconfig_unit}=0A=
-		if [ -z "${_mdconfig_config}" ]; then=0A=
-			break=0A=
-		else=0A=
-			_mdconfig_list=3D"${_mdconfig_list}${_mdconfig_list:+ =
}md${_mdconfig_unit}"=0A=
-			_mdconfig_unit=3D$((${_mdconfig_unit} + 1))=0A=
-		fi=0A=
+	for _mdconfig_config in `list_vars mdconfig_md[0-9]\* |=0A=
+		sort_lite -nk1.12`=0A=
+	do=0A=
+		_mdconfig_unit=3D${_mdconfig_config#mdconfig_md}=0A=
+		_mdconfig_list=3D"$_mdconfig_list md$_mdconfig_unit"=0A=
 	done=0A=
+	_mdconfig_list=3D"${_mdconfig_list# }"=0A=
 fi=0A=
 =0A=
 run_rc_command "${_mdconfig_cmd}"=0A=
Index: head/etc/rc.d/mdconfig2=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
--- etc/rc.d/mdconfig2	(revision 263957)=0A=
+++ etc/rc.d/mdconfig2	(working copy)=0A=
@@ -211,17 +211,14 @@ fi=0A=
 =0A=
 load_rc_config $name=0A=
 =0A=
-_mdconfig2_unit=3D0=0A=
 if [ -z "${_mdconfig2_list}" ]; then=0A=
-	while :; do=0A=
-		eval _mdconfig2_config=3D\$mdconfig_md${_mdconfig2_unit}=0A=
-		if [ -z "${_mdconfig2_config}" ]; then=0A=
-			break=0A=
-		else=0A=
-			_mdconfig2_list=3D"${_mdconfig2_list}${_mdconfig2_list:+ =
}md${_mdconfig2_unit}"=0A=
-			_mdconfig2_unit=3D$((${_mdconfig2_unit} + 1))=0A=
-		fi=0A=
+	for _mdconfig2_config in `list_vars mdconfig_md[0-9]\* |=0A=
+		sort_lite -nk1.12`=0A=
+	do=0A=
+		_mdconfig2_unit=3D${_mdconfig2_config#mdconfig_md}=0A=
+		_mdconfig2_list=3D"$_mdconfig2_list md$_mdconfig2_unit"=0A=
 	done=0A=
+	_mdconfig2_list=3D"${_mdconfig2_list# }"=0A=
 fi=0A=
 =0A=
 run_rc_command "${_mdconfig2_cmd}"=0A=
Index: head/etc/rc.subr=0A=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=0A=
--- etc/rc.subr	(revision 263957)=0A=
+++ etc/rc.subr	(working copy)=0A=
@@ -54,6 +54,20 @@ JID=3D`$PS -p $$ -o jid=3D`=0A=
 #	functions=0A=
 #	---------=0A=
 =0A=
+# list_vars pattern=0A=
+#	List vars matching pattern.=0A=
+# =0A=
+list_vars()=0A=
+{=0A=
+	set | { while read LINE; do=0A=
+		var=3D"${LINE%%=3D*}"=0A=
+		case "$var" in=0A=
+		"$LINE"|*[!a-zA-Z0-9_]*) continue ;;=0A=
+		$1) echo $var=0A=
+		esac=0A=
+	done; }=0A=
+}=0A=
+=0A=
 # set_rcvar_obsolete oldvar [newvar] [msg]=0A=
 #	Define obsolete variable.=0A=
 #	Global variable $rcvars_obsolete is used.=0A=
@@ -314,7 +328,109 @@ _find_processes()=0A=
 	eval $_proccheck=0A=
 }=0A=
 =0A=
+# sort_lite [-n] [-k POS]=0A=
+#	A lite version of sort(1) supporting a few options that can be used=0A=
+#	before the real sort(1) is available (e.g., in scripts that run prior=0A=
+#	to mountcritremote). Requirs only shell built-in functionality.=0A=
 #=0A=
+sort_lite()=0A=
+{=0A=
+	local nitems=3D0=0A=
+	local skip_leading=3D0 trim=3D=0A=
+	local numeric=3D key=3D1=0A=
+=0A=
+	local OPTIND flag=0A=
+	while getopts nk: flag; do=0A=
+		case "$flag" in=0A=
+		n) numeric=3D1 ;;=0A=
+		k) key=3D"${OPTARG%%,*}" ;; # Unlike sort(1) only one POS allowed=0A=
+		\?) return 1 ;;=0A=
+		esac=0A=
+	done=0A=
+	shift $(( $OPTIND - 1 ))=0A=
+=0A=
+	# If first argument is an integer, ignore N leading bytes=0A=
+	case "$1" in=0A=
+	*[!0-9]*|"") : not an integer ;;=0A=
+	*) skip_leading=3D$1=0A=
+	esac=0A=
+=0A=
+	# Create transformation pattern to trim leading text if desired=0A=
+	case "$key" in=0A=
+	""|[!0-9]*|*[!0-9.]*)=0A=
+		echo "sort_lite: invalid number at field start:" \=0A=
+		     "invalid count at start of \`$key'" >&2=0A=
+		return 1=0A=
+		;;=0A=
+	*.*)=0A=
+		skip_leading=3D${key#*.} key=3D${key%%.*}=0A=
+		while [ ${skip_leading:-0} -gt 1 ] 2> /dev/null; do=0A=
+			trim=3D"$trim?" skip_leading=3D$(( $skip_leading - 1 ))=0A=
+		done=0A=
+	esac=0A=
+=0A=
+	# Copy input to series of local numbered variables=0A=
+	local LINE=0A=
+	while read LINE; do=0A=
+		nitems=3D$(( $nitems + 1 ))=0A=
+		local src_$nitems=0A=
+		setvar src_$nitems "$LINE"=0A=
+	done=0A=
+=0A=
+	# Sort numbered locals using insertion sort=0A=
+	local i=3D1 k=3D0 item kitem gt=3D">"=0A=
+	[ "$numeric" ] && gt=3D"-gt"=0A=
+	while [ $i -le $nitems ]; do=0A=
+		eval item=3D\"\$src_$i\" kitem=3D\"\${src_$k:-0}\"=0A=
+		key_item=3D"$item" key_kitem=3D"$kitem"=0A=
+		_key=3D$key=0A=
+		while [ $_key -gt 1 ]; do=0A=
+			key_item=3D"${key_item#*[$IFS]}"=0A=
+			while case "$key_item" in=0A=
+				[$IFS]*) true;; *) false; esac=0A=
+			do=0A=
+				key_item=3D"${key_item#?}"=0A=
+			done=0A=
+			key_kitem=3D"${key_kitem#*[$IFS]}"=0A=
+			while case "$key_kitem" in=0A=
+				[$IFS]*) true;; *) false; esac=0A=
+			do=0A=
+				key_kitem=3D"${key_kitem#?}"=0A=
+			done=0A=
+			_key=3D$(( $_key - 1 ))=0A=
+		done=0A=
+		while [ $k -gt 0 -a \=0A=
+		    "${key_kitem#$trim}" $gt "${key_item#$trim}" ] 2> /dev/null=0A=
+		do=0A=
+			eval dest_$(( $k + 1 ))=3D\"\$dest_$k\"=0A=
+			k=3D$(( $k - 1 ))=0A=
+			eval kitem=3D\"\$src_$k\"=0A=
+			key_kitem=3D"$kitem"=0A=
+			_key=3D$key=0A=
+			while [ $_key -gt 1 ]; do=0A=
+				key_kitem=3D"${key_kitem#*[$IFS]}"=0A=
+				while case "$key_kitem" in=0A=
+					[$IFS]*) true;; *) false; esac=0A=
+				do=0A=
+					key_kitem=3D"${key_kitem#?}"=0A=
+				done=0A=
+				_key=3D$(( $_key - 1 ))=0A=
+			done=0A=
+		done=0A=
+		local dest_$(( $k + 1 ))=3D"$item"=0A=
+		i=3D$(( $i + 1 ))=0A=
+		k=3D$(( $i - 1 ))=0A=
+	done=0A=
+=0A=
+	# Print sorted results=0A=
+	i=3D1=0A=
+	while [ $i -le $nitems ]; do=0A=
+		eval echo \"\$dest_$i\"=0A=
+		i=3D$(( $i + 1 ))=0A=
+	done=0A=
+}=0A=
+=0A=
+#=0A=
 # wait_for_pids pid [pid ...]=0A=
 #	spins until none of the pids exist=0A=
 #=0A=
@@ -1524,19 +1640,20 @@ load_kld()=0A=
 	return 0=0A=
 }=0A=
 =0A=
-# ltr str src dst=0A=
+# ltr str src dst [var]=0A=
 #	Change every $src in $str to $dst.=0A=
 #	Useful when /usr is not yet mounted and we cannot use tr(1), sed(1) =
nor=0A=
-#	awk(1).=0A=
+#	awk(1). If var is non-NULL, set it to the result.=0A=
 ltr()=0A=
 {=0A=
-	local _str _src _dst _out _com=0A=
-	_str=3D$1=0A=
-	_src=3D$2=0A=
-	_dst=3D$3=0A=
+	local _str _src _dst _out _com _var=0A=
+	_str=3D"$1"=0A=
+	_src=3D"$2"=0A=
+	_dst=3D"$3"=0A=
+	_var=3D"$4"=0A=
 	_out=3D""=0A=
 =0A=
-	IFS=3D${_src}=0A=
+	local IFS=3D"${_src}"=0A=
 	for _com in ${_str}; do=0A=
 		if [ -z "${_out}" ]; then=0A=
 			_out=3D"${_com}"=0A=
@@ -1544,7 +1661,11 @@ ltr()=0A=
 			_out=3D"${_out}${_dst}${_com}"=0A=
 		fi=0A=
 	done=0A=
-	echo "${_out}"=0A=
+	if [ -n "${_var}" ]; then=0A=
+		setvar "${_var}" "${_out}"=0A=
+	else=0A=
+		echo "${_out}"=0A=
+	fi=0A=
 }=0A=
 =0A=
 # Creates a list of providers for GELI encryption.=0A=

------=_NextPart_000_06A0_01CF4CD9.6391AFA0--



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?069f01cf4d14$0ff03980$2fd0ac80$>