dt_fd_forward: fix uses of uninit values
`NetworkToHost(x)` where `x` is uninitialized (or isn't fully written
with bytes) angers our static analyzer, and may be easy to accidentally
use going forward. If we rephrase `HandleResult` to instead take a
callable type that produces a T, we can sidestep all of this.
Caught by the static analyzer:
> art/dt_fd_forward/dt_fd_forward.cc:554:30: warning: 1st function call
argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
> art/dt_fd_forward/dt_fd_forward.cc:563:30: warning: 1st function call
argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
> art/dt_fd_forward/dt_fd_forward.cc:572:30: warning: 1st function call
argument is an uninitialized value [clang-analyzer-core.CallAndMessage]
Bug: None
Test: TreeHugger, `WITH_TIDY=1 CLANG_ANALYZER_CHECKS=1 mma`
Change-Id: Id759bd36085bdd34399b64b08363b4682746fdfa
diff --git a/dt_fd_forward/dt_fd_forward.cc b/dt_fd_forward/dt_fd_forward.cc
index 2a6bfda..0ff8770 100644
--- a/dt_fd_forward/dt_fd_forward.cc
+++ b/dt_fd_forward/dt_fd_forward.cc
@@ -506,14 +506,24 @@
pkt_->type.cmd.data = ReadRemaining();
}
- template <typename T>
- T HandleResult(IOResult res, T val, T fail) {
+ // `produceVal` is a function which produces the success value. It'd be a bit
+ // syntactically simpler to simply take a `T success`, but doing so invites
+ // the possibility of operating on uninitalized data, since we often want to
+ // either return the failure value, or return a massaged version of what we
+ // read off the wire, e.g.,
+ //
+ // ```
+ // IOResult res = transport->ReadFully(&out, sizeof(out));
+ // return HandleResult(res, fail, [&] { return SomeTransform(&out); });
+ // ```
+ template <typename T, typename Fn>
+ T HandleResult(IOResult res, T fail, Fn produceVal) {
switch (res) {
case IOResult::kError:
is_err_ = true;
return fail;
case IOResult::kOk:
- return val;
+ return produceVal();
case IOResult::kEOF:
is_eof_ = true;
pkt_->type.cmd.len = 0;
@@ -537,7 +547,7 @@
} else {
out = reinterpret_cast<jbyte*>(transport_->Alloc(rem));
IOResult res = transport_->ReadFully(out, rem);
- jbyte* ret = HandleResult(res, out, static_cast<jbyte*>(nullptr));
+ jbyte* ret = HandleResult(res, static_cast<jbyte*>(nullptr), [&] { return out; });
if (ret != out) {
transport_->Free(out);
}
@@ -551,7 +561,7 @@
}
jbyte out;
IOResult res = transport_->ReadFully(&out, sizeof(out));
- return HandleResult(res, NetworkToHost(out), static_cast<jbyte>(-1));
+ return HandleResult(res, static_cast<jbyte>(-1), [&] { return NetworkToHost(out); });
}
jshort ReadInt16() {
@@ -560,7 +570,7 @@
}
jshort out;
IOResult res = transport_->ReadFully(&out, sizeof(out));
- return HandleResult(res, NetworkToHost(out), static_cast<jshort>(-1));
+ return HandleResult(res, static_cast<jshort>(-1), [&] { return NetworkToHost(out); });
}
jint ReadInt32() {
@@ -569,7 +579,7 @@
}
jint out;
IOResult res = transport_->ReadFully(&out, sizeof(out));
- return HandleResult(res, NetworkToHost(out), -1);
+ return HandleResult(res, -1, [&] { return NetworkToHost(out); });
}
FdForwardTransport* transport_;