diff mbox series

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

Message ID 20230523172314.7826-11-jonatan.schlag@ipfire.org
State Changes Requested
Headers
Series [RFC,01/15] Remove ipsec interface creation from network startup |

Commit Message

Jonatan Schlag May 23, 2023, 5:23 p.m. UTC 
  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@ipfire.org>
---
 src/initscripts/system/network | 52 ++++++++++++++++++++--------------
 1 file changed, 31 insertions(+), 21 deletions(-)

Comments

Michael Tremer May 24, 2023, 9 a.m. UTC | #1 
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@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@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
>
diff mbox series

Patch

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