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

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

[Jonatan Schlag]

[-- 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
> > 
> 

[PATCH 12/21] initscript functions: add readhash

[Jonatan Schlag]

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

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}"
+}
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