resolved: put /etc/hosts hashmaps in a structure and pass that around

This hides the details of juggling the two hashmaps from the callers a bit.
It also makes memory management a bit easier, because those two hashmaps share
some strings, so we can only free them together.

etc_hosts_parse() is made responsible to free the half-filled data structures
on error, which makes the caller a bit simpler.

No functional change. A refactoring to prepare for later changes.

diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c
index f42a73f7ba04f10a8872d3ed53a11428f595496b..a400331b0dd4156bbf08dbd0a38f86561b5c9f38 100644 (file)
--- a/src/resolve/resolved-etc-hosts.c
+++ b/src/resolve/resolved-etc-hosts.c
@@ -25,35 +25,35 @@ typedef struct EtcHostsItemByName {
         size_t n_addresses, n_allocated;
 } EtcHostsItemByName;
 
-void manager_etc_hosts_flush(Manager *m) {
-        EtcHostsItem *item;
-        EtcHostsItemByName *bn;
-
-        while ((item = hashmap_steal_first(m->etc_hosts_by_address))) {
-                strv_free(item->names);
-                free(item);
-        }
+static inline void etc_hosts_item_free(EtcHostsItem *item) {
+        strv_free(item->names);
+        free(item);
+}
 
-        while ((bn = hashmap_steal_first(m->etc_hosts_by_name))) {
-                free(bn->name);
-                free(bn->addresses);
-                free(bn);
-        }
+static inline void etc_hosts_item_by_name_free(EtcHostsItemByName *item) {
+        free(item->name);
+        free(item->addresses);
+        free(item);
+}
 
-        m->etc_hosts_by_address = hashmap_free(m->etc_hosts_by_address);
-        m->etc_hosts_by_name = hashmap_free(m->etc_hosts_by_name);
+static void etc_hosts_free(EtcHosts *hosts) {
+        hosts->by_address = hashmap_free_with_destructor(hosts->by_address, etc_hosts_item_free);
+        hosts->by_name = hashmap_free_with_destructor(hosts->by_name, etc_hosts_item_by_name_free);
+}
 
+void manager_etc_hosts_flush(Manager *m) {
+        etc_hosts_free(&m->etc_hosts);
         m->etc_hosts_mtime = USEC_INFINITY;
 }
 
-static int parse_line(Manager *m, unsigned nr, const char *line) {
+static int parse_line(EtcHosts *hosts, unsigned nr, const char *line) {
         _cleanup_free_ char *address_str = NULL;
         struct in_addr_data address = {};
         bool suppressed = false;
         EtcHostsItem *item;
         int r;
 
-        assert(m);
+        assert(hosts);
         assert(line);
 
         r = extract_first_word(&line, &address_str, NULL, EXTRACT_RELAX);
@@ -78,9 +78,9 @@ static int parse_line(Manager *m, unsigned nr, const char *line) {
         else {
                 /* If this is a normal address, then, simply add entry mapping it to the specified names */
 
-                item = hashmap_get(m->etc_hosts_by_address, &address);
+                item = hashmap_get(hosts->by_address, &address);
                 if (!item) {
-                        r = hashmap_ensure_allocated(&m->etc_hosts_by_address, &in_addr_data_hash_ops);
+                        r = hashmap_ensure_allocated(&hosts->by_address, &in_addr_data_hash_ops);
                         if (r < 0)
                                 return log_oom();
 
@@ -90,7 +90,7 @@ static int parse_line(Manager *m, unsigned nr, const char *line) {
 
                         item->address = address;
 
-                        r = hashmap_put(m->etc_hosts_by_address, &item->address, item);
+                        r = hashmap_put(hosts->by_address, &item->address, item);
                         if (r < 0) {
                                 free(item);
                                 return log_oom();
@@ -124,9 +124,9 @@ static int parse_line(Manager *m, unsigned nr, const char *line) {
                                 return log_oom();
                 }
 
-                bn = hashmap_get(m->etc_hosts_by_name, name);
+                bn = hashmap_get(hosts->by_name, name);
                 if (!bn) {
-                        r = hashmap_ensure_allocated(&m->etc_hosts_by_name, &dns_name_hash_ops);
+                        r = hashmap_ensure_allocated(&hosts->by_name, &dns_name_hash_ops);
                         if (r < 0)
                                 return log_oom();
 
@@ -134,7 +134,7 @@ static int parse_line(Manager *m, unsigned nr, const char *line) {
                         if (!bn)
                                 return log_oom();
 
-                        r = hashmap_put(m->etc_hosts_by_name, name, bn);
+                        r = hashmap_put(hosts->by_name, name, bn);
                         if (r < 0) {
                                 free(bn);
                                 return log_oom();
@@ -161,12 +161,37 @@ static int parse_line(Manager *m, unsigned nr, const char *line) {
         return 0;
 }
 
+static int etc_hosts_parse(EtcHosts *hosts, FILE *f) {
+        _cleanup_(etc_hosts_free) EtcHosts t = {};
+        char line[LINE_MAX];
+        unsigned nr = 0;
+        int r;
+
+        FOREACH_LINE(line, f, return log_error_errno(errno, "Failed to read /etc/hosts: %m")) {
+                char *l;
+
+                nr++;
+
+                l = strstrip(line);
+                if (isempty(l))
+                        continue;
+                if (l[0] == '#')
+                        continue;
+
+                r = parse_line(&t, nr, l);
+                if (r < 0)
+                        return r;
+        }
+
+        *hosts = t;
+        t = (EtcHosts) {}; /* prevent cleanup */
+        return 0;
+}
+
 static int manager_etc_hosts_read(Manager *m) {
         _cleanup_fclose_ FILE *f = NULL;
-        char line[LINE_MAX];
         struct stat st;
         usec_t ts;
-        unsigned nr = 0;
         int r;
 
         assert_se(sd_event_now(m->event, clock_boottime_or_monotonic(), &ts) >= 0);
@@ -179,12 +204,11 @@ static int manager_etc_hosts_read(Manager *m) {
 
         if (m->etc_hosts_mtime != USEC_INFINITY) {
                 if (stat("/etc/hosts", &st) < 0) {
-                        if (errno == ENOENT) {
-                                r = 0;
-                                goto clear;
-                        }
+                        if (errno != ENOENT)
+                                return log_error_errno(errno, "Failed to stat /etc/hosts: %m");
 
-                        return log_error_errno(errno, "Failed to stat /etc/hosts: %m");
+                        manager_etc_hosts_flush(m);
+                        return 0;
                 }
 
                 /* Did the mtime change? If not, there's no point in re-reading the file. */
@@ -194,12 +218,11 @@ static int manager_etc_hosts_read(Manager *m) {
 
         f = fopen("/etc/hosts", "re");
         if (!f) {
-                if (errno == ENOENT) {
-                        r = 0;
-                        goto clear;
-                }
+                if (errno != ENOENT)
+                        return log_error_errno(errno, "Failed to open /etc/hosts: %m");
 
-                return log_error_errno(errno, "Failed to open /etc/hosts: %m");
+                manager_etc_hosts_flush(m);
+                return 0;
         }
 
         /* Take the timestamp at the beginning of processing, so that any changes made later are read on the next
@@ -210,30 +233,15 @@ static int manager_etc_hosts_read(Manager *m) {
 
         manager_etc_hosts_flush(m);
 
-        FOREACH_LINE(line, f, return log_error_errno(errno, "Failed to read /etc/hosts: %m")) {
-                char *l;
-
-                nr++;
-
-                l = strstrip(line);
-                if (isempty(l))
-                        continue;
-                if (l[0] == '#')
-                        continue;
-
-                r = parse_line(m, nr, l);
-                if (r == -ENOMEM) /* On OOM we abandon the half-built-up structure. All other errors we ignore and proceed */
-                        goto clear;
-        }
+        r = etc_hosts_parse(&m->etc_hosts, f);
+        if (r == -ENOMEM)
+                /* On OOM we bail. All other errors we ignore and proceed. */
+                return r;
 
         m->etc_hosts_mtime = timespec_load(&st.st_mtim);
         m->etc_hosts_last = ts;
 
         return 1;
-
-clear:
-        manager_etc_hosts_flush(m);
-        return r;
 }
 
 int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) {
@@ -265,7 +273,7 @@ int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) {
                 EtcHostsItem *item;
                 DnsResourceKey *found_ptr = NULL;
 
-                item = hashmap_get(m->etc_hosts_by_address, &k);
+                item = hashmap_get(m->etc_hosts.by_address, &k);
                 if (!item)
                         return 0;
 
@@ -314,7 +322,7 @@ int manager_etc_hosts_lookup(Manager *m, DnsQuestion* q, DnsAnswer **answer) {
                 return 1;
         }
 
-        bn = hashmap_get(m->etc_hosts_by_name, name);
+        bn = hashmap_get(m->etc_hosts.by_name, name);
         if (!bn)
                 return 0;
 

diff --git a/src/resolve/resolved-manager.h b/src/resolve/resolved-manager.h
index 798040378e1fc5096e493f6ba72868cbed47aee8..c9cf2f0ee6abc504668a8e1c261b204c6fa1a5d2 100644 (file)
--- a/src/resolve/resolved-manager.h
+++ b/src/resolve/resolved-manager.h
@@ -23,6 +23,11 @@ typedef struct Manager Manager;
 #define MANAGER_SEARCH_DOMAINS_MAX 32
 #define MANAGER_DNS_SERVERS_MAX 32
 
+typedef struct EtcHosts {
+        Hashmap *by_address;
+        Hashmap *by_name;
+} EtcHosts;
+
 struct Manager {
         sd_event *event;
 
@@ -114,8 +119,7 @@ struct Manager {
         unsigned n_dnssec_verdict[_DNSSEC_VERDICT_MAX];
 
         /* Data from /etc/hosts */
-        Hashmap* etc_hosts_by_address;
-        Hashmap* etc_hosts_by_name;
+        EtcHosts etc_hosts;
         usec_t etc_hosts_last, etc_hosts_mtime;
         bool read_etc_hosts;