resolved: unify code for parsing dns server information

Let's use the same parser when parsing dns server information from
/etc/resolv.conf and our native configuration file.

Also, move all code that manages lists of dns servers to a single place.
resolved-dns-server.c

7 files changed, 110 insertions, 90 deletions

diff --git a/src/resolve/resolved-conf.c b/src/resolve/resolved-conf.c
index 9207719551..8b49d46a59 100644
--- a/src/resolve/resolved-conf.c
+++ b/src/resolve/resolved-conf.c
@@ -27,53 +27,65 @@
 #include "resolved-conf.h"
 #include "string-util.h"
 
-int manager_parse_dns_server(Manager *m, DnsServerType type, const char *string) {
-        DnsServer *first;
-        int r;
+int manager_add_dns_server_by_string(Manager *m, DnsServerType type, const char *word) {
+        union in_addr_union address;
+        DnsServer *first, *s;
+        int family, r;
 
         assert(m);
-        assert(string);
+        assert(word);
+
+        r = in_addr_from_string_auto(word, &family, &address);
+        if (r < 0)
+                return r;
 
         first = type == DNS_SERVER_FALLBACK ? m->fallback_dns_servers : m->dns_servers;
 
+        /* Filter out duplicates */
+        LIST_FOREACH(servers, s, first)
+                if (s->family == family && in_addr_equal(family, &s->address, &address))
+                        break;
+
+        if (s) {
+                /*
+                 * Drop the marker. This is used to find the servers
+                 * that ceased to exist, see
+                 * manager_mark_dns_servers() and
+                 * manager_flush_marked_dns_servers().
+                 */
+                s->marked = false;
+                return 0;
+        }
+
+        return dns_server_new(m, NULL, type, NULL, family, &address);
+}
+
+int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string) {
+        int r;
+
+        assert(m);
+        assert(string);
+
         for(;;) {
                 _cleanup_free_ char *word = NULL;
-                union in_addr_union addr;
-                bool found = false;
-                DnsServer *s;
-                int family;
 
                 r = extract_first_word(&string, &word, NULL, 0);
                 if (r < 0)
-                        return log_error_errno(r, "Failed to parse resolved dns server syntax \"%s\": %m", string);
+                        return r;
                 if (r == 0)
                         break;
 
-                r = in_addr_from_string_auto(word, &family, &addr);
+                r = manager_add_dns_server_by_string(m, type, word);
                 if (r < 0) {
-                        log_warning("Ignoring invalid DNS address '%s'", word);
+                        log_warning_errno(r, "Failed to add DNS server address '%s', ignoring.", word);
                         continue;
                 }
-
-                /* Filter out duplicates */
-                LIST_FOREACH(servers, s, first)
-                        if (s->family == family && in_addr_equal(family, &s->address, &addr)) {
-                                found = true;
-                                break;
-                        }
-
-                if (found)
-                        continue;
-
-                r = dns_server_new(m, NULL, type, NULL, family, &addr);
-                if (r < 0)
-                        return r;
         }
 
         return 0;
 }
 
