Re: [PATCH 2/5] functions.network: Add network_zone_exists

[Jonatan Schlag <jonatan.schlag@ipfire.org>]

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

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;hb=6d501c05583a4efa513ff4b04a48ef41d5e8170e#l147
> 

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=9dd5205e5a1cb60c4a0bb003ec2a8348848822f9;hb=refs/heads/next#l22
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(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