public inbox for network@lists.ipfire.org
 help / color / mirror / Atom feed
* [PATCH] Fix parsing of hids and ids
@ 2018-01-28 15:25 Jonatan Schlag
  2018-02-06  0:57 ` Michael Tremer
  0 siblings, 1 reply; 2+ messages in thread
From: Jonatan Schlag @ 2018-01-28 15:25 UTC (permalink / raw)
  To: network

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

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
 					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} ];
-- 
2.6.3


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH] Fix parsing of hids and ids
  2018-01-28 15:25 [PATCH] Fix parsing of hids and ids Jonatan Schlag
@ 2018-02-06  0:57 ` Michael Tremer
  0 siblings, 0 replies; 2+ messages in thread
From: Michael Tremer @ 2018-02-06  0:57 UTC (permalink / raw)
  To: network

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

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

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2018-02-06  0:57 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-01-28 15:25 [PATCH] Fix parsing of hids and ids Jonatan Schlag
2018-02-06  0:57 ` Michael Tremer

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox