From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: network@lists.ipfire.org
Subject:
 Re: [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit
Date: Sun, 13 Aug 2017 13:10:07 +0100
Message-ID: <1502626207.14476.2.camel@ipfire.org>
In-Reply-To: <1502533349-13935-1-git-send-email-jonatan.schlag@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============9058549894642107238=="
List-Id: <network.lists.ipfire.org>

--===============9058549894642107238==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

Hi,

this patchset looks fine, but I think the solution is a little bit too
complicated.

Could we not let hook_parse_cmdline() remain untouched and just let it parse =
all
inputs? We need to parse all arguments anyways before we can make a decision.
We could then check for any conflicts later in hook_edit().

That looks easier to me since:

a) the format of hook_parse_cmdline() would be the same throughout all zones,
ports and configs

b) we do not need to carry around the ID variable. It will be empty any ways
when we are creating a new configuration and then become difficult to handle.

Your thoughts?

Best,
-Michael

On Sat, 2017-08-12 at 12:22 +0200, Jonatan Schlag wrote:
> We now check if we get the config, which we edit in this moment and when we
> continue.
>=20
> Fixes: #11451
>=20
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/functions/functions.zone | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>=20
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index 1eb492f..941314d 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -1155,12 +1155,13 @@ zone_config_check_same_setting() {
>  	# with the same setting is already configured for this zone.
>  	# Returns True when yes and False when no.
> =20
> -	assert [ $# -eq 4 ]
> +	assert [ $# -eq 5 ]
> =20
>  	local zone=3D${1}
>  	local hook=3D${2}
> -	local key=3D${3}
> -	local value=3D${4}
> +	local id=3D${3}
> +	local key=3D${4}
> +	local value=3D${5}
> =20
>  	# The key should be local for this function
>  	local ${key}
> @@ -1171,6 +1172,12 @@ zone_config_check_same_setting() {
>  		if  [[ $(zone_config_get_hook "${zone}" "${config}") !=3D
> ${hook} ]]; then
>  			continue
>  		fi
> +
> +		# Check if we get the config we want to create or edit (we
> must ignore this config on edit)
> +		if [[ "${hook}.${id}" =3D "${config}" ]]; then
> +			continue
> +		fi
> +
>  		# Get the value of the key for a given function
>  		zone_config_settings_read "${zone}" "${config}" \
>                  --ignore-superfluous-settings "${key}"

--===============9058549894642107238==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KCmlRSXpCQUFCQ2dBZEZpRUU1L3JXNWwzR0dl
Mnlwa3R4Z0hudy8yK1FDUWNGQWxtUVFaOEFDZ2tRZ0hudy8yK1EKQ1FjRjZ4QUFoeVJ4blQ0MEZO
bW5RNlFaM3lyRlB4aVpNNmoyRVE3M2FkZGxDMEhEYms4OFgrU1hYU21vZnRheAp5OGVHZWh3T1dm
LzN5TGVnTWMzQXlWN083NkdjSHRhb3E5VEtna0Myam9XOS9FYUoxbzBFcFZjRnV4SWpyU0R0Cld0
aXp1dlZHbDk0UXo5VTA4c3g4TjhyZ2pyZllBQmx4dUEyaDZ6K2Z1MUl1VWJ3UDVYQURnYkk4VmtI
NHNia3gKcmF3dWo4cUxFZ1RtbUFqWnlQYTc5dUwzeGdYeXA4RTNXTTZFMGJvWlhzT2FWcFRGNFNw
ZU1YYjZjMUY0c2FZRAovNkltcVZ5SktqZXdadHpVZSt2d2xsWVk0NTY1bXJIb1pURC9WWDNlNmtp
ZC80Z2kxSUcvWi9jRGNrbTFTUHYvCnZKbzRkTkUwU2ZoR1FuU1R4elZVazhzU0lseHk5UG9yekZC
WXk5enJaS2l3YVdtdE8reDY0UkxNWEcwZXA3clIKMy93dUFBZDVnU2x6ZUM5bG42Mkcrc3JTeUl3
UnpnM3VNWXJpQmkyZDR3TEFMZnE2SnI1ekdhQzM2ZVZkTWhFQgptZjR1MkdlYk5VMFdha3Jhd01l
TnYzczVPTklBWDdnUCtURTZaRWxFZ0E2TEVFMS90UHY5dVdHaWdJVGxNZzNuClBBek9YM3kxakpH
bDFKZW1qRGREZlZZK2dXOGd4WllGaGFjTkhyUHNPVk1GemMxTGJsZGZXbG1vY2l5UWlCUlQKVHRq
WitidXpva0QrSVRWWFZWaGh4c3NRUWhjTGR0YnA5b3M3akhhS0lpSVZXRDZneFZPUnJmZmcwMHNm
ZVl5OApoRXhKd1hQLzc1dTJ3a0NKak8wZER4MzBWSVB1cjd2SlVSc2tOdysvQ29pUmNoY1RESms9
Cj1XWUozCi0tLS0tRU5EIFBHUCBTSUdOQVRVUkUtLS0tLQo=

--===============9058549894642107238==--