commit af4e43cc5b2eacbfe600f9ab0e459f09c35c0c3f
author Yifan Hong <elsk@google.com> Fri Mar 03 19:10:52 2017 -0800
committer Yifan Hong <elsk@google.com> Fri Mar 03 19:48:36 2017 -0800
tree c484f7f824a56bd7a0d4650b1db94703bff53e5b
parent 6558c77eb11d68069d0c374c39c0f955294f14b8

return_status::checkStatus => assertOk

checkStatus doesn't really set mCheckStatus to true, and hence is
a misnomer. Change it to assertOk (correspond to isOk()).

And hence the move assignment operator should not call checkStatus
(because it is valid to move into a Return<T> object that !isOk()
but has mCheckedStatus to true), because caller is fully aware of
the error that will be overwritten.

Test: libhidl_test
Change-Id: I86a44967b68619d1467d2cc9caaa39124b156121

base/Status.cpp

diff --git a/base/Status.cpp b/base/Status.cpp
index 8367fb8..e7320a8 100644
--- a/base/Status.cpp
+++ b/base/Status.cpp
@@ -98,7 +98,7 @@
 }
 
 namespace details {
-    void return_status::checkStatus() const {
+    void return_status::assertOk() const {
         if (!isOk()) {
             LOG(FATAL) << "Attempted to retrieve value from failed HIDL call: " << description();
         }
@@ -110,6 +110,16 @@
             LOG(FATAL) << "Failed HIDL return status not checked: " << description();
         }
     }
+
+    return_status &return_status::operator=(return_status &&other) {
+        if (!mCheckedStatus && !isOk()) {
+            LOG(FATAL) << "Failed HIDL return status not checked: " << description();
+        }
+        std::swap(mStatus, other.mStatus);
+        std::swap(mCheckedStatus, other.mCheckedStatus);
+        return *this;
+    }
+
 }  // namespace details
 
 }  // namespace hardware

base/include/hidl/Status.h

diff --git a/base/include/hidl/Status.h b/base/include/hidl/Status.h
index 2f26ffa..c5c5bd9 100644
--- a/base/include/hidl/Status.h
+++ b/base/include/hidl/Status.h
@@ -147,7 +147,7 @@
         Status mStatus {};
         mutable bool mCheckedStatus = false;
     protected:
-        void checkStatus() const;
+        void assertOk() const;
     public:
         return_status() {}
         return_status(Status s) : mStatus(s) {}
@@ -158,12 +158,7 @@
         return_status(return_status &&other) {
             *this = std::move(other);
         }
-        return_status &operator=(return_status &&other) {
-            checkStatus();
-            std::swap(mStatus, other.mStatus);
-            std::swap(mCheckedStatus, other.mCheckedStatus);
-            return *this;
-        }
+        return_status &operator=(return_status &&other);
 
         ~return_status();
 
@@ -196,7 +191,7 @@
     ~Return() = default;
 
     operator T() const {
-        checkStatus();
+        assertOk();
         return mVal;
     }
 
@@ -222,7 +217,7 @@
     ~Return() = default;
 
     operator sp<T>() const {
-        checkStatus();
+        assertOk();
         return mVal;
     }
 };