Re: [PATCH 1/5] zone: zone_config_check_same_setting check for config to edit

[Michael Tremer <michael.tremer@ipfire.org>]

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

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.
> 
> Fixes: #11451
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/functions/functions.zone | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
> 
> 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.
>  
> -	assert [ $# -eq 4 ]
> +	assert [ $# -eq 5 ]
>  
>  	local zone=${1}
>  	local hook=${2}
> -	local key=${3}
> -	local value=${4}
> +	local id=${3}
> +	local key=${4}
> +	local value=${5}
>  
>  	# 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}") !=
> ${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}" = "${config}" ]]; then
> +			continue
> +		fi
> +
>  		# Get the value of the key for a given function
>  		zone_config_settings_read "${zone}" "${config}" \
>                  --ignore-superfluous-settings "${key}"

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]