-int config_parse_dnsv(
+int config_parse_dns_servers(
                 const char *unit,
                 const char *filename,
                 unsigned line,
@@ -98,7 +110,7 @@ int config_parse_dnsv(
                 manager_flush_dns_servers(m, ltype);
         else {
                 /* Otherwise, add to the list */
-                r = manager_parse_dns_server(m, ltype, rvalue);
+                r = manager_parse_dns_server_string_and_warn(m, ltype, rvalue);
                 if (r < 0) {
                         log_syntax(unit, LOG_ERR, filename, line, r, "Failed to parse DNS server string '%s'. Ignoring.", rvalue);
                         return 0;
diff --git a/src/resolve/resolved-conf.h b/src/resolve/resolved-conf.h
index b3dbea7b6b..170f3e94aa 100644
--- a/src/resolve/resolved-conf.h
+++ b/src/resolve/resolved-conf.h
@@ -26,7 +26,10 @@
 int manager_parse_dns_server(Manager *m, DnsServerType type, const char *string);
 int manager_parse_config_file(Manager *m);
 
+int manager_parse_dns_server_string_and_warn(Manager *m, DnsServerType type, const char *string);
+int manager_add_dns_server_by_string(Manager *m, DnsServerType type, const char *word);
+
 const struct ConfigPerfItem* resolved_gperf_lookup(const char *key, unsigned length);
 
-int config_parse_dnsv(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
+int config_parse_dns_servers(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
 int config_parse_support(const char *unit, const char *filename, unsigned line, const char *section, unsigned section_line, const char *lvalue, int ltype, const char *rvalue, void *data, void *userdata);
diff --git a/src/resolve/resolved-dns-server.c b/src/resolve/resolved-dns-server.c
index e803f635ab..3d6a6c92b3 100644
--- a/src/resolve/resolved-dns-server.c
+++ b/src/resolve/resolved-dns-server.c
@@ -161,3 +161,45 @@ const struct hash_ops dns_server_hash_ops = {
         .hash = dns_server_hash_func,
         .compare = dns_server_compare_func
 };
+
+void manager_flush_dns_servers(Manager *m, DnsServerType type) {
+        DnsServer **first, *s;
+
+        assert(m);
+
+        first = type == DNS_SERVER_FALLBACK ? &m->fallback_dns_servers : &m->dns_servers;
+
+        while (*first) {
+                s = *first;
+
+                LIST_REMOVE(servers, *first, s);
+                dns_server_unref(s);
+        }
+}
+
+void manager_flush_marked_dns_servers(Manager *m, DnsServerType type) {
+        DnsServer **first, *s, *next;
+
+        assert(m);
+
+        first = type == DNS_SERVER_FALLBACK ? &m->fallback_dns_servers : &m->dns_servers;
+
+        LIST_FOREACH_SAFE(servers, s, next, *first) {
+                if (!s->marked)
+                        continue;
+
+                LIST_REMOVE(servers, *first, s);
+                dns_server_unref(s);
+        }
+}
+
+void manager_mark_dns_servers(Manager *m, DnsServerType type) {
+        DnsServer *first, *s;
+
+        assert(m);
+
+        first = type == DNS_SERVER_FALLBACK ? m->fallback_dns_servers : m->dns_servers;
+
+        LIST_FOREACH(servers, s, first)
+                s->marked = true;
+}
diff --git a/src/resolve/resolved-dns-server.h b/src/resolve/resolved-dns-server.h
index 10111fd6bd..51cae62e42 100644
--- a/src/resolve/resolved-dns-server.h
+++ b/src/resolve/resolved-dns-server.h
@@ -68,6 +68,10 @@ DnsServer* dns_server_unref(DnsServer *s);
 void dns_server_packet_received(DnsServer *s, usec_t rtt);
 void dns_server_packet_lost(DnsServer *s, usec_t usec);
 
+void manager_flush_dns_servers(Manager *m, DnsServerType t);
+void manager_flush_marked_dns_servers(Manager *m, DnsServerType type);
+void manager_mark_dns_servers(Manager *m, DnsServerType type);
+
 DEFINE_TRIVIAL_CLEANUP_FUNC(DnsServer*, dns_server_unref);
 
 extern const struct hash_ops dns_server_hash_ops;
diff --git a/src/resolve/resolved-gperf.gperf b/src/resolve/resolved-gperf.gperf
index 8e78fbf06a..dc4ef74b8c 100644
--- a/src/resolve/resolved-gperf.gperf
+++ b/src/resolve/resolved-gperf.gperf
@@ -14,6 +14,6 @@ struct ConfigPerfItem;
 %struct-type
 %includes
 %%
-Resolve.DNS,          config_parse_dnsv,    DNS_SERVER_SYSTEM,   0
-Resolve.FallbackDNS,  config_parse_dnsv,    DNS_SERVER_FALLBACK, 0
-Resolve.LLMNR,        config_parse_support, 0,                   offsetof(Manager, llmnr_support)
+Resolve.DNS,          config_parse_dns_servers,    DNS_SERVER_SYSTEM,   0
+Resolve.FallbackDNS,  config_parse_dns_servers,    DNS_SERVER_FALLBACK, 0
+Resolve.LLMNR,        config_parse_support,        0,                   offsetof(Manager, llmnr_support)
diff --git a/src/resolve/resolved-manager.c b/src/resolve/resolved-manager.c
index 7c6500e477..c35379aa39 100644
--- a/src/resolve/resolved-manager.c
+++ b/src/resolve/resolved-manager.c
@@ -477,7 +477,7 @@ int manager_new(Manager **ret) {
         m->llmnr_support = SUPPORT_YES;
         m->read_resolv_conf = true;
 
-        r = manager_parse_dns_server(m, DNS_SERVER_FALLBACK, DNS_SERVERS);
+        r = manager_parse_dns_server_string_and_warn(m, DNS_SERVER_FALLBACK, DNS_SERVERS);
         if (r < 0)
                 return r;
 
@@ -583,7 +583,7 @@ int manager_read_resolv_conf(Manager *m) {
         _cleanup_fclose_ FILE *f = NULL;
         struct stat st, own;
         char line[LINE_MAX];
-        DnsServer *s, *nx;
+        DnsServer *s;
         usec_t t;
         int r;
 
@@ -633,14 +633,12 @@ int manager_read_resolv_conf(Manager *m) {
                 goto clear;
         }
 
-        LIST_FOREACH(servers, s, m->dns_servers)
-                s->marked = true;
+        manager_mark_dns_servers(m, DNS_SERVER_SYSTEM);
 
         FOREACH_LINE(line, f, r = -errno; goto clear) {
-                union in_addr_union address;
-                int family;
-                char *l;
+                _cleanup_strv_free_ char **d = NULL;
                 const char *a;
+                char *l;
 
                 truncate_nl(line);
 
@@ -649,33 +647,16 @@ int manager_read_resolv_conf(Manager *m) {
                         continue;
 
                 a = first_word(l, "nameserver");
-                if (!a)
-                        continue;
+                if (a) {
+                        r = manager_add_dns_server_by_string(m, DNS_SERVER_SYSTEM, a);
+                        if (r < 0)
+                                log_warning_errno(r, "Failed to parse DNS server address '%s', ignoring.", a);
 
-                r = in_addr_from_string_auto(a, &family, &address);
-                if (r < 0) {
-                        log_warning("Failed to parse name server %s.", a);
                         continue;
                 }
-
-                LIST_FOREACH(servers, s, m->dns_servers)
-                        if (s->family == family && in_addr_equal(family, &s->address, &address) > 0)
-                                break;
-
-                if (s)
-                        s->marked = false;
-                else {
-                        r = dns_server_new(m, NULL, DNS_SERVER_SYSTEM, NULL, family, &address);
-                        if (r < 0)
-                                goto clear;
-                }
         }
 
-        LIST_FOREACH_SAFE(servers, s, nx, m->dns_servers)
-                if (s->marked) {
-                        LIST_REMOVE(servers, m->dns_servers, s);
-                        dns_server_unref(s);
-                }
+        manager_flush_marked_dns_servers(m, DNS_SERVER_SYSTEM);
 
         /* Whenever /etc/resolv.conf changes, start using the first
          * DNS server of it. This is useful to deal with broken
@@ -716,13 +697,14 @@ static void write_resolv_conf_server(DnsServer *s, FILE *f, unsigned *count) {
 
         if (*count == MAXNS)
                 fputs("# Too many DNS servers configured, the following entries may be ignored.\n", f);
+        (*count) ++;
 
         fprintf(f, "nameserver %s\n", t);
-        (*count) ++;
 }
 
 static void write_resolv_conf_search(
-                const char *domain, FILE *f,
+                const char *domain,
+                FILE *f,
                 unsigned *count,
                 unsigned *length) {
 
@@ -740,10 +722,11 @@ static void write_resolv_conf_search(
                 return;
         }
 
-        fprintf(f, " %s", domain);
-
         (*length) += strlen(domain);
         (*count) ++;
+
+        fputc(' ', f);
+        fputs(domain, f);
 }
 
 static int write_resolv_conf_contents(FILE *f, OrderedSet *dns, OrderedSet *domains) {
@@ -758,8 +741,8 @@ static int write_resolv_conf_contents(FILE *f, OrderedSet *dns, OrderedSet *doma
         if (ordered_set_isempty(dns))
                 fputs("# No DNS servers known.\n", f);
         else {
-                DnsServer *s;
                 unsigned count = 0;
+                DnsServer *s;
 
                 ORDERED_SET_FOREACH(s, dns, i)
                         write_resolv_conf_server(s, f, &count);
@@ -802,7 +785,7 @@ int manager_write_resolv_conf(Manager *m) {
         if (!domains)
                 return -ENOMEM;
 
-        /* First add the system-wide servers */
+        /* First add the system-wide servers and domains */
         LIST_FOREACH(servers, s, m->dns_servers) {
                 r = ordered_set_put(dns, s);
                 if (r == -EEXIST)
@@ -1420,28 +1403,6 @@ void manager_verify_all(Manager *m) {
                 dns_zone_verify_all(&s->zone);
 }
 
-void manager_flush_dns_servers(Manager *m, DnsServerType t) {
-        DnsServer *s;
-
-        assert(m);
-
-        if (t == DNS_SERVER_SYSTEM)
-                while (m->dns_servers) {
-                        s = m->dns_servers;
-
-                        LIST_REMOVE(servers, m->dns_servers, s);
-                        dns_server_unref(s);
-                }
-
-        if (t == DNS_SERVER_FALLBACK)
-                while (m->fallback_dns_servers) {
-                        s = m->fallback_dns_servers;
-
-                        LIST_REMOVE(servers, m->fallback_dns_servers, s);
-                        dns_server_unref(s);
-                }
-}
-
 int manager_is_own_hostname(Manager *m, const char *name) {
         int r;
 
diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h
index 074ce6c63d..29b08cf0e9 100644
--- a/src/resolve/resolved-manager.h
+++ b/src/resolve/resolved-manager.h
@@ -138,8 +138,6 @@ DnsScope* manager_find_scope(Manager *m, DnsPacket *p);
 
 void manager_verify_all(Manager *m);
 
-void manager_flush_dns_servers(Manager *m, DnsServerType t);
-
 DEFINE_TRIVIAL_CLEANUP_FUNC(Manager*, manager_free);
 
 #define EXTRA_CMSG_SPACE 1024