From owner-freebsd-rc@FreeBSD.ORG Mon Mar 31 19:04:49 2014 Return-Path: Delivered-To: rc@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id ADC01E68; Mon, 31 Mar 2014 19:04:49 +0000 (UTC) Received: from mx1.fisglobal.com (mx1.fisglobal.com [199.200.24.190]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (Client CN "mx1.fisglobal.com", Issuer "VeriSign Class 3 Secure Server CA - G3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 70C748F9; Mon, 31 Mar 2014 19:04:48 +0000 (UTC) Received: from smarthost.fisglobal.com ([10.132.206.192]) by ltcfislmsgpa07.fnfis.com (8.14.5/8.14.5) with ESMTP id s2VJ4kVt001301 (version=TLSv1/SSLv3 cipher=AES128-SHA bits=128 verify=NOT); Mon, 31 Mar 2014 14:04:46 -0500 Received: from THEMADHATTER (10.242.181.54) by smarthost.fisglobal.com (10.132.206.192) with Microsoft SMTP Server id 14.3.174.1; Mon, 31 Mar 2014 14:04:43 -0500 From: Sender: Devin Teske To: "'Hiroki Sato'" , References: <04c901cf4c5d$a6a4a030$f3ede090$@FreeBSD.org> <0EBE3981-DC85-414D-85B8-7638F172040A@dataix.net> <04f701cf4c85$d1929680$74b7c380$@FreeBSD.org> <20140331.121757.1100840815853109946.hrs@allbsd.org> In-Reply-To: <20140331.121757.1100840815853109946.hrs@allbsd.org> Subject: RE: network.subr _aliasN handling Date: Mon, 31 Mar 2014 12:04:35 -0700 Message-ID: <069f01cf4d14$0ff03980$2fd0ac80$@FreeBSD.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="----=_NextPart_000_06A0_01CF4CD9.6391AFA0" X-Mailer: Microsoft Outlook 15.0 Thread-Index: AQLl7DYKykKh8p6TvLmmwe5q3zMB9AI3LylbArF10lEChFGO0piS3SKQ Content-Language: en-us X-Proofpoint-Virus-Version: vendor=fsecure engine=2.50.10432:5.11.87, 1.0.14, 0.0.0000 definitions=2014-03-31_03:2014-03-31,2014-03-31,1970-01-01 signatures=0 Cc: jhellenthal@dataix.net, rc@FreeBSD.org X-BeenThere: freebsd-rc@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: "Discussion related to /etc/rc.d design and implementation." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Mar 2014 19:04:49 -0000 ------=_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 > > 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--