diff options
author | Steven Moreland <smoreland@google.com> | 2019-03-05 19:37:51 -0800 |
---|---|---|
committer | Steven Moreland <smoreland@google.com> | 2019-03-06 18:32:06 +0000 |
commit | 23a901c9d4c8dd5a83bd10332f10a166db78320a (patch) | |
tree | 9ed626a610a4ece6d9e4d3ce1cfcdb04afd257b3 /base | |
parent | 00891fe1d449c581f3a11e54682b3550cb5a67ac (diff) | |
download | libhidl-23a901c9d4c8dd5a83bd10332f10a166db78320a.tar.gz |
Add setProcessHidlReturnRestriction.
For critical processes (e.g. init/hwservicemanager), it's nice to check
that every error is checked. If these processes restart, it may be hard
to debug the system.
Bug: 124861676
Bug: 121004730
Test: use w/ hwservicemanager
Change-Id: I0d340c31e392bfb86a188dab902e6d20fa836814
Diffstat (limited to 'base')
-rw-r--r-- | base/Status.cpp | 21 | ||||
-rw-r--r-- | base/include/hidl/Status.h | 23 |
2 files changed, 41 insertions, 3 deletions
diff --git a/base/Status.cpp b/base/Status.cpp index 5a4c918..90474a0 100644 --- a/base/Status.cpp +++ b/base/Status.cpp @@ -18,6 +18,7 @@ #include <android-base/logging.h> #include <hidl/Status.h> +#include <utils/CallStack.h> #include <unordered_map> @@ -142,6 +143,11 @@ std::ostream& operator<< (std::ostream& stream, const Status& s) { return stream; } +static HidlReturnRestriction gReturnRestriction = HidlReturnRestriction::NONE; +void setProcessHidlReturnRestriction(HidlReturnRestriction restriction) { + gReturnRestriction = restriction; +} + namespace details { void return_status::assertOk() const { if (!isOk()) { @@ -151,9 +157,22 @@ namespace details { return_status::~return_status() { // mCheckedStatus must be checked before isOk since isOk modifies mCheckedStatus - if (!mCheckedStatus && !isOk()) { + if (mCheckedStatus) return; + + if (!isOk()) { LOG(FATAL) << "Failed HIDL return status not checked: " << description(); } + + if (gReturnRestriction == HidlReturnRestriction::NONE) { + return; + } + + if (gReturnRestriction == HidlReturnRestriction::ERROR_IF_UNCHECKED) { + LOG(ERROR) << "Failed to check status of HIDL Return."; + CallStack::logStack("unchecked HIDL return", CallStack::getCurrent(10).get(), ANDROID_LOG_ERROR); + } else { + LOG(FATAL) << "Failed to check status of HIDL Return."; + } } return_status& return_status::operator=(return_status&& other) noexcept { diff --git a/base/include/hidl/Status.h b/base/include/hidl/Status.h index b69d4e4..29434f2 100644 --- a/base/include/hidl/Status.h +++ b/base/include/hidl/Status.h @@ -139,9 +139,8 @@ namespace details { template <typename T, typename U> friend Return<U> StatusOf(const Return<T> &other); - protected: - void assertOk() const; public: + void assertOk() const; return_status() {} return_status(const Status& s) : mStatus(s) {} @@ -185,6 +184,26 @@ namespace details { }; } // namespace details +enum class HidlReturnRestriction { + // Okay to ignore checking transport errors. This would instead rely on init to reset state + // after an error in the underlying transport. This is the default and expected for most + // usecases. + NONE, + // Log when there is an unchecked error. + ERROR_IF_UNCHECKED, + // Fatal when there is an unchecked error. + FATAL_IF_UNCHECKED, +}; + +/** + * This should be called during process initialization (e.g. before binder threadpool is created). + * + * Note: default of HidlReturnRestriction::NONE should be good for most usecases. See above. + * + * The restriction will be applied when Return objects are deconstructed. + */ +void setProcessHidlReturnRestriction(HidlReturnRestriction restriction); + template<typename T> class Return : public details::return_status { private: T mVal {}; |