[PATCH] convert-dns-settings: Fix check to prevent doubble-adding the same server

[PATCH] convert-dns-settings: Fix check to prevent doubble-adding the same server

[Stefan Schantl]

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

Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
---
 src/scripts/convert-dns-settings | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/src/scripts/convert-dns-settings b/src/scripts/convert-dns-settings
index e9d4de86b..073356f50 100755
--- a/src/scripts/convert-dns-settings
+++ b/src/scripts/convert-dns-settings
@@ -43,17 +43,24 @@ main() {
 				for var in DNS1 DNS2; do
 					local server="${!var}"
 
-					# Check if the current server is allready part
-					# of the array.
-					for element in "${SERVERS[@]}"; do
-						[[ $element == $server ]] && continue
-					done
-
-					SERVERS+=($server)
+					# Check if the servers array is empty.
+					if [ ${#SERVERS[@]} -eq 0 ]; then
+						# Allways add the first found nameserver to the array.
+						SERVERS+=($server)
+					else
+						# Check if the current server is allready part ot the array.
+						if [[ ! "${SERVERS[@]}" =~ "${server}" ]]; then
+							# Add the server to the array.
+							SERVERS+=($server)
+						fi
+					fi
 				done
 
 				# Remove DNS1 and DNS2 settings from profile file.
-				sed -i "/^DNS[12]?=/d" $file
+				sed -i "/^DNS[12]=/d" $file
+
+				# Unset the local variables for the next round.
+				unset DNS1 DNS2
 			done
 
 		elif [ -s "/var/ipfire/dns/settings" ]; then
-- 
2.25.0

Re: [PATCH] convert-dns-settings: Fix check to prevent doubble-adding the same server

[Michael Tremer]

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

Hi,

I am not sure if I agree with this patch, yet...

> On 28 Jan 2020, at 15:14, Stefan Schantl <stefan.schantl(a)ipfire.org> wrote:
> 
> Signed-off-by: Stefan Schantl <stefan.schantl(a)ipfire.org>
> ---
> src/scripts/convert-dns-settings | 23 +++++++++++++++--------
> 1 file changed, 15 insertions(+), 8 deletions(-)
> 
> diff --git a/src/scripts/convert-dns-settings b/src/scripts/convert-dns-settings
> index e9d4de86b..073356f50 100755
> --- a/src/scripts/convert-dns-settings
> +++ b/src/scripts/convert-dns-settings
> @@ -43,17 +43,24 @@ main() {
> 				for var in DNS1 DNS2; do
> 					local server="${!var}"
> 
> -					# Check if the current server is allready part
> -					# of the array.
> -					for element in "${SERVERS[@]}"; do
> -						[[ $element == $server ]] && continue
> -					done
> -
> -					SERVERS+=($server)
> +					# Check if the servers array is empty.
> +					if [ ${#SERVERS[@]} -eq 0 ]; then
> +						# Allways add the first found nameserver to the array.
> +						SERVERS+=($server)

Please quote things in shell and use curly brackets for variables. It should read: SERVERS+=( “${server}” )

> +					else
> +						# Check if the current server is allready part ot the array.
> +						if [[ ! "${SERVERS[@]}" =~ "${server}" ]]; then

Why would this not work when the array is empty? i.e. why do we need to check above?

> +							# Add the server to the array.
> +							SERVERS+=($server)

Same.

> +						fi
> +					fi
> 				done
> 
> 				# Remove DNS1 and DNS2 settings from profile file.
> -				sed -i "/^DNS[12]?=/d" $file
> +				sed -i "/^DNS[12]=/d" $file

Again missing quotes, but why is the DNS=Automatic or DNS=Manual settings not being removed any more?

The question mark was intentional.

> +
> +				# Unset the local variables for the next round.
> +				unset DNS1 DNS2
> 			done
> 
> 		elif [ -s "/var/ipfire/dns/settings" ]; then
> —
> 2.25.0
> 


-Michael