commit 0f3461db91b75d9acd7e0bec8b1e97cbb3bafdf7
author Jeff Tinker <jtinker@google.com> Tue Jan 03 10:40:55 2017 -0800
committer Steven Moreland <smoreland@google.com> Thu Jan 05 12:06:25 2017 -0800
tree 5b8b47c7b0682d1f1eb5502785c2418665dc8017
parent d272cb9e881c0aa23e4964f101131ab68affd85d

Permit assignment from a null hidl_memory

Bug: 34102220
Test: libhidl_test, hidl_test

Change-Id: Ic0da09f0f6a79643ee44c6b13ef606b2217956e0

Android.bp

diff --git a/Android.bp b/Android.bp
index 5fe6ad5..d44a5e7 100644
--- a/Android.bp
+++ b/Android.bp
@@ -30,6 +30,7 @@
         "libhwbinder",
         "liblog",
         "libutils",
+        "libcutils",
     ],
     static_libs: ["libgtest"],
 

base/include/hidl/HidlSupport.h

diff --git a/base/include/hidl/HidlSupport.h b/base/include/hidl/HidlSupport.h
index 1b0184a..78bc2c8 100644
--- a/base/include/hidl/HidlSupport.h
+++ b/base/include/hidl/HidlSupport.h
@@ -226,8 +226,15 @@
     // copy assignment
     hidl_memory &operator=(const hidl_memory &other) {
         if (this != &other) {
-            mOwnsHandle = true;
-            mHandle = native_handle_clone(other.mHandle);
+            cleanup();
+
+            if (other.mHandle == nullptr) {
+                mHandle = nullptr;
+                mOwnsHandle = false;
+            } else {
+                mOwnsHandle = true;
+                mHandle = native_handle_clone(other.mHandle);
+            }
             mSize = other.mSize;
             mName = other.mName;
         }
@@ -238,10 +245,7 @@
     // TODO move constructor/move assignment
 
     ~hidl_memory() {
-        // TODO if we had previously mapped from this object, unmap
-        if (mOwnsHandle) {
-            native_handle_close(mHandle);
-        }
+        cleanup();
     }
 
     const native_handle_t* handle() const {
@@ -260,11 +264,19 @@
     static const size_t kOffsetOfHandle;
     // offsetof(hidl_memory, mName) exposed since mHandle is private.
     static const size_t kOffsetOfName;
+
 private:
     bool mOwnsHandle;
     hidl_handle mHandle;
     size_t mSize;
     hidl_string mName;
+
+    void cleanup() {
+        // TODO(b/33812533): native_handle_delete
+        if (mOwnsHandle && mHandle != nullptr) {
+            native_handle_close(mHandle);
+        }
+    }
 };
 
 ////////////////////////////////////////////////////////////////////////////////

test_main.cpp

diff --git a/test_main.cpp b/test_main.cpp
index bdf4b49..5349fc8 100644
--- a/test_main.cpp
+++ b/test_main.cpp
@@ -110,6 +110,30 @@
     EXPECT_FALSE(hs1 == stringNE);
 }
 
+TEST_F(LibHidlTest, MemoryTest) {
+    using android::hardware::hidl_memory;
+
+    hidl_memory mem1 = hidl_memory(); // default constructor
+    hidl_memory mem2 = mem1; // copy constructor (nullptr)
+
+    EXPECT_EQ(nullptr, mem2.handle());
+
+    native_handle_t* testHandle = native_handle_create(0 /* numInts */, 0 /* numFds */);
+
+    hidl_memory mem3 = hidl_memory("foo", testHandle, 42 /* size */); // owns testHandle
+    hidl_memory mem4 = mem3; // copy constructor (regular handle)
+
+    EXPECT_EQ(mem3.name(), mem4.name());
+    EXPECT_EQ(mem3.size(), mem4.size());
+    EXPECT_NE(nullptr, mem4.handle());
+    EXPECT_NE(mem3.handle(), mem4.handle()); // check handle cloned
+
+    hidl_memory mem5 = hidl_memory("foo", nullptr, 0); // hidl memory works with nullptr handle
+    hidl_memory mem6 = mem5;
+    EXPECT_EQ(nullptr, mem5.handle());
+    EXPECT_EQ(nullptr, mem6.handle());
+}
+
 TEST_F(LibHidlTest, VecInitTest) {
     using android::hardware::hidl_vec;
     using std::vector;