Hi,

I think this patch confuses a few things that we should not confuse.

HID = Human IDs - readable by a human - that's it; and IDs which are readable by
a machine and easier to handle - therefore faster code

On Sun, 2018-01-28 at 15:25 +0000, Jonatan Schlag wrote:
> If we get an id insted of a hid we failed beacuse
> we passed the wrong variable.
> 
> I also added some more comments.
> 
> Fixes: #11453
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/functions/functions.zone | 11 +++++++----
>  1 file changed, 7 insertions(+), 4 deletions(-)
> 
> diff --git a/src/functions/functions.zone b/src/functions/functions.zone
> index f724f50..2d3d2c7 100644
> --- a/src/functions/functions.zone
> +++ b/src/functions/functions.zone
> @@ -527,7 +527,8 @@ zone_config() {
>  			zone_config_list "${zone}" "$@"
>  			;;
>  		*)
> -			# usually ${1} is a valid hid
> +			# usually ${cmd} is valid hid
> +			# We get ${hid} ${cmd} $@ from the cli
>  			local hid=${cmd}
>  			local cmd=${1}
>  			shift 1
> @@ -538,11 +539,11 @@ zone_config() {
>  			if isset id && [[ ${cmd} == "edit" ]]; then
>  				 zone_config_edit "${zone}" "${id}" "$@"
>  
> -			# If we didn't get a valid hid we check if we got a
> valid id
> +			# If we didn't get a valid hid we check if we got a
> valid id (saved in ${hid})
>  			else
> -				if zone_config_id_is_valid ${zone} ${id} &&
> [[ ${cmd} == "edit" ]]; then
> +				if zone_config_id_is_valid ${zone} ${hid} &&
> [[ ${cmd} == "edit" ]]; then

The function zone_config_id_is_valid should never take a HID. If it does its
name is wrong.

>  					shift 1
> -					zone_config_edit "${zone}" "${id}"
> "$@"
> +					zone_config_edit "${zone}" "${hid}"
> "$@"

The internal functions should never handle HIDs.

>  				else
>  					# in ${hid} is saved the command
> after network zone ${zone} config
>  					error "Unrecognized argument: ${hid}"
> @@ -1171,6 +1172,8 @@ zone_config_id_is_valid() {
>  	local zone=${1}
>  	local id=${2}
>  
> +	log DEBUG "Checking if id: ${id} is valid for zone: ${zone}"
> +
>  	local zone_path="${NETWORK_ZONES_DIR}/${zone}"
>  
>  	[ -f ${zone_path}/configs/*.${id} ];

-Michael