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/libnetdutils/include/netdutils/Status.h b/libnetdutils/include/netdutils/Status.h
index 6104a38..2433670 100644
--- a/libnetdutils/include/netdutils/Status.h
+++ b/libnetdutils/include/netdutils/Status.h
@@ -28,13 +28,16 @@
// or moderate performance code. This can definitely be improved but
// for now short string optimization is expected to keep the common
// success case fast.
-class Status {
+//
+// Status is implicitly movable via the default noexcept move constructor
+// and noexcept move-assignment operator.
+class [[nodiscard]] Status {
public:
Status() = default;
-
explicit Status(int code) : mCode(code) {}
- Status(int code, const std::string& msg) : mCode(code), mMsg(msg) { assert(!ok()); }
+ // Constructs an error Status, |code| must be non-zero.
+ Status(int code, std::string msg) : mCode(code), mMsg(std::move(msg)) { assert(!ok()); }
int code() const { return mCode; }
@@ -42,6 +45,9 @@
const std::string& msg() const { return mMsg; }
+ // Explicitly ignores the Status without triggering [[nodiscard]] errors.
+ void ignoreError() const {}
+
bool operator==(const Status& other) const { return code() == other.code(); }
bool operator!=(const Status& other) const { return !(*this == other); }
@@ -67,10 +73,14 @@
return status.ok();
}
-// Document that status is expected to be ok. This function may log
-// (or assert when running in debug mode) if status has an unexpected
-// value.
-void expectOk(const Status& status);
+// For use only in tests.
+#define EXPECT_OK(status) EXPECT_TRUE((status).ok())
+
+// Documents that status is expected to be ok. This function may log
+// (or assert when running in debug mode) if status has an unexpected value.
+inline void expectOk(const Status& /*status*/) {
+ // TODO: put something here, for now this function serves solely as documentation.
+}
// Convert POSIX errno to a Status object.
// If Status is extended to have more features, this mapping may