From mboxrd@z Thu Jan 1 00:00:00 1970 From: Michael Tremer To: network@lists.ipfire.org Subject: Re: [PATCH 1/8] zone: add config hid functions Date: Tue, 25 Jul 2017 23:22:31 +0100 Message-ID: <1501021351.20312.31.camel@ipfire.org> In-Reply-To: <1500998833-10543-2-git-send-email-jonatan.schlag@ipfire.org> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="===============8909690172413406823==" List-Id: --===============8909690172413406823== Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Hi, On Tue, 2017-07-25 at 18:07 +0200, Jonatan Schlag wrote: > The following patch adds the basic functions which are we need to work with > hids. >=20 > Fixes: #11406 (partially) >=20 > Signed-off-by: Jonatan Schlag > --- > =C2=A0src/functions/functions.zone | 86 > ++++++++++++++++++++++++++++++++++++++++++++ > =C2=A01 file changed, 86 insertions(+) >=20 > diff --git a/src/functions/functions.zone b/src/functions/functions.zone > index bba1705..28a22fe 100644 > --- a/src/functions/functions.zone > +++ b/src/functions/functions.zone > @@ -656,6 +656,75 @@ zone_config_list_ids() { > =C2=A0 echo ${ids} > =C2=A0} > =C2=A0 > +# List all hids of a zone > +zone_config_list_hids() { > + assert [ $# -eq 1 ] > + local zone=3D${1} > + > + local config > + local hids > + > + for config in $(zone_configs_list ${zone}); do > + # Get hook from config > + hook=3D"$(zone_config_get_hook "${zone}" "${config}")" > + # Append hids to the list > + list_append hids "$(zone_config_get_hid "${zone}" > "${config}")" > + done > + > + echo ${hids} > +} In this function, you are using a hook variable that is never initalised nor used. So I would say that can just be removed. And unless you insist that the function returns its result in one line, you could just let "zone_config_get_hid" write to stdout and remove the hids variable and list_append entirely. That will make this function way faster si= nce it doesn't need to fork as many sub-shells. > + > +# get the hid from a given config > +zone_config_get_hid() { > + assert [ $# -eq 2 ] > + local zone=3D${1} > + local config=3D${2} > + > + local hook > + > + hook=3D"$(zone_config_get_hook "${zone}" "${config}")" > + echo "$(hook_exec "config" "${hook}" "hid" "${zone}" "${config}")" > +} Same here: echo "$(some function)" could just be: some function Same result. But doesn't call echo or the sub-shell to call the function. > + > +# Checks if a hid is valid for a given zone > +zone_config_hid_is_valid() { > + assert [ $# -eq 2] > + local zone=3D${1} > + local hid=3D${2} > + > + local _hid > + for _hid in $(zone_config_list_hids "${zone}"); do > + if [[ ${_hid} =3D=3D ${hid} ]]; then > + return ${EXIT_TRUE} > + fi > + done > + > + return ${EXIT_FALSE} > + > +} Empty line before the closing bracket of the function. And the check for equality in POSIX shell is just "=3D" and not "=3D=3D". > + > +# This function converts a hid to a id > +zone_config_convert_hid_to_id() { > + assert [ $# -eq 2 ] > + local zone=3D${1} > + local hid=3D${2} > + > + local hook > + local config > + local id > + > + for config in $(zone_configs_list ${zone}); do > + # Get hook from config > + hook=3D"$(zone_config_get_hook "${zone}" "${config}")" > + if [[ "$(hook_exec "config" "${hook}" "hid" "${zone}" > "${config}")" =3D=3D "${hid}" ]]; then > + id=3D$(config_get_id_from_config "${config}") > + break > + fi > + done > + > + echo ${id} > +} Same as above. You could just let "config_get_id_from_config" print the ID and then break or return. The ID variable is not needed. > + > =C2=A0zone_show() { > =C2=A0 local zone=3D${1} > =C2=A0 > @@ -1114,6 +1183,23 @@ zone_config_id_is_valid() { > =C2=A0 [ -f ${zone_path}/configs/*.${id} ]; > =C2=A0} > =C2=A0 > +# This function checks if a given hid is valid for a zone > +# Return True when yes and false when no > +zone_config_hid_is_valid() { > + assert [ $# -eq 2 ] > + local zone=3D${1} > + local hid=3D${2} > + > + local _hid > + for _hid in $(zone_config_list_hids ${zone}); do > + if [[ ${_hid} =3D=3D ${hid} ]]; then > + return ${EXIT_TRUE} > + fi > + done > + > + return ${EXIT_FALSE} > +} This function is a duplicate. I also think it should be called "zone_config_hid_exists" or so since we technically don't know what a valid HID looks like. Every hook can have something different here. > + > =C2=A0zone_config_get_hook_from_id() { > =C2=A0 # Returns the hook for a given id > =C2=A0 assert [ $# -eq 2 ] Best, -Michael --===============8909690172413406823== Content-Type: application/pgp-signature Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="signature.asc" MIME-Version: 1.0 LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjIKCmlRSWNCQUFC Q2dBR0JRSlpkOFNuQUFvSkVJQjU4UDl2a0FrSDJ2UVAvMjUxZXZqZlpWUGJtL080YUxZcGJjWWsK allVSUpqMHRxZVVSL1pDS1lzN2o2RkMwWGJRQ0UzazlsbzJMVldJRkczZXVTZllaZ1pDTzA1Wlpn bU40dWdSUAp1MzkyNktBMXZmdUJvbk5Gam0vdzZHNnZ1MTc0VjViMVZCcVN4YjlPNmZpY0k4ZXho QUxVMEdYUG9kUGNrMzFOClFPck8yR3ZUN2tkNUhlRG1GQnNlOWV0cWpaUER5cEdTdy93WnJkaTRQ R3A2U3d2bS8rYXlaRXI0ZDRzR1FHZDgKY0RaSEtuWXh0YndwNlhwQm54eVBIYUM2TDVKclZaRWVE eC9veXZyMG1iRUhUdU9iVC9tRFVpUWNBekY0QjFteQpOalJqcFkzcU9iSGIrbUVVNUtud3V2SEJX SlgvVzA5Qld1cnV5eG5lZkZSeXJ0bmdPQUJLeVNsWFFNSUowcFFjCnBrV21yMkZWRVNRamo5WEVR d29WcU1zaXJvNmNBSDhOSDZnRnBhT2NVaTdDd0ExWFFya2hOYXZmNzF6TWZJemQKeW5HMlNIZHdL d0JoQUw4bklvVGIvM0xTN1JjdzRsaXB3NkNGbWROZ3pwY3lVSm8xUGhUdjlxalNOK2UxbmkzSwo3 YlQ2QTRlb0ZGNHhHcVdVQ2VJS2pVdE1SUE85YTFZbGZQd0N2cFkyQVBITExzT08vV3I1WEp4VnZV MktKaWk3CmFnSUNzSWVQZnNFWjlSeG9NWXhKRTV4cDl0bm0rKy9XandjaG5JbVRVQi9NYXRyZU15 ZS9XbUZJZjJlbmdkTDYKQkFNUzBQbVhyWFhGN2haNHZKS3NsWXhxVFgyMDBiamxkUTRpemM3NkNZ U3F4ZmRNbDV4bVhHTXJuaHRMSjZGMwpNTm0yNmNjdHhuNVI5WXBOWDdqQwo9NmladAotLS0tLUVO RCBQR1AgU0lHTkFUVVJFLS0tLS0K --===============8909690172413406823==--