commit e817f39795930dad84218cfb5c76932b29906d3b
author Ytai Ben-Tsvi <ytai@google.com> Tue Oct 20 09:16:19 2020 -0700
committer Ytai Ben-Tsvi <ytai@google.com> Tue Oct 20 16:36:15 2020 -0700
tree 7878422a4291c6ff6c878cba114bb678719a9408
parent 9bcf4989bf3e09167503e94571b15626e56f4453

Remove null support in SharedFileRegion

ParcelFileDescriptor does not support null (negative) file descriptors
so we remove it from SharedFileRegion and expect that a @nullable
SharedFileRegion is used for cases where a null file descriptor is
allowed.

Test: atest shmemTest
Change-Id: Icf226181cf6de9759ed3145ff6b5ad0d8fd650af

media/libshmem/ShmemTest.cpp

diff --git a/media/libshmem/ShmemTest.cpp b/media/libshmem/ShmemTest.cpp
index 4f11b51..d076ad0 100644
--- a/media/libshmem/ShmemTest.cpp
+++ b/media/libshmem/ShmemTest.cpp
@@ -17,6 +17,7 @@
 
 #include "binder/MemoryBase.h"
 #include "binder/MemoryHeapBase.h"
+#include "cutils/ashmem.h"
 #include "media/ShmemCompat.h"
 #include "media/ShmemUtil.h"
 
@@ -24,11 +25,22 @@
 namespace media {
 namespace {
 
-// Creates a SharedFileRegion instance with a null FD.
+// Creates a SharedFileRegion instance.
 SharedFileRegion makeSharedFileRegion(int64_t offset, int64_t size) {
     SharedFileRegion shmem;
     shmem.offset = offset;
     shmem.size = size;
+    int fd = ashmem_create_region("", size + offset);
+    assert(fd >= 0);
+    shmem.fd = os::ParcelFileDescriptor(base::unique_fd(fd));
+    return shmem;
+}
+
+// Creates a SharedFileRegion instance with an invalid FD.
+SharedFileRegion makeInvalidSharedFileRegion(int64_t offset, int64_t size) {
+    SharedFileRegion shmem;
+    shmem.offset = offset;
+    shmem.size = size;
     return shmem;
 }
 
@@ -46,9 +58,7 @@
     EXPECT_TRUE(validateSharedFileRegion(makeSharedFileRegion(1, 2)));
     EXPECT_FALSE(validateSharedFileRegion(makeSharedFileRegion(-1, 2)));
     EXPECT_FALSE(validateSharedFileRegion(makeSharedFileRegion(2, -1)));
-    EXPECT_TRUE(validateSharedFileRegion(makeSharedFileRegion(
-            std::numeric_limits<int64_t>::max(),
-            std::numeric_limits<int64_t>::max())));
+    EXPECT_FALSE(validateSharedFileRegion(makeInvalidSharedFileRegion(1, 2)));
 }
 
 TEST(ShmemTest, Conversion) {
@@ -72,12 +82,11 @@
 TEST(ShmemTest, NullConversion) {
     sp<IMemory> reconstructed;
     {
-        SharedFileRegion shmem;
+        std::optional<SharedFileRegion> shmem;
         sp<IMemory> imem;
-        ASSERT_TRUE(convertIMemoryToSharedFileRegion(imem, &shmem));
-        ASSERT_EQ(0, shmem.size);
-        ASSERT_LT(shmem.fd.get(), 0);
-        ASSERT_TRUE(convertSharedFileRegionToIMemory(shmem, &reconstructed));
+        ASSERT_TRUE(convertNullableIMemoryToSharedFileRegion(imem, &shmem));
+        ASSERT_FALSE(shmem.has_value());
+        ASSERT_TRUE(convertNullableSharedFileRegionToIMemory(shmem, &reconstructed));
     }
     ASSERT_EQ(nullptr, reconstructed);
 }