convert-dns-settings: Fix check to prevent doubble-adding the same server
Commit Message
Signed-off-by: Stefan Schantl <stefan.schantl@ipfire.org>
---
src/scripts/convert-dns-settings | 23 +++++++++++++++--------
1 file changed, 15 insertions(+), 8 deletions(-)
Comments
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
@@ -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