From 37b7cc8d9a111c40767e2124fbdc368cc8da5c1c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 31 Jul 2018 10:01:46 +0200 Subject: [PATCH] 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. --- src/resolve/resolved-etc-hosts.c | 166 ++++++++++++++++--------------- src/resolve/resolved-manager.h | 8 +- 2 files changed, 93 insertions(+), 81 deletions(-) diff --git a/src/resolve/resolved-etc-hosts.c b/src/resolve/resolved-etc-hosts.c index f42a73f7ba0..a400331b0dd 100644 --- 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; +static inline void etc_hosts_item_free(EtcHostsItem *item) { + strv_free(item->names); + free(item); +} + +static inline void etc_hosts_item_by_name_free(EtcHostsItemByName *item) { + free(item->name); + free(item->addresses); + free(item); +} + +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) { - EtcHostsItem *item; - EtcHostsItemByName *bn; - - while ((item = hashmap_steal_first(m->etc_hosts_by_address))) { - strv_free(item->names); - free(item); - } - - while ((bn = hashmap_steal_first(m->etc_hosts_by_name))) { - free(bn->name); - free(bn->addresses); - free(bn); - } - - m->etc_hosts_by_address = hashmap_free(m->etc_hosts_by_address); - m->etc_hosts_by_name = hashmap_free(m->etc_hosts_by_name); - + 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,55 +161,12 @@ static int parse_line(Manager *m, unsigned nr, const char *line) { return 0; } -static int manager_etc_hosts_read(Manager *m) { - _cleanup_fclose_ FILE *f = NULL; +static int etc_hosts_parse(EtcHosts *hosts, FILE *f) { + _cleanup_(etc_hosts_free) EtcHosts t = {}; 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); - - /* See if we checked /etc/hosts recently already */ - if (m->etc_hosts_last != USEC_INFINITY && m->etc_hosts_last + ETC_HOSTS_RECHECK_USEC > ts) - return 0; - - m->etc_hosts_last = ts; - - if (m->etc_hosts_mtime != USEC_INFINITY) { - if (stat("/etc/hosts", &st) < 0) { - if (errno == ENOENT) { - r = 0; - goto clear; - } - - return log_error_errno(errno, "Failed to stat /etc/hosts: %m"); - } - - /* Did the mtime change? If not, there's no point in re-reading the file. */ - if (timespec_load(&st.st_mtim) == m->etc_hosts_mtime) - return 0; - } - - f = fopen("/etc/hosts", "re"); - if (!f) { - if (errno == ENOENT) { - r = 0; - goto clear; - } - - return log_error_errno(errno, "Failed to open /etc/hosts: %m"); - } - - /* Take the timestamp at the beginning of processing, so that any changes made later are read on the next - * invocation */ - r = fstat(fileno(f), &st); - if (r < 0) - return log_error_errno(errno, "Failed to fstat() /etc/hosts: %m"); - - manager_etc_hosts_flush(m); - FOREACH_LINE(line, f, return log_error_errno(errno, "Failed to read /etc/hosts: %m")) { char *l; @@ -221,19 +178,70 @@ static int manager_etc_hosts_read(Manager *m) { 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 = 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; + struct stat st; + usec_t ts; + int r; + + assert_se(sd_event_now(m->event, clock_boottime_or_monotonic(), &ts) >= 0); + + /* See if we checked /etc/hosts recently already */ + if (m->etc_hosts_last != USEC_INFINITY && m->etc_hosts_last + ETC_HOSTS_RECHECK_USEC > ts) + return 0; + + m->etc_hosts_last = ts; + + if (m->etc_hosts_mtime != USEC_INFINITY) { + if (stat("/etc/hosts", &st) < 0) { + if (errno != ENOENT) + 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. */ + if (timespec_load(&st.st_mtim) == m->etc_hosts_mtime) + return 0; + } + + f = fopen("/etc/hosts", "re"); + if (!f) { + if (errno != ENOENT) + 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 + * invocation */ + r = fstat(fileno(f), &st); + if (r < 0) + return log_error_errno(errno, "Failed to fstat() /etc/hosts: %m"); + + manager_etc_hosts_flush(m); + + 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 798040378e1..c9cf2f0ee6a 100644 --- 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;