From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH 2/5] functions.network: Add network_zone_exists
Date: Tue, 12 Mar 2024 09:59:05 +0000
Message-ID: <05526FFB-B7D3-4065-94C6-CBB1FDCA2C5F@ipfire.org>
In-Reply-To: <20240302110928.10377-2-jonatan.schlag@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============8454192695291207405=="
List-Id: <development.lists.ipfire.org>

--===============8454192695291207405==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hello,

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

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

I suppose you got your inspiration from here:

  https://git.ipfire.org/?p=3Dipfire-2.x.git;a=3Dblob;f=3Dconfig/cfgroot/head=
er.pl;hb=3D6d501c05583a4efa513ff4b04a48ef41d5e8170e#l147

Should we not go and redefine CONFIG_TYPE=3D1 as =E2=80=9Chas a green network=
=E2=80=9D to make it consistent?

I would also suggest to keep the function name short and rename it to =E2=80=
=9Czone_exists=E2=80=9D. Since the function is in =E2=80=9Cnetworks=E2=80=9D,=
 I think it is pretty clear what this function would do. There is litte chanc=
e this name would clash with anything else.

> On 2 Mar 2024, at 11:09, Jonatan Schlag <jonatan.schlag(a)ipfire.org> wrote:
>=20
> As our Network is quite static a case does the trick
>=20
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> src/initscripts/networking/functions.network | 22 ++++++++++++++++++++
> 1 file changed, 22 insertions(+)
>=20
> 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=3D"${1}"
> +
> + case "${zone}" in
> + "blue")
> + [ "${CONFIG_TYPE}" =3D "3" ] || [ "${CONFIG_TYPE}" =3D "4" ]
> + ;;
> + "green")
> + [ -n "${GREEN_DEV}" ] && [ -v "GREEN_DEV" ]

What is test -v doing? I can=E2=80=99t even find it on the man page.

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

-Michael


--===============8454192695291207405==--