[Openvpn-devel,v3] move macOS dns-updown common code into functions

Message ID 20250711100853.242102-1-frank@lichtenheld.com
State New
Headers show
Series [Openvpn-devel,v3] move macOS dns-updown common code into functions | expand

Commit Message

Frank Lichtenheld July 11, 2025, 10:08 a.m. UTC
From: Heiko Hund <heiko@ist.eigentlich.net>

Change-Id: Id6f70237c7205063b001528a40391678b0d093ac
Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
---

This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.

Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1074
This mail reflects revision 3 of this Change.

Acked-by according to Gerrit (reflected above):
Frank Lichtenheld <frank@lichtenheld.com>

Comments

Gert Doering July 15, 2025, 11:22 a.m. UTC | #1
Looks reasonable, and Arne & Heiko confirm that it works.  So I've not
done any testing, just skimmed over the change to see what it touches.

Your patch has been applied to the master branch.

commit 022cd3e7fe19e013709ecd3104aaf93b6e6abe89
Author: Heiko Hund
Date:   Fri Jul 11 12:08:53 2025 +0200

     move macOS dns-updown common code into functions

     Signed-off-by: Heiko Hund <heiko@ist.eigentlich.net>
     Acked-by: Frank Lichtenheld <frank@lichtenheld.com>
     Message-Id: <20250711100853.242102-1-frank@lichtenheld.com>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg32105.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering

Patch

diff --git a/distro/dns-scripts/macos-dns-updown.sh b/distro/dns-scripts/macos-dns-updown.sh
index f0640ee..56f1009 100644
--- a/distro/dns-scripts/macos-dns-updown.sh
+++ b/distro/dns-scripts/macos-dns-updown.sh
@@ -104,7 +104,7 @@ 
 
         n=$((n+1))
     done
-    return $n
+    echo $n
 }
 
 function get_search_domains {
@@ -157,41 +157,23 @@ 
     echo -e "${cmds}" | /usr/sbin/scutil
 }
 
-function set_dns {
-    find_compat_profile
-    local n=$?
-
+function addresses_string {
+    local n=$1
     local i=1
-    local addrs=""
+    local addresses=""
     while :; do
         local addr_var=dns_server_${n}_address_${i}
         local addr="${!addr_var}"
         [ -n "$addr" ] || break
-
-        local port_var=dns_server_${n}_port_${i}
-        if [ -n "${!port_var}" ]; then
-            if [[ "$addr" =~ : ]]; then
-                addr="[$addr]"
-            fi
-            addrs+="${addr}:${!port_var}${sni} "
-        else
-            addrs+="${addr}${sni} "
-        fi
+        addresses+="${addr} "
         i=$((i+1))
     done
+    echo "$addresses"
+}
 
-    i=1
-    local match_domains=""
-    while :; do
-        domain_var=dns_server_${n}_resolve_domain_${i}
-        [ -n "${!domain_var}" ] || break
-        # Add as match domain, if it doesn't already exist
-        [[ "$match_domains" =~ (^| )${!domain_var}( |$) ]] \
-            || match_domains+="${!domain_var} "
-        i=$((i+1))
-    done
-
-    i=1
+function search_domains_string {
+    local n=$1
+    local i=1
     local search_domains=""
     while :; do
         domain_var=dns_search_domain_${i}
@@ -201,11 +183,34 @@ 
             || search_domains+="${!domain_var} "
         i=$((i+1))
     done
+    echo "$search_domains"
+}
+
+function match_domains_string {
+    local n=$1
+    local i=1
+    local match_domains=""
+    while :; do
+        domain_var=dns_server_${n}_resolve_domain_${i}
+        [ -n "${!domain_var}" ] || break
+        # Add as match domain, if it doesn't already exist
+        [[ "$match_domains" =~ (^| )${!domain_var}( |$) ]] \
+            || match_domains+="${!domain_var} "
+        i=$((i+1))
+    done
+    echo "$match_domains"
+}
+
+function set_dns {
+    local n="$(find_compat_profile)"
+    local addresses="$(addresses_string $n)"
+    local search_domains="$(search_domains_string $n)"
+    local match_domains="$(match_domains_string $n)"
 
     if [ -n "$match_domains" ]; then
         local cmds=""
         cmds+="d.init\n"
-        cmds+="d.add ServerAddresses * ${addrs}\n"
+        cmds+="d.add ServerAddresses * ${addresses}\n"
         cmds+="d.add SupplementalMatchDomains * ${match_domains}\n"
         cmds+="d.add SupplementalMatchDomainsNoSearch # 1\n"
         cmds+="add ${itf_dns_key}\n"
@@ -222,7 +227,7 @@ 
         cmds+="get $(primary_dns_key)\n"
         cmds+="set ${dns_backup_key}\n"
         cmds+="d.init\n"
-        cmds+="d.add ServerAddresses * ${addrs}\n"
+        cmds+="d.add ServerAddresses * ${addresses}\n"
         cmds+="d.add SearchDomains * ${search_domains}\n"
         cmds+="d.add SearchOrder # 5000\n"
         cmds+="set $(primary_dns_key)\n"
@@ -233,22 +238,12 @@ 
 }
 
 function unset_dns {
-    find_compat_profile
-    local n=$?
+    local n="$(find_compat_profile)"
+    local addresses="$(addresses_string $n)"
+    local search_domains="$(search_domains_string $n)"
+    local match_domains="$(match_domains_string $n)"
 
-    local i=1
-    local search_domains=""
-    while :; do
-        domain_var=dns_search_domain_${i}
-        [ -n "${!domain_var}" ] || break
-        # Add as search domain, if it doesn't already exist
-        [[ "$search_domains" =~ (^| )${!domain_var}( |$) ]] \
-            || search_domains+="${!domain_var} "
-        i=$((i+1))
-    done
-
-    domain_var=dns_server_${n}_resolve_domain_1
-    if [ -n "${!domain_var}" ]; then
+    if [ -n "$match_domains" ]; then
         echo "remove ${itf_dns_key}" | /usr/sbin/scutil
         unset_search_domains "$search_domains"
     else