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

Hi,

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

On 28 Jan 2020, at 15:14, Stefan Schantl stefan.schantl@ipfire.org wrote:

Signed-off-by: Stefan Schantl stefan.schantl@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