Merge "Explicitly allocate ResState on the call stack"
am: 7803769cac
Change-Id: I5ca2d577b8a70dd59f52647318c583a5ebdc669a
diff --git a/Android.bp b/Android.bp
index d05881f..bc2c0cd 100644
--- a/Android.bp
+++ b/Android.bp
@@ -50,7 +50,6 @@
"res_mkquery.cpp",
"res_query.cpp",
"res_send.cpp",
- "res_state.cpp",
"res_stats.cpp",
"Dns64Configuration.cpp",
"DnsProxyListener.cpp",
diff --git a/PREUPLOAD.cfg b/PREUPLOAD.cfg
index c8dbf77..27eac94 100644
--- a/PREUPLOAD.cfg
+++ b/PREUPLOAD.cfg
@@ -1,5 +1,6 @@
[Builtin Hooks]
clang_format = true
+commit_msg_test_field = false
[Builtin Hooks Options]
clang_format = --commit ${PREUPLOAD_COMMIT} --style file --extensions c,h,cc,cpp
diff --git a/getaddrinfo.cpp b/getaddrinfo.cpp
index 4207251..46137de 100644
--- a/getaddrinfo.cpp
+++ b/getaddrinfo.cpp
@@ -56,6 +56,7 @@
#include <android-base/logging.h>
#include "netd_resolv/resolv.h"
+#include "res_init.h"
#include "resolv_cache.h"
#include "resolv_private.h"
@@ -1445,18 +1446,11 @@
return EAI_FAMILY;
}
- res_state res = res_get_state();
- if (!res) return EAI_MEMORY;
-
- /* this just sets our netid val in the thread private data so we don't have to
- * modify the api's all the way down to res_send.c's res_nsend. We could
- * fully populate the thread private data here, but if we get down there
- * and have a cache hit that would be wasted, so we do the rest there on miss
- */
- res_setnetcontext(res, netcontext, event);
+ ResState res;
+ res_init(&res, netcontext, event);
int he;
- if (res_searchN(name, &q, res, &he) < 0) {
+ if (res_searchN(name, &q, &res, &he) < 0) {
// Return h_errno (he) to catch more detailed errors rather than EAI_NODATA.
// Note that res_searchN() doesn't set the pair NETDB_INTERNAL and errno.
// See also herrnoToAiErrno().
@@ -1612,7 +1606,8 @@
LOG(DEBUG) << __func__ << ": (" << cl << ", " << type << ")";
- n = res_nmkquery(res, QUERY, name, cl, type, NULL, 0, NULL, buf, sizeof(buf));
+ n = res_nmkquery(QUERY, name, cl, type, /*data=*/nullptr, /*datalen=*/0, buf, sizeof(buf),
+ res->netcontext_flags);
if (n > 0 &&
(res->netcontext_flags &
(NET_CONTEXT_FLAG_USE_DNS_OVER_TLS | NET_CONTEXT_FLAG_USE_EDNS)) &&
diff --git a/gethnamaddr.cpp b/gethnamaddr.cpp
index 0d9683c..2a8c92b 100644
--- a/gethnamaddr.cpp
+++ b/gethnamaddr.cpp
@@ -74,6 +74,7 @@
#include "hostent.h"
#include "netd_resolv/resolv.h"
+#include "res_init.h"
#include "resolv_cache.h"
#include "resolv_private.h"
#include "stats.pb.h"
@@ -116,7 +117,7 @@
static int dns_gethtbyaddr(const unsigned char* uaddr, int len, int af,
const android_net_context* netcontext, getnamaddr* info,
NetworkDnsEventReported* event);
-static int dns_gethtbyname(const char* name, int af, getnamaddr* info);
+static int dns_gethtbyname(ResState* res, const char* name, int af, getnamaddr* info);
#define BOUNDED_INCR(x) \
do { \
@@ -393,9 +394,8 @@
NetworkDnsEventReported* event) {
getnamaddr info;
- res_state res = res_get_state();
- if (res == nullptr) return EAI_MEMORY;
- res_setnetcontext(res, netcontext, event);
+ ResState res;
+ res_init(&res, netcontext, event);
size_t size;
switch (af) {
@@ -450,7 +450,7 @@
info.buf = buf;
info.buflen = buflen;
if (_hf_gethtbyname2(name, af, &info)) {
- int error = dns_gethtbyname(name, af, &info);
+ int error = dns_gethtbyname(&res, name, af, &info);
if (error != 0) return error;
}
*result = hp;
@@ -652,7 +652,7 @@
});
}
-static int dns_gethtbyname(const char* name, int addr_type, getnamaddr* info) {
+static int dns_gethtbyname(ResState* res, const char* name, int addr_type, getnamaddr* info) {
int n, type;
info->hp->h_addrtype = addr_type;
@@ -670,9 +670,6 @@
}
auto buf = std::make_unique<querybuf>();
- res_state res = res_get_state();
- if (!res) return EAI_MEMORY;
-
int he;
n = res_nsearch(res, name, C_IN, type, buf->buf, (int)sizeof(buf->buf), &he);
if (n < 0) {
@@ -734,12 +731,10 @@
auto buf = std::make_unique<querybuf>();
- res_state res = res_get_state();
- if (!res) return EAI_MEMORY;
-
- res_setnetcontext(res, netcontext, event);
+ ResState res;
+ res_init(&res, netcontext, event);
int he;
- n = res_nquery(res, qbuf, C_IN, T_PTR, buf->buf, (int)sizeof(buf->buf), &he);
+ n = res_nquery(&res, qbuf, C_IN, T_PTR, buf->buf, (int)sizeof(buf->buf), &he);
if (n < 0) {
LOG(DEBUG) << __func__ << ": res_nquery failed (" << n << ")";
// Note that res_nquery() doesn't set the pair NETDB_INTERNAL and errno.
diff --git a/res_cache.cpp b/res_cache.cpp
index 4b5ba28..d26ad76 100644
--- a/res_cache.cpp
+++ b/res_cache.cpp
@@ -140,6 +140,7 @@
* *****************************************
*/
const int CONFIG_MAX_ENTRIES = 64 * 2 * 5;
+constexpr int DNSEVENT_SUBSAMPLING_MAP_DEFAULT_KEY = -1;
static time_t _time_now(void) {
struct timeval tv;
diff --git a/res_init.cpp b/res_init.cpp
index 12f4124..a1619d9 100644
--- a/res_init.cpp
+++ b/res_init.cpp
@@ -92,17 +92,19 @@
#include "netd_resolv/resolv.h"
#include "resolv_private.h"
-// Set up Resolver state default settings.
-// Note: this is called with statp zero-initialized
-void res_init(res_state statp) {
- statp->netid = NETID_UNSET;
+void res_init(ResState* statp, const struct android_net_context* _Nonnull netcontext,
+ android::net::NetworkDnsEventReported* _Nonnull event) {
+ memset(statp, 0, sizeof *statp);
+
+ statp->netid = netcontext->dns_netid;
+ statp->uid = netcontext->uid;
statp->id = arc4random_uniform(65536);
- statp->_mark = MARK_UNSET;
+ statp->_mark = netcontext->dns_mark;
+ statp->netcontext_flags = netcontext->flags;
+ statp->event = event;
statp->ndots = 1;
statp->_vcsock = -1;
- statp->_flags = 0;
- statp->netcontext_flags = 0;
for (int ns = 0; ns < MAXNS; ns++) {
statp->nssocks[ns] = -1;
@@ -142,14 +144,3 @@
}
}
}
-
-void res_setnetcontext(res_state statp, const struct android_net_context* netcontext,
- android::net::NetworkDnsEventReported* _Nonnull event) {
- if (statp != nullptr) {
- statp->netid = netcontext->dns_netid;
- statp->uid = netcontext->uid;
- statp->_mark = netcontext->dns_mark;
- statp->netcontext_flags = netcontext->flags;
- statp->event = event;
- }
-}
diff --git a/res_init.h b/res_init.h
index af76414..4c759c4 100644
--- a/res_init.h
+++ b/res_init.h
@@ -15,5 +15,8 @@
*/
#pragma once
-struct __res_state;
-void res_init(__res_state* statp);
+#include "resolv_private.h"
+
+// TODO: make this a constructor for ResState
+void res_init(ResState* res, const struct android_net_context* netcontext,
+ android::net::NetworkDnsEventReported* event);
diff --git a/res_mkquery.cpp b/res_mkquery.cpp
index 5dde9be..fb8a074 100644
--- a/res_mkquery.cpp
+++ b/res_mkquery.cpp
@@ -93,19 +93,15 @@
"11", "12", "13", "ZONEINIT", "ZONEREF",
};
-/*
- * Form all types of queries.
- * Returns the size of the result or -1.
- */
-int res_nmkquery(res_state statp, int op, /* opcode of query */
- const char* dname, /* domain name */
- int cl, int type, /* class and type of query */
- const uint8_t* data, /* resource record data */
- int datalen, /* length of data */
- const uint8_t* /*newrr_in*/, /* new rr for modify or append */
- uint8_t* buf, /* buffer to put query */
- int buflen) /* size of buffer */
-{
+// Form all types of queries. Returns the size of the result or -1.
+int res_nmkquery(int op, // opcode of query
+ const char* dname, // domain name
+ int cl, int type, // class and type of query
+ const uint8_t* data, // resource record data
+ int datalen, // length of data
+ uint8_t* buf, // buffer to put query
+ int buflen, // size of buffer
+ int netcontext_flags) {
HEADER* hp;
uint8_t *cp, *ep;
int n;
@@ -123,7 +119,7 @@
hp->id = htons(arc4random_uniform(65536));
hp->opcode = op;
hp->rd = true;
- hp->ad = (statp->netcontext_flags & NET_CONTEXT_FLAG_USE_DNS_OVER_TLS) != 0U;
+ hp->ad = (netcontext_flags & NET_CONTEXT_FLAG_USE_DNS_OVER_TLS) != 0U;
hp->rcode = NOERROR;
cp = buf + HFIXEDSZ;
ep = buf + buflen;
diff --git a/res_query.cpp b/res_query.cpp
index f54ff3f..43c81fa 100644
--- a/res_query.cpp
+++ b/res_query.cpp
@@ -123,7 +123,8 @@
LOG(DEBUG) << __func__ << ": (" << cl << ", " << type << ")";
- n = res_nmkquery(statp, QUERY, name, cl, type, NULL, 0, NULL, buf, sizeof(buf));
+ n = res_nmkquery(QUERY, name, cl, type, /*data=*/nullptr, 0, buf, sizeof(buf),
+ statp->netcontext_flags);
if (n > 0 &&
(statp->netcontext_flags &
(NET_CONTEXT_FLAG_USE_DNS_OVER_TLS | NET_CONTEXT_FLAG_USE_EDNS)) &&
diff --git a/res_send.cpp b/res_send.cpp
index 6616ca3..9eee05d 100644
--- a/res_send.cpp
+++ b/res_send.cpp
@@ -107,6 +107,7 @@
#include "netd_resolv/stats.h"
#include "private/android_filesystem_config.h"
#include "res_debug.h"
+#include "res_init.h"
#include "resolv_cache.h"
#include "stats.pb.h"
@@ -1237,9 +1238,9 @@
uint8_t* ans, int ansLen, int* rcode, uint32_t flags,
NetworkDnsEventReported* event) {
assert(event != nullptr);
- res_state res = res_get_state();
- res_setnetcontext(res, netContext, event);
- _resolv_populate_res_for_net(res);
+ ResState res;
+ res_init(&res, netContext, event);
+ _resolv_populate_res_for_net(&res);
*rcode = NOERROR;
- return res_nsend(res, msg, msgLen, ans, ansLen, rcode, flags);
+ return res_nsend(&res, msg, msgLen, ans, ansLen, rcode, flags);
}
diff --git a/res_state.cpp b/res_state.cpp
deleted file mode 100644
index 7388082..0000000
--- a/res_state.cpp
+++ /dev/null
@@ -1,95 +0,0 @@
-/*
- * Copyright (C) 2008 The Android Open Source Project
- * All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions
- * are met:
- * * Redistributions of source code must retain the above copyright
- * notice, this list of conditions and the following disclaimer.
- * * Redistributions in binary form must reproduce the above copyright
- * notice, this list of conditions and the following disclaimer in
- * the documentation and/or other materials provided with the
- * distribution.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
- * "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
- * LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS
- * FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE
- * COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT,
- * INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING,
- * BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS
- * OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED
- * AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT
- * OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
- * SUCH DAMAGE.
- */
-
-#define LOG_TAG "resolv"
-
-#include <arpa/inet.h>
-#include <arpa/nameser.h>
-#include <netdb.h>
-#include <pthread.h>
-#include <stdlib.h>
-#include <string.h>
-#include <unistd.h> /* for gettid() */
-
-#include <android-base/logging.h>
-
-#include "res_init.h"
-#include "resolv_cache.h"
-#include "resolv_private.h"
-
-typedef struct {
- // TODO: Have one __res_state per network so we don't have to repopulate frequently.
- struct __res_state _nres[1];
-} _res_thread;
-
-static _res_thread* res_thread_alloc(void) {
- _res_thread* rt = (_res_thread*) calloc(1, sizeof(*rt));
- return rt;
-}
-
-static void res_thread_free(void* _rt) {
- _res_thread* rt = (_res_thread*) _rt;
-
- LOG(VERBOSE) << __func__ << ": rt=" << rt << " for thread=" << gettid();
-
- res_nclose(rt->_nres);
- free(rt);
-}
-
-static pthread_key_t _res_key;
-
-__attribute__((constructor)) static void __res_key_init() {
- pthread_key_create(&_res_key, res_thread_free);
-}
-
-static _res_thread* res_thread_get(void) {
- _res_thread* rt = (_res_thread*) pthread_getspecific(_res_key);
- if (rt != NULL) {
- return rt;
- }
-
- /* It is the first time this function is called in this thread,
- * we need to create a new thread-specific DNS resolver state. */
- rt = res_thread_alloc();
- if (rt == NULL) {
- return NULL;
- }
- pthread_setspecific(_res_key, rt);
-
- LOG(VERBOSE) << __func__ << ": tid=" << gettid() << ", rt=" << rt;
- res_init(rt->_nres);
- return rt;
-}
-
-struct __res_state _nres;
-
-res_state res_get_state(void) {
- _res_thread* rt = res_thread_get();
-
- return rt ? rt->_nres : NULL;
-}
diff --git a/resolv_cache.h b/resolv_cache.h
index 9a9787e..2b372f6 100644
--- a/resolv_cache.h
+++ b/resolv_cache.h
@@ -35,15 +35,11 @@
#include <unordered_map>
#include <vector>
-struct __res_state;
-struct resolv_cache;
-
-constexpr int DNSEVENT_SUBSAMPLING_MAP_DEFAULT_KEY = -1;
-
-/* sets the name server addresses to the provided res_state structure. The
- * name servers are retrieved from the cache which is associated
- * with the network to which the res_state structure is associated */
-void _resolv_populate_res_for_net(struct __res_state* statp);
+// Sets the name server addresses to the provided ResState.
+// The name servers are retrieved from the cache which is associated
+// with the network to which ResState is associated.
+struct ResState;
+void _resolv_populate_res_for_net(ResState* statp);
std::vector<unsigned> resolv_list_caches();
diff --git a/resolv_cache_unit_test.cpp b/resolv_cache_unit_test.cpp
index f62f317..1db209f 100644
--- a/resolv_cache_unit_test.cpp
+++ b/resolv_cache_unit_test.cpp
@@ -30,6 +30,7 @@
#include <gtest/gtest.h>
#include "netd_resolv/stats.h"
+#include "res_init.h"
#include "resolv_cache.h"
#include "resolv_private.h"
#include "tests/dns_responder/dns_responder.h"
@@ -64,9 +65,10 @@
};
std::vector<char> makeQuery(int op, const char* qname, int qclass, int qtype) {
- res_state res = res_get_state();
uint8_t buf[MAXPACKET] = {};
- const int len = res_nmkquery(res, op, qname, qclass, qtype, NULL, 0, NULL, buf, sizeof(buf));
+ const int len = res_nmkquery(op, qname, qclass, qtype, /*data=*/nullptr, /*datalen=*/0, buf,
+ sizeof(buf),
+ /*netcontext_flags=*/0);
return std::vector<char>(buf, buf + len);
}
diff --git a/resolv_private.h b/resolv_private.h
index 4c881c2..3272844 100644
--- a/resolv_private.h
+++ b/resolv_private.h
@@ -87,7 +87,7 @@
struct sockaddr_in6 sin6;
};
-struct __res_state {
+struct ResState {
unsigned netid; // NetId: cache key and socket mark
uid_t uid; // uid of the app that sent the DNS lookup
int nscount; // number of name srvers
@@ -103,7 +103,9 @@
uint32_t netcontext_flags;
};
-typedef struct __res_state* res_state;
+// TODO: remove these legacy aliases
+typedef ResState __res_state;
+typedef ResState* res_state;
/* Retrieve a local copy of the stats for the given netid. The buffer must have space for
* MAXNS __resolver_stats. Returns the revision id of the resolvers used.
@@ -140,9 +142,6 @@
extern const char* const _res_opcodes[];
-/* Things involving an internal (static) resolver context. */
-struct __res_state* res_get_state(void);
-
int res_hnok(const char*);
int res_ownok(const char*);
int res_mailok(const char*);
@@ -157,16 +156,12 @@
int res_nquery(res_state, const char*, int, int, uint8_t*, int, int*);
int res_nsearch(res_state, const char*, int, int, uint8_t*, int, int*);
int res_nquerydomain(res_state, const char*, const char*, int, int, uint8_t*, int, int*);
-int res_nmkquery(res_state, int, const char*, int, int, const uint8_t*, int, const uint8_t*,
- uint8_t*, int);
+int res_nmkquery(int op, const char* qname, int cl, int type, const uint8_t* data, int datalen,
+ uint8_t* buf, int buflen, int netcontext_flags);
int res_nsend(res_state, const uint8_t*, int, uint8_t*, int, int*, uint32_t);
void res_nclose(res_state);
int res_nopt(res_state, int, uint8_t*, int, int);
-struct android_net_context;
-void res_setnetcontext(res_state, const struct android_net_context*,
- android::net::NetworkDnsEventReported* event);
-
int getaddrinfo_numeric(const char* hostname, const char* servname, addrinfo hints,
addrinfo** result);