Check for ignored Status results
This change makes error-checking more explicit for anything that
returns a Status object and adds a grep-friendly ignoreError() method
for those situations where you really didn't care.
The C++17 [[nodiscard]] attribute is equivalent to the pre-existing
GNU extension __attribute__((warn_unused_result)).
A number of sites where errors were previously being ignored had to be
adjusted to either ignore the error explicitly, handling it or
propagating it to the caller. The latter two actions could potentially
introduce regressions, so please double-check in case I guessed wrong.
Finally, I added a few tests to verify that Status objects can be
cheaply moved around without copying the embedded std::string object,
since this happens at every function call boundary.
Test: atest netd_integration_test
Test: atest netd_unit_test
Test: atest netd_benchmark netdutils_test
Test: m netd ndc bpfloader libnetd_client
Change-Id: I1015c916bf6c6e3b038a683a2a126a01417d3a1c
diff --git a/server/TrafficController.cpp b/server/TrafficController.cpp
index b686ae8..325ebaf 100644
--- a/server/TrafficController.cpp
+++ b/server/TrafficController.cpp
@@ -147,13 +147,6 @@
ebpfSupported = hasBpfSupport();
}
-Status initialOwnerMap(BpfMap<uint32_t, uint8_t>& map) {
- map.clear();
- uint32_t mapSettingKey = CONFIGURATION_KEY;
- uint8_t defaultMapState = 0;
- return map.writeValue(mapSettingKey, defaultMapState, BPF_NOEXIST);
-}
-
Status TrafficController::initMaps() {
std::lock_guard ownerMapGuard(mOwnerMatchMutex);
RETURN_IF_NOT_OK(
@@ -203,7 +196,7 @@
RETURN_IF_NOT_OK(
mUidOwnerMap.getOrCreate(UID_OWNER_MAP_SIZE, UID_OWNER_MAP_PATH, BPF_MAP_TYPE_HASH));
RETURN_IF_NOT_OK(changeOwnerAndMode(UID_OWNER_MAP_PATH, AID_ROOT, "UidOwnerMap", true));
- mUidOwnerMap.clear();
+ RETURN_IF_NOT_OK(mUidOwnerMap.clear());
return netdutils::status::ok;
}
@@ -213,7 +206,7 @@
return netdutils::status::ok;
}
- /* When netd restart from a crash without total system reboot, the program
+ /* When netd restarts from a crash without total system reboot, the program
* is still attached to the cgroup, detach it so the program can be freed
* and we can load and attach new program into the target cgroup.
*
@@ -231,7 +224,6 @@
addInterface(ifacePair.first.c_str(), ifacePair.second);
}
-
auto result = makeSkDestroyListener();
if (!isOk(result)) {
ALOGE("Unable to create SkDestroyListener: %s", toString(result).c_str());
@@ -242,13 +234,17 @@
const auto rxHandler = [this](const nlmsghdr&, const Slice msg) {
inet_diag_msg diagmsg = {};
if (extract(msg, diagmsg) < sizeof(inet_diag_msg)) {
- ALOGE("unrecognized netlink message: %s", toString(msg).c_str());
+ ALOGE("Unrecognized netlink message: %s", toString(msg).c_str());
return;
}
uint64_t sock_cookie = static_cast<uint64_t>(diagmsg.id.idiag_cookie[0]) |
(static_cast<uint64_t>(diagmsg.id.idiag_cookie[1]) << 32);
- mCookieTagMap.deleteValue(sock_cookie);
+ Status s = mCookieTagMap.deleteValue(sock_cookie);
+ if (!isOk(s)) {
+ ALOGE("Failed to delete cookie %" PRIx64 ": %s", sock_cookie, toString(s).c_str());
+ return;
+ }
};
expectOk(mSkDestroyListener->subscribe(kSockDiagMsgType, rxHandler));
@@ -345,7 +341,6 @@
}
int TrafficController::deleteTagData(uint32_t tag, uid_t uid) {
-
if (!ebpfSupported) {
if (legacy_deleteTagData(tag, uid)) return -errno;
return 0;
@@ -365,7 +360,7 @@
// Move forward to next cookie in the map.
return netdutils::status::ok;
};
- mCookieTagMap.iterateWithValue(deleteMatchedCookieEntries);
+ mCookieTagMap.iterateWithValue(deleteMatchedCookieEntries).ignoreError();
// Now we go through the Tag stats map and delete the data entry with correct uid and tag
// combination. Or all tag stats under that uid if the target tag is 0.
const auto deleteMatchedUidTagEntries = [uid, tag](const StatsKey& key,
@@ -381,7 +376,7 @@
}
return netdutils::status::ok;
};
- mTagStatsMap.iterate(deleteMatchedUidTagEntries);
+ mTagStatsMap.iterate(deleteMatchedUidTagEntries).ignoreError();
// If the tag is not zero, we already deleted all the data entry required. If tag is 0, we also
// need to delete the stats stored in uidStatsMap and counterSet map.
if (tag != 0) return 0;
@@ -391,7 +386,7 @@
ALOGE("Failed to delete counterSet data(uid=%u, tag=%u): %s\n", uid, tag,
strerror(res.code()));
}
- mUidStatsMap.iterate(deleteMatchedUidTagEntries);
+ mUidStatsMap.iterate(deleteMatchedUidTagEntries).ignoreError();
auto deleteAppUidStatsEntry = [uid](const uint32_t& key, BpfMap<uint32_t, StatsValue>& map) {
if (key == uid) {
@@ -403,7 +398,7 @@
}
return netdutils::status::ok;
};
- mAppUidStatsMap.iterate(deleteAppUidStatsEntry);
+ mAppUidStatsMap.iterate(deleteAppUidStatsEntry).ignoreError();
return 0;
}
@@ -471,7 +466,7 @@
auto oldMatch = map.readValue(uid);
if (isOk(oldMatch)) {
uint8_t newMatch = oldMatch.value() | match;
- map.writeValue((uint32_t) uid, newMatch, BPF_ANY);
+ RETURN_IF_NOT_OK(map.writeValue((uint32_t) uid, newMatch, BPF_ANY));
} else {
RETURN_IF_NOT_OK(map.writeValue(uid, match, BPF_ANY));
}