Am 2024-03-12 10:59, schrieb Michael Tremer:
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;h...
No, that is precisely the code copied from the network script.
Should we not go and redefine CONFIG_TYPE=1 as “has a green network” to make it consistent?
According to here, there always exists a green network: https://git.ipfire.org/?p=ipfire-2.x.git;a=blob;f=src/setup/networking.c;h=9... So we could just return True.
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.
Just for the records
"I would prefer to give those function a “network_” prefix so that it becomes clear where this function is from and ideally shorten “is used” to just “have”. Like so: network_have_BLUE() and network_have_ORANGE(). " - "Re: [Patch RFC 04/15] network initscripts: check if the zone in the current config exists"
An ever moving target is hard to hit :-)
Jonatan
On 2 Mar 2024, at 11:09, Jonatan Schlag jonatan.schlag@ipfire.org wrote:
As our Network is quite static a case does the trick
Signed-off-by: Jonatan Schlag jonatan.schlag@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