Fix parsing of hids and ids

Message ID 1517153148-6844-1-git-send-email-jonatan.schlag@ipfire.org
State New
Headers show
Series
  • Fix parsing of hids and ids
Related show

Commit Message

Jonatan Schlag Jan. 29, 2018, 2:25 a.m. UTC
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@ipfire.org>
---
 src/functions/functions.zone | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

Comments

Michael Tremer Feb. 6, 2018, 11:57 a.m. UTC | #1
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@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

Patch

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
 					shift 1
-					zone_config_edit "${zone}" "${id}" "$@"
+					zone_config_edit "${zone}" "${hid}" "$@"
 				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} ];