Re: [PATCH 12/21] initscript functions: add readhash

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

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

Hi,

Am Freitag, dem 31.05.2024 um 10:53 +0100 schrieb Michael Tremer:
> Hello,
> 
> > On 20 May 2024, at 10:06, Jonatan Schlag
> > <jonatan.schlag(a)ipfire.org> wrote:
> > 
> > To avoid the usage of eval and to store the config in an key value
> > array, we introduce an new function. The tests only check if we
> > read the correct value to the correct variable.
> > 
> > One comment on the implementation as this has created some
> > headache:
> > 
> > > From
> > > https://www.gnu.org/software/bash/manual/bash.html#Bourne-Shell-Builtins
> > 
> > "When used in a function, declare makes each name local, as with
> > the local command, unless the -g option is used."
> > 
> > So we need to use -g here
> > 
> > Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> > ---
> > src/initscripts/system/functions               | 15 +++++++++++++++
> > tests/src/initscripts/system/functions/data/1  | 17
> > +++++++++++++++++
> > tests/src/initscripts/system/functions/test.sh | 16
> > ++++++++++++++++
> > 3 files changed, 48 insertions(+)
> > create mode 100644 tests/src/initscripts/system/functions/data/1
> > create mode 100755 tests/src/initscripts/system/functions/test.sh
> > 
> > diff --git a/src/initscripts/system/functions
> > b/src/initscripts/system/functions
> > index b610143ab..44ce999d3 100644
> > --- a/src/initscripts/system/functions
> > +++ b/src/initscripts/system/functions
> > @@ -891,3 +891,18 @@ volume_fs_type() {
> > 
> > stat -f --format="%T" ${1}
> > }
> > +
> > +readhash() {
> > + local array="${1}"
> > + local file="${2}"
> > +
> > + declare -A -g "${array}"
> > +
> > + local line
> > + while read -r line; do
> > + local key="${line%=*}"
> > + local val="${line#*=}"
> > +
> > + printf -v "${array}[${key}]" "%s" "${val}"
> > + done < "${file}"
> > +}
> 
> Okay, so here we are getting to the main bit.
> 
> I think this is nice and simply. The function creates the array
> regardless and would empty any content which is what we want.

Thank you.
> 
> You should however check if the file exists and can be read as the
> function would now not return any error. It would write some error
> message, but you cannot programmatically check that.

I will add code for that.
> 
> Should we have a matching writehash function?

I would add this function when we need it. Less dead code :-).

Jonatan
> 
> -Michael
> 
> > diff --git a/tests/src/initscripts/system/functions/data/1
> > b/tests/src/initscripts/system/functions/data/1
> > new file mode 100644
> > index 000000000..8aca9422b
> > --- /dev/null
> > +++ b/tests/src/initscripts/system/functions/data/1
> > @@ -0,0 +1,17 @@
> > +CONFIG_TYPE=3
> > +GREEN_DEV=green0
> > +GREEN_MACADDR=00:c0:08:8a:a0:47
> > +GREEN_DRIVER=r8175
> > +RED_DEV=red0
> > +RED_MACADDR=00:c0:08:8a:a0:56
> > +RED_DRIVER=r8283
> > +BLUE_DEV='blue0 net0'
> > +BLUE_MACADDR=bc:30:7d:58:6b:e3
> > +BLUE_DRIVER=rt2800
> > +RED_DHCP_HOSTNAME=ipfire
> > +RED_DHCP_FORCE_MTU=
> > +RED_ADDRESS=0.0.0.0
> > +RED_NETMASK=0.0.0.0
> > +RED_TYPE=PPPOE
> > +RED_NETADDRESS=0.0.0.0
> > +
> > diff --git a/tests/src/initscripts/system/functions/test.sh
> > b/tests/src/initscripts/system/functions/test.sh
> > new file mode 100755
> > index 000000000..ec502e199
> > --- /dev/null
> > +++ b/tests/src/initscripts/system/functions/test.sh
> > @@ -0,0 +1,16 @@
> > +#!/usr/bin/bash
> > +
> > +SCRIPT_PATH="$(dirname "$(readlink -f "$0")")"
> > +
> > +ROOT="$(readlink -f "${SCRIPT_PATH}/../../../../..")"
> > +
> > +. ${ROOT}/tests/lib.sh
> > +
> > +. ${ROOT}/src/initscripts/system/functions
> > +
> > +# read the date in
> > +readhash "CONFIG" "${SCRIPT_PATH}/data/1"
> > +
> > +# test if we read the correct data
> > +test_that_key_in_arry_has_value "CONFIG" "RED_DHCP_HOSTNAME"
> > "ipfire"
> > +test_that_key_in_arry_has_value "CONFIG" "BLUE_MACADDR"
> > "bc:30:7d:58:6b:e3"
> > -- 
> > 2.39.2
> > 
>