Store DNS server count in resolv_cache.
Instead of keeping a sentinel after nameservers[], nsaddrinfo[] and
nstats[], store the server count in the structure, freeing up memory and
eliminating the need to enumerate the server count every time
_resolv_is_nameservers_equal_locked() is invoked.
Also increase MAXNS from 3 to 4.
BUG: 28153323
Change-Id: I11a7257af695157c9e32019cd00c67b535b63c75
diff --git a/libc/dns/include/resolv_netid.h b/libc/dns/include/resolv_netid.h
index 09c5498..266193a 100644
--- a/libc/dns/include/resolv_netid.h
+++ b/libc/dns/include/resolv_netid.h
@@ -87,8 +87,8 @@
const struct android_net_context *, struct addrinfo **) __used_in_netd;
/* set name servers for a network */
-extern void _resolv_set_nameservers_for_net(unsigned netid, const char** servers, int numservers,
- const char *domains, const struct __res_params* params) __used_in_netd;
+extern int _resolv_set_nameservers_for_net(unsigned netid, const char** servers,
+ unsigned numservers, const char *domains, const struct __res_params* params) __used_in_netd;
/* flush the cache associated with a certain network */
extern void _resolv_flush_cache_for_net(unsigned netid) __used_in_netd;
diff --git a/libc/dns/include/resolv_params.h b/libc/dns/include/resolv_params.h
index 353ae4d..f6948c0 100644
--- a/libc/dns/include/resolv_params.h
+++ b/libc/dns/include/resolv_params.h
@@ -20,11 +20,11 @@
#include <stdint.h>
/* Hard-coded defines */
-#define MAXNS 3 /* max # name servers we'll track */
+#define MAXNS 4 /* max # name servers we'll track */
#define MAXNSSAMPLES 64 /* max # samples to store per server */
/* Defaults used for initializing __res_params */
-#define SUCCESS_THRESHOLD 75 /* if successes * 100 / total_samples is less than
+#define SUCCESS_THRESHOLD 75 /* if successes * 100 / total_samples is less than
* this value, the server is considered failing
*/
#define NSSAMPLE_VALIDITY 1800 /* Sample validity in seconds.
diff --git a/libc/dns/resolv/res_cache.c b/libc/dns/resolv/res_cache.c
index 15f9aa4..5f51c49 100644
--- a/libc/dns/resolv/res_cache.c
+++ b/libc/dns/resolv/res_cache.c
@@ -1228,18 +1228,17 @@
PendingReqInfo pending_requests;
} Cache;
-// The nameservers[], nsaddrinfo[] and nsstats[] are containing MAXNS + 1 elements, because the
-// number of nameservers is not known and the code relies on the n+1-st entry to be null to
-// recognize the end.
struct resolv_cache_info {
unsigned netid;
Cache* cache;
struct resolv_cache_info* next;
- char* nameservers[MAXNS + 1];
- struct addrinfo* nsaddrinfo[MAXNS + 1];
+ int nscount;
+ char* nameservers[MAXNS];
+ struct addrinfo* nsaddrinfo[MAXNS];
int revision_id; // # times the nameservers have been replaced
struct __res_params params;
- struct __res_stats nsstats[MAXNS + 1];
+ struct __res_stats nsstats[MAXNS];
+ // TODO: replace with char* defdname[MAXDNSRCH]
char defdname[256];
int dnsrch_offset[MAXDNSRCH+1]; // offsets into defdname
};
@@ -1949,15 +1948,40 @@
params->max_samples = 0;
}
-void
-_resolv_set_nameservers_for_net(unsigned netid, const char** servers, int numservers,
+int
+_resolv_set_nameservers_for_net(unsigned netid, const char** servers, unsigned numservers,
const char *domains, const struct __res_params* params)
{
- int i, rt, index;
- struct addrinfo hints;
char sbuf[NI_MAXSERV];
register char *cp;
int *offset;
+ struct addrinfo* nsaddrinfo[MAXNS];
+
+ if (numservers > MAXNS) {
+ XLOG("%s: numservers=%u, MAXNS=%u", __FUNCTION__, numservers, MAXNS);
+ return E2BIG;
+ }
+
+ // Parse the addresses before actually locking or changing any state, in case there is an error.
+ // As a side effect this also reduces the time the lock is kept.
+ struct addrinfo hints = {
+ .ai_family = AF_UNSPEC,
+ .ai_socktype = SOCK_DGRAM,
+ .ai_flags = AI_NUMERICHOST
+ };
+ snprintf(sbuf, sizeof(sbuf), "%u", NAMESERVER_PORT);
+ for (unsigned i = 0; i < numservers; i++) {
+ // The addrinfo structures allocated here are freed in _free_nameservers_locked().
+ int rt = getaddrinfo(servers[i], sbuf, &hints, &nsaddrinfo[i]);
+ if (rt != 0) {
+ for (unsigned j = 0 ; j < i ; j++) {
+ freeaddrinfo(nsaddrinfo[j]);
+ nsaddrinfo[j] = NULL;
+ }
+ XLOG("%s: getaddrinfo(%s)=%s", __FUNCTION__, servers[i], gai_strerror(rt));
+ return EINVAL;
+ }
+ }
pthread_once(&_res_cache_once, _res_cache_init);
pthread_mutex_lock(&_res_cache_list_lock);
@@ -1978,24 +2002,13 @@
if (!_resolv_is_nameservers_equal_locked(cache_info, servers, numservers)) {
// free current before adding new
_free_nameservers_locked(cache_info);
-
- memset(&hints, 0, sizeof(hints));
- hints.ai_family = PF_UNSPEC;
- hints.ai_socktype = SOCK_DGRAM; /*dummy*/
- hints.ai_flags = AI_NUMERICHOST;
- snprintf(sbuf, sizeof(sbuf), "%u", NAMESERVER_PORT);
-
- index = 0;
- for (i = 0; i < numservers && i < MAXNS; i++) {
- rt = getaddrinfo(servers[i], sbuf, &hints, &cache_info->nsaddrinfo[index]);
- if (rt == 0) {
- cache_info->nameservers[index] = strdup(servers[i]);
- index++;
- XLOG("%s: netid = %u, addr = %s\n", __FUNCTION__, netid, servers[i]);
- } else {
- cache_info->nsaddrinfo[index] = NULL;
- }
+ unsigned i;
+ for (i = 0; i < numservers; i++) {
+ cache_info->nsaddrinfo[i] = nsaddrinfo[i];
+ cache_info->nameservers[i] = strdup(servers[i]);
+ XLOG("%s: netid = %u, addr = %s\n", __FUNCTION__, netid, servers[i]);
}
+ cache_info->nscount = numservers;
// code moved from res_init.c, load_domain_search_list
strlcpy(cache_info->defdname, domains, sizeof(cache_info->defdname));
@@ -2040,26 +2053,16 @@
}
pthread_mutex_unlock(&_res_cache_list_lock);
+ return 0;
}
static int
_resolv_is_nameservers_equal_locked(struct resolv_cache_info* cache_info,
const char** servers, int numservers)
{
- int i;
- char** ns;
- int currentservers;
- int equal = 1;
-
- if (numservers > MAXNS) numservers = MAXNS;
-
- // Find out how many nameservers we had before.
- currentservers = 0;
- for (ns = cache_info->nameservers; *ns; ns++)
- currentservers++;
-
- if (currentservers != numservers)
+ if (cache_info->nscount != numservers) {
return 0;
+ }
// Compare each name server against current name servers.
// TODO: this is incorrect if the list of current or previous nameservers
@@ -2067,26 +2070,25 @@
// filters out duplicates, but we should probably fix it. It's also
// insensitive to the order of the nameservers; we should probably fix that
// too.
- for (i = 0; i < numservers && equal; i++) {
- ns = cache_info->nameservers;
- equal = 0;
- while(*ns) {
- if (strcmp(*ns, servers[i]) == 0) {
- equal = 1;
+ for (int i = 0; i < numservers; i++) {
+ for (int j = 0 ; ; j++) {
+ if (j >= numservers) {
+ return 0;
+ }
+ if (strcmp(cache_info->nameservers[i], servers[j]) == 0) {
break;
}
- ns++;
}
}
- return equal;
+ return 1;
}
static void
_free_nameservers_locked(struct resolv_cache_info* cache_info)
{
int i;
- for (i = 0; i <= MAXNS; i++) {
+ for (i = 0; i < cache_info->nscount; i++) {
free(cache_info->nameservers[i]);
cache_info->nameservers[i] = NULL;
if (cache_info->nsaddrinfo[i] != NULL) {
@@ -2096,6 +2098,7 @@
cache_info->nsstats[i].sample_count =
cache_info->nsstats[i].sample_next = 0;
}
+ cache_info->nscount = 0;
_res_cache_clear_stats_locked(cache_info);
++cache_info->revision_id;
}
@@ -2182,7 +2185,6 @@
int
_resolv_cache_get_resolver_stats( unsigned netid, struct __res_params* params,
struct __res_stats stats[MAXNS]) {
-
int revision_id = -1;
pthread_mutex_lock(&_res_cache_list_lock);
diff --git a/libc/dns/resolv/res_send.c b/libc/dns/resolv/res_send.c
index 9ceeeb7..bfc6e1a 100644
--- a/libc/dns/resolv/res_send.c
+++ b/libc/dns/resolv/res_send.c
@@ -488,10 +488,10 @@
* Send request, RETRY times, or until successful.
*/
for (try = 0; try < statp->retry; try++) {
- struct __res_stats stats[MAXNS + 1];
+ struct __res_stats stats[MAXNS];
struct __res_params params;
int revision_id = _resolv_cache_get_resolver_stats(statp->netid, ¶ms, stats);
- bool usable_servers[MAXNS + 1];
+ bool usable_servers[MAXNS];
_res_stats_get_usable_servers(¶ms, stats, statp->nscount, usable_servers);
for (ns = 0; ns < statp->nscount; ns++) {
diff --git a/libc/dns/resolv/res_state.c b/libc/dns/resolv/res_state.c
index afccd99..0e02a8f 100644
--- a/libc/dns/resolv/res_state.c
+++ b/libc/dns/resolv/res_state.c
@@ -128,7 +128,7 @@
rt->_pi = (struct prop_info*) __system_property_find("net.change");
if (rt->_pi == NULL) {
/* Still nothing, return current state */
- D("%s: exiting for tid=%d rt=%d since system property not found",
+ D("%s: exiting for tid=%d rt=%p since system property not found",
__FUNCTION__, gettid(), rt);
return rt;
}
