Hello,

We really need to go through the entire code base and come up with a single way how to check whether a zone exists or not.

In your proposed patch, you have different ways to check whether a zone exists or not. One is to check if there is a device (e.g. GREEN_DEV), and sometimes you are checking CONFIG_TYPE.

I suppose you got your inspiration from here:

  https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=config/cfgroot/header.pl;hb=6d501c05583a4efa513ff4b04a48ef41d5e8170e#l147

Should we not go and redefine CONFIG_TYPE=1 as “has a green network” to make it consistent?

I would also suggest to keep the function name short and rename it to “zone_exists”. Since the function is in “networks”, I think it is pretty clear what this function would do. There is litte chance this name would clash with anything else.

> On 2 Mar 2024, at 11:09, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
> 
> As our Network is quite static a case does the trick
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/initscripts/networking/functions.network | 22 ++++++++++++++++++++
> 1 file changed, 22 insertions(+)
> 
> diff --git a/src/initscripts/networking/functions.network b/src/initscripts/networking/functions.network
> index dedbb6f7f..ad0d3f36a 100644
> --- a/src/initscripts/networking/functions.network
> +++ b/src/initscripts/networking/functions.network
> @@ -289,3 +289,25 @@ qmi_assign_address() {
> # Change the MAC address
> ip link set "${intf}" address "${address}"
> }
> +
> +network_zone_exists(){
> + local zone="${1}"
> +
> + case "${zone}" in
> + "blue")
> + [ "${CONFIG_TYPE}" = "3" ] || [ "${CONFIG_TYPE}" = "4" ]
> + ;;
> + "green")
> + [ -n "${GREEN_DEV}" ] && [ -v "GREEN_DEV" ]

What is test -v doing? I can’t even find it on the man page.

> + ;;
> + "orange")
> + [ "${CONFIG_TYPE}" = "2" ] || [ "${CONFIG_TYPE}" = "4" ]
> + ;;
> + "red")
> + return ${EXIT_TRUE}
> + ;;
> + *)
> + return ${EXIT_FALSE}
> + ;;
> + esac
> +}
> -- 
> 2.39.2
> 

-Michael