Enable -Wsign-compare for netd and fix warnings
Test: atest netd_unit_test netd_integration_test resolv_integration_test
Change-Id: I84347de8f3a3ec0dcc8979037b9c265d145a35f7
diff --git a/Android.bp b/Android.bp
index 64831b8..32e719b 100644
--- a/Android.bp
+++ b/Android.bp
@@ -11,9 +11,9 @@
// Override -Wno-error=implicit-fallthrough from soong
"-Werror=implicit-fallthrough",
"-Wnullable-to-nonnull-conversion",
+ "-Wsign-compare",
"-Wthread-safety",
"-Wunused-parameter",
- "-Wsign-compare",
],
tidy: true,
tidy_checks: [
diff --git a/client/FwmarkClient.cpp b/client/FwmarkClient.cpp
index 9435528..ed1459c 100644
--- a/client/FwmarkClient.cpp
+++ b/client/FwmarkClient.cpp
@@ -25,7 +25,7 @@
#include <sys/un.h>
#include <unistd.h>
-#define ARRAY_SIZE(a) (sizeof(a) / sizeof(*(a)))
+#include <iterator>
namespace {
@@ -94,7 +94,7 @@
msghdr message;
memset(&message, 0, sizeof(message));
message.msg_iov = iov;
- message.msg_iovlen = ARRAY_SIZE(iov);
+ message.msg_iovlen = std::size(iov);
union {
cmsghdr cmh;
diff --git a/libnetdutils/InternetAddressesTest.cpp b/libnetdutils/InternetAddressesTest.cpp
index 59d8210..2129ca7 100644
--- a/libnetdutils/InternetAddressesTest.cpp
+++ b/libnetdutils/InternetAddressesTest.cpp
@@ -383,24 +383,22 @@
{"2001:db8:cafe:d00d::", 46, "2001:db8:cafc::"},
};
- int totalTests = 0;
- for (unsigned int i = 0; i < arraysize(testExpectations); i++, totalTests++) {
+ for (const auto& expectation : testExpectations) {
IPAddress ip;
- EXPECT_TRUE(IPAddress::forString(testExpectations[i].ip, &ip))
- << "Failed to parse IP address " << testExpectations[i].ip;
+ EXPECT_TRUE(IPAddress::forString(expectation.ip, &ip))
+ << "Failed to parse IP address " << expectation.ip;
IPAddress ipTruncated;
- EXPECT_TRUE(IPAddress::forString(testExpectations[i].ipTruncated, &ipTruncated))
- << "Failed to parse IP address " << testExpectations[i].ipTruncated;
+ EXPECT_TRUE(IPAddress::forString(expectation.ipTruncated, &ipTruncated))
+ << "Failed to parse IP address " << expectation.ipTruncated;
- IPPrefix prefix(ip, testExpectations[i].cidrLen);
+ IPPrefix prefix(ip, expectation.cidrLen);
- EXPECT_EQ(testExpectations[i].cidrLen, prefix.length())
- << "Unexpected cidrLen " << testExpectations[i].cidrLen;
+ EXPECT_EQ(expectation.cidrLen, prefix.length())
+ << "Unexpected cidrLen " << expectation.cidrLen;
EXPECT_EQ(ipTruncated, prefix.ip())
<< "Unexpected IP truncation: " << prefix.ip() << ", expected: " << ipTruncated;
}
- EXPECT_EQ(static_cast<int>(arraysize(testExpectations)), totalTests);
}
TEST(IPPrefixTest, GamutOfOperators) {
diff --git a/resolv/PrivateDnsConfiguration.cpp b/resolv/PrivateDnsConfiguration.cpp
index 616662e..b262a2f 100644
--- a/resolv/PrivateDnsConfiguration.cpp
+++ b/resolv/PrivateDnsConfiguration.cpp
@@ -26,15 +26,15 @@
#include "netdutils/BackoffSequence.h"
int resolv_set_private_dns_for_net(unsigned netid, uint32_t mark, const char** servers,
- const unsigned numServers, const char* tlsName,
- const uint8_t** fingerprints, const unsigned numFingerprint) {
+ const int numServers, const char* tlsName,
+ const uint8_t** fingerprints, const int numFingerprint) {
std::vector<std::string> tlsServers;
- for (unsigned i = 0; i < numServers; i++) {
+ for (int i = 0; i < numServers; i++) {
tlsServers.push_back(std::string(servers[i]));
}
std::set<std::vector<uint8_t>> tlsFingerprints;
- for (unsigned i = 0; i < numFingerprint; i++) {
+ for (int i = 0; i < numFingerprint; i++) {
// Each fingerprint stored are 32(SHA256_SIZE) bytes long.
tlsFingerprints.emplace(std::vector<uint8_t>(fingerprints[i], fingerprints[i] + 32));
}
@@ -194,7 +194,7 @@
const auto netPair = mPrivateDnsTransports.find(netId);
if (netPair != mPrivateDnsTransports.end()) {
- status->numServers = netPair->second.size();
+ status->numServers = static_cast<int>(netPair->second.size());
int count = 0;
for (const auto& serverPair : netPair->second) {
status->serverStatus[count].ss = serverPair.first.ss;
diff --git a/resolv/include/netd_resolv/resolv.h b/resolv/include/netd_resolv/resolv.h
index f50744c..9989766 100644
--- a/resolv/include/netd_resolv/resolv.h
+++ b/resolv/include/netd_resolv/resolv.h
@@ -91,7 +91,7 @@
struct ExternalPrivateDnsStatus {
PrivateDnsMode mode;
- unsigned numServers;
+ int numServers;
struct PrivateDnsInfo {
sockaddr_storage ss;
const char* hostname;
@@ -119,15 +119,14 @@
// Set name servers for a network
LIBNETD_RESOLV_PUBLIC int resolv_set_nameservers_for_net(unsigned netid, const char** servers,
- unsigned numservers, const char* domains,
+ int numservers, const char* domains,
const __res_params* params);
LIBNETD_RESOLV_PUBLIC int resolv_set_private_dns_for_net(unsigned netid, uint32_t mark,
- const char** servers,
- const unsigned numServers,
+ const char** servers, int numServers,
const char* tlsName,
const uint8_t** fingerprints,
- const unsigned numFingerprints);
+ int numFingerprints);
LIBNETD_RESOLV_PUBLIC void resolv_delete_private_dns_for_net(unsigned netid);
diff --git a/resolv/res_cache.cpp b/resolv/res_cache.cpp
index fd1a324..60a60df 100644
--- a/resolv/res_cache.cpp
+++ b/resolv/res_cache.cpp
@@ -1729,7 +1729,7 @@
params->base_timeout_msec = 0; // 0 = legacy algorithm
}
-int resolv_set_nameservers_for_net(unsigned netid, const char** servers, unsigned numservers,
+int resolv_set_nameservers_for_net(unsigned netid, const char** servers, const int numservers,
const char* domains, const __res_params* params) {
char* cp;
int* offset;
@@ -1744,13 +1744,13 @@
// As a side effect this also reduces the time the lock is kept.
char sbuf[NI_MAXSERV];
snprintf(sbuf, sizeof(sbuf), "%u", NAMESERVER_PORT);
- for (unsigned i = 0; i < numservers; i++) {
+ for (int i = 0; i < numservers; i++) {
// The addrinfo structures allocated here are freed in free_nameservers_locked().
const addrinfo hints = {
.ai_family = AF_UNSPEC, .ai_socktype = SOCK_DGRAM, .ai_flags = AI_NUMERICHOST};
int rt = getaddrinfo_numeric(servers[i], sbuf, hints, &nsaddrinfo[i]);
if (rt != 0) {
- for (unsigned j = 0; j < i; j++) {
+ for (int j = 0; j < i; j++) {
freeaddrinfo(nsaddrinfo[j]);
}
VLOG << __func__ << ": getaddrinfo_numeric(" << servers[i]
@@ -1778,8 +1778,7 @@
if (!resolv_is_nameservers_equal_locked(cache_info, servers, numservers)) {
// free current before adding new
free_nameservers_locked(cache_info);
- unsigned i;
- for (i = 0; i < numservers; i++) {
+ for (int i = 0; i < numservers; i++) {
cache_info->nsaddrinfo[i] = nsaddrinfo[i];
cache_info->nameservers[i] = strdup(servers[i]);
VLOG << __func__ << ": netid = " << netid << ", addr = " << servers[i];
@@ -1804,7 +1803,7 @@
res_cache_clear_stats_locked(cache_info);
++cache_info->revision_id;
}
- for (unsigned j = 0; j < numservers; j++) {
+ for (int j = 0; j < numservers; j++) {
freeaddrinfo(nsaddrinfo[j]);
}
}
diff --git a/resolv/resolver_test.cpp b/resolv/resolver_test.cpp
index d0c0ba5..287c2eb 100644
--- a/resolv/resolver_test.cpp
+++ b/resolv/resolver_test.cpp
@@ -140,7 +140,7 @@
std::vector<int32_t> stats32;
auto rv = mNetdSrv->getResolverInfo(TEST_NETID, servers, domains, tlsServers, ¶ms32,
&stats32);
- if (!rv.isOk() || params32.size() != INetd::RESOLVER_PARAMS_COUNT) {
+ if (!rv.isOk() || params32.size() != static_cast<size_t>(INetd::RESOLVER_PARAMS_COUNT)) {
return false;
}
*params = __res_params {
@@ -456,10 +456,10 @@
INetd::RESOLVER_PARAMS_MAX_SAMPLES,
INetd::RESOLVER_PARAMS_BASE_TIMEOUT_MSEC,
};
- int size = static_cast<int>(params_offsets.size());
+ const int size = static_cast<int>(params_offsets.size());
EXPECT_EQ(size, INetd::RESOLVER_PARAMS_COUNT);
std::sort(params_offsets.begin(), params_offsets.end());
- for (int i = 0 ; i < size ; ++i) {
+ for (int i = 0; i < size; ++i) {
EXPECT_EQ(params_offsets[i], i);
}
}
@@ -479,8 +479,8 @@
ASSERT_TRUE(SetResolversForNetwork(servers, domains, mDefaultParams_Binder));
const hostent* result = gethostbyname(mapping.host.c_str());
- size_t total_queries = std::accumulate(dns.begin(), dns.end(), 0,
- [this, &mapping](size_t total, auto& d) {
+ const size_t total_queries =
+ std::accumulate(dns.begin(), dns.end(), 0, [this, &mapping](size_t total, auto& d) {
return total + GetNumQueriesForType(*d, ns_type::ns_t_a, mapping.entry.c_str());
});
@@ -696,7 +696,7 @@
// for the next sample_lifetime seconds.
// TODO: This approach is implementation-dependent, change once metrics reporting is available.
addrinfo hints = {.ai_family = AF_INET6};
- for (int i = 0 ; i < sample_count ; ++i) {
+ for (int i = 0; i < sample_count; ++i) {
std::string domain = StringPrintf("nonexistent%d", i);
ScopedAddrinfo result = safe_getaddrinfo(domain.c_str(), nullptr, &hints);
}
diff --git a/server/DnsProxyListener.cpp b/server/DnsProxyListener.cpp
index 73a1e2a..23897ad 100644
--- a/server/DnsProxyListener.cpp
+++ b/server/DnsProxyListener.cpp
@@ -126,7 +126,7 @@
RESOLV_STUB.resolv_get_private_dns_status_for_net(dns_netid, &privateDnsStatus);
switch (static_cast<PrivateDnsMode>(privateDnsStatus.mode)) {
case PrivateDnsMode::OPPORTUNISTIC:
- for (unsigned i = 0; i < privateDnsStatus.numServers; i++) {
+ for (int i = 0; i < privateDnsStatus.numServers; i++) {
if (privateDnsStatus.serverStatus[i].validation == Validation::success) {
return true;
}
diff --git a/server/ResolverController.cpp b/server/ResolverController.cpp
index a0dc3de..39a0b83 100644
--- a/server/ResolverController.cpp
+++ b/server/ResolverController.cpp
@@ -321,7 +321,7 @@
ExternalPrivateDnsStatus privateDnsStatus = {PrivateDnsMode::OFF, 0, {}};
RESOLV_STUB.resolv_get_private_dns_status_for_net(netId, &privateDnsStatus);
- for (unsigned i = 0; i < privateDnsStatus.numServers; i++) {
+ for (int i = 0; i < privateDnsStatus.numServers; i++) {
std::string tlsServer_str = addrToString(&(privateDnsStatus.serverStatus[i].ss));
tlsServers->push_back(std::move(tlsServer_str));
}
@@ -418,7 +418,7 @@
} else {
dw.println("Private DNS configuration (%u entries)", privateDnsStatus.numServers);
dw.incIndent();
- for (unsigned i = 0; i < privateDnsStatus.numServers; i++) {
+ for (int i = 0; i < privateDnsStatus.numServers; i++) {
dw.println("%s name{%s} status{%s}",
addrToString(&(privateDnsStatus.serverStatus[i].ss)).c_str(),
privateDnsStatus.serverStatus[i].hostname,
diff --git a/server/TetherController.cpp b/server/TetherController.cpp
index 62dcbbf..7d7be58 100644
--- a/server/TetherController.cpp
+++ b/server/TetherController.cpp
@@ -325,11 +325,10 @@
// dnsmasq can't parse commands larger than this due to the fixed-size buffer
// in check_android_listeners(). The receiving buffer is 1024 bytes long, but
// dnsmasq reads up to 1023 bytes.
-#define MAX_CMD_SIZE 1023
+const size_t MAX_CMD_SIZE = 1023;
+// TODO: convert callers to the overload taking a vector<string>
int TetherController::setDnsForwarders(unsigned netId, char **servers, int numServers) {
- int i;
-
Fwmark fwmark;
fwmark.netId = netId;
fwmark.explicitlySelected = true;
@@ -339,7 +338,7 @@
std::string daemonCmd = StringPrintf("update_dns%s0x%x", SEPARATOR, fwmark.intValue);
mDnsForwarders.clear();
- for (i = 0; i < numServers; i++) {
+ for (int i = 0; i < numServers; i++) {
ALOGD("setDnsForwarders(0x%x %d = '%s')", fwmark.intValue, i, servers[i]);
addrinfo *res, hints = { .ai_flags = AI_NUMERICHOST };
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index c4029f0..d7c449e 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -940,7 +940,6 @@
for (size_t i = 0; i < std::size(kTestData); i++) {
const auto& td = kTestData[i];
-
const binder::Status status =
mNetd->setProcSysNet(td.ipversion, td.which, td.ifname, td.parameter, td.value);