Re: [Patch RFC 10/15] network startup: Refactor how cmd args are processed

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 3625 bytes --]

Phew, this has already been a rubbish script.

One option could have been using “printf -v” to get rid of the eval call.

Otherwise, I fail to see the improvement here? It is just a different way to write this?!

Since you have introduced new functions to check whether a certain zone is available, why no just call those functions like this:

  if network_has_BLUE; then
    /etc/init.d/networking/local BLUE start
  fi

-Michael

> On 23 May 2023, at 18:23, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
> 
> This avoids eval and all other sorts of things. We also now exit when we
> get an invalid zone name.
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/initscripts/system/network | 52 ++++++++++++++++++++--------------
> 1 file changed, 31 insertions(+), 21 deletions(-)
> 
> diff --git a/src/initscripts/system/network b/src/initscripts/system/network
> index 06240f53c..008fbbe2b 100644
> --- a/src/initscripts/system/network
> +++ b/src/initscripts/system/network
> @@ -31,41 +31,51 @@ if ! [[ "${DO}" == "start" ||  "${DO}" == "restart" || "${DO}" == "stop" ]]; the
> exit 1
> fi
> 
> -if [ -n "${1}" ]; then
> - ALL=0
> - for i in green red blue orange; do
> - eval "${i}=0"
> - done
> -else
> - ALL=1
> - for i in green red blue orange; do
> - eval "${i}=1"
> - done
> +declare -A ZONE_ACTION
> +
> +ZONE_ACTION[blue]=false
> +ZONE_ACTION[green]=false
> +ZONE_ACTION[orange]=false
> +ZONE_ACTION[red]=false
> +
> +if [ $# -eq 0 ]; then
> + ZONE_ACTION[blue]=true
> + ZONE_ACTION[green]=true
> + ZONE_ACTION[orange]=true
> + ZONE_ACTION[red]=true
> fi
> 
> -while [ ! $# = 0 ]; do
> +while [ $# -ne 0 ]; do
> + ZONE_VALID=false
> for i in green red blue orange; do
> if [ "${i}" == "${1}" ]; then
> - eval "${i}=1"
> + ZONE_ACTION[${i}]=true
> + ZONE_VALID=true
> shift
> + break
> fi
> done
> +
> + if ! ${ZONE_VALID}; then
> + echo "'${1}' is not a valid zone. Cannot go on."
> + exit 1
> + fi
> done
> 
> case "${DO}" in
> start)
> # Starting interfaces...
> # GREEN
> - [ "$green" == "1" ] && /etc/rc.d/init.d/networking/green start
> + ${ZONE_ACTION[green]} && /etc/rc.d/init.d/networking/green start
> 
> # BLUE
> - [ "$blue" == "1" ] && /etc/rc.d/init.d/networking/blue start
> + ${ZONE_ACTION[blue]} && /etc/rc.d/init.d/networking/blue start
> 
> # ORANGE
> - [ "$orange" == "1" ] && /etc/rc.d/init.d/networking/orange start
> + ${ZONE_ACTION[orange]} && /etc/rc.d/init.d/networking/orange start
> 
> # RED
> - [ "$red" == "1" ] && /etc/rc.d/init.d/networking/red start
> + ${ZONE_ACTION[red]} && /etc/rc.d/init.d/networking/red start
> 
> boot_mesg "Mounting network file systems..."
> mount -a -O _netdev
> @@ -79,16 +89,16 @@ case "${DO}" in
> 
> # Stopping interfaces...
> # GREEN
> - [ "$green" == "1" ] && /etc/rc.d/init.d/networking/green stop
> + ${ZONE_ACTION[green]} && /etc/rc.d/init.d/networking/green stop
> 
> # BLUE
> - [ "$blue" == "1" ] && /etc/rc.d/init.d/networking/blue stop
> + ${ZONE_ACTION[blue]} && /etc/rc.d/init.d/networking/blue stop
> 
> # ORANGE
> - [ "$orange" == "1" ] && /etc/rc.d/init.d/networking/orange stop
> + ${ZONE_ACTION[orange]} && /etc/rc.d/init.d/networking/orange stop
> 
> # RED
> - if [ "$red" == "1" ]; then
> + if ${ZONE_ACTION[red]}; then
> /etc/rc.d/init.d/networking/red stop
> fi
> 
> @@ -97,7 +107,7 @@ case "${DO}" in
> 
> restart)
> for i in green red blue orange; do
> - if [ "${!i}" == "1" ]; then
> + if {ZONE_ACTION[${i}]}; then
> ARGS+=" ${i}"
> fi
> done
> -- 
> 2.30.2
>