Move BufferHubMetadata off pdx::fileHandle
Use android::base::unique_fd to replace LocalHandle. For BorrowedHandle,
a const reference of the unique_fd is returned.
Updated the test cases to fit in current behavior.
Test: BufferHubMetadata_test, BufferHubBuffer_test, BufferNode_test,
buffer_hub-test (passed)
Fix: 118888072
Change-Id: I34de335ed9a10864ac226cd4d7d261ba0078045d
diff --git a/libs/ui/BufferHubBuffer.cpp b/libs/ui/BufferHubBuffer.cpp
index 8cc1a4e..dd79775 100644
--- a/libs/ui/BufferHubBuffer.cpp
+++ b/libs/ui/BufferHubBuffer.cpp
@@ -36,10 +36,12 @@
#include <private/dvr/bufferhub_rpc.h>
#pragma clang diagnostic pop
-#include <ui/BufferHubBuffer.h>
-
#include <poll.h>
+#include <android-base/unique_fd.h>
+#include <ui/BufferHubBuffer.h>
+
+using android::base::unique_fd;
using android::dvr::BufferTraits;
using android::dvr::DetachedBufferRPC;
using android::dvr::NativeHandleWrapper;
@@ -132,7 +134,9 @@
const int bufferId = bufferTraits.id();
// Import the metadata.
- mMetadata = BufferHubMetadata::Import(bufferTraits.take_metadata_handle());
+ LocalHandle metadataHandle = bufferTraits.take_metadata_handle();
+ unique_fd metadataFd(metadataHandle.Release());
+ mMetadata = BufferHubMetadata::Import(std::move(metadataFd));
if (!mMetadata.IsValid()) {
ALOGE("BufferHubBuffer::ImportGraphicBuffer: invalid metadata.");
diff --git a/libs/ui/BufferHubMetadata.cpp b/libs/ui/BufferHubMetadata.cpp
index 1e08ed1..18d9a2c 100644
--- a/libs/ui/BufferHubMetadata.cpp
+++ b/libs/ui/BufferHubMetadata.cpp
@@ -48,47 +48,47 @@
return {};
}
- // Hand over the ownership of the fd to a pdx::LocalHandle immediately after the successful
- // return of ashmem_create_region. The ashmemHandle is going to own the fd and to prevent fd
+ // Hand over the ownership of the fd to a unique_fd immediately after the successful
+ // return of ashmem_create_region. The ashmemFd is going to own the fd and to prevent fd
// leaks during error handling.
- pdx::LocalHandle ashmemHandle{fd};
+ unique_fd ashmemFd{fd};
- if (ashmem_set_prot_region(ashmemHandle.Get(), kAshmemProt) != 0) {
+ if (ashmem_set_prot_region(ashmemFd.get(), kAshmemProt) != 0) {
ALOGE("BufferHubMetadata::Create: failed to set protect region.");
return {};
}
- return BufferHubMetadata::Import(std::move(ashmemHandle));
+ return BufferHubMetadata::Import(std::move(ashmemFd));
}
/* static */
-BufferHubMetadata BufferHubMetadata::Import(pdx::LocalHandle ashmemHandle) {
- if (!ashmem_valid(ashmemHandle.Get())) {
+BufferHubMetadata BufferHubMetadata::Import(unique_fd ashmemFd) {
+ if (!ashmem_valid(ashmemFd.get())) {
ALOGE("BufferHubMetadata::Import: invalid ashmem fd.");
return {};
}
- size_t metadataSize = static_cast<size_t>(ashmem_get_size_region(ashmemHandle.Get()));
+ size_t metadataSize = static_cast<size_t>(ashmem_get_size_region(ashmemFd.get()));
size_t userMetadataSize = metadataSize - kMetadataHeaderSize;
// Note that here the buffer state is mapped from shared memory as an atomic object. The
// std::atomic's constructor will not be called so that the original value stored in the memory
// region can be preserved.
auto metadataHeader = static_cast<MetadataHeader*>(mmap(nullptr, metadataSize, kAshmemProt,
- MAP_SHARED, ashmemHandle.Get(),
+ MAP_SHARED, ashmemFd.get(),
/*offset=*/0));
if (metadataHeader == nullptr) {
ALOGE("BufferHubMetadata::Import: failed to map region.");
return {};
}
- return BufferHubMetadata(userMetadataSize, std::move(ashmemHandle), metadataHeader);
+ return BufferHubMetadata(userMetadataSize, std::move(ashmemFd), metadataHeader);
}
-BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle,
+BufferHubMetadata::BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd,
MetadataHeader* metadataHeader)
: mUserMetadataSize(userMetadataSize),
- mAshmemHandle(std::move(ashmemHandle)),
+ mAshmemFd(std::move(ashmemFd)),
mMetadataHeader(metadataHeader) {}
BufferHubMetadata::~BufferHubMetadata() {
diff --git a/libs/ui/include/ui/BufferHubMetadata.h b/libs/ui/include/ui/BufferHubMetadata.h
index 94a9000..4261971 100644
--- a/libs/ui/include/ui/BufferHubMetadata.h
+++ b/libs/ui/include/ui/BufferHubMetadata.h
@@ -33,12 +33,17 @@
#pragma clang diagnostic ignored "-Wundefined-func-template"
#pragma clang diagnostic ignored "-Wunused-template"
#pragma clang diagnostic ignored "-Wweak-vtables"
-#include <pdx/file_handle.h>
#include <private/dvr/buffer_hub_defs.h>
#pragma clang diagnostic pop
+#include <android-base/unique_fd.h>
+
namespace android {
+namespace {
+using base::unique_fd;
+} // namespace
+
class BufferHubMetadata {
public:
// Creates a new BufferHubMetadata backed by an ashmem region.
@@ -50,11 +55,8 @@
// Imports an existing BufferHubMetadata from an ashmem FD.
//
- // TODO(b/112338294): Refactor BufferHub to use Binder as its internal IPC backend instead of
- // UDS.
- //
- // @param ashmemHandle Ashmem file handle representing an ashmem region.
- static BufferHubMetadata Import(pdx::LocalHandle ashmemHandle);
+ // @param ashmemFd Ashmem file descriptor representing an ashmem region.
+ static BufferHubMetadata Import(unique_fd ashmemFd);
BufferHubMetadata() = default;
@@ -67,7 +69,7 @@
mUserMetadataSize = other.mUserMetadataSize;
other.mUserMetadataSize = 0;
- mAshmemHandle = std::move(other.mAshmemHandle);
+ mAshmemFd = std::move(other.mAshmemFd);
// The old raw mMetadataHeader pointer must be cleared, otherwise the destructor will
// automatically mummap() the shared memory.
@@ -79,25 +81,25 @@
// Returns true if the metadata is valid, i.e. the metadata has a valid ashmem fd and the ashmem
// has been mapped into virtual address space.
- bool IsValid() const { return mAshmemHandle.IsValid() && mMetadataHeader != nullptr; }
+ bool IsValid() const { return mAshmemFd.get() != -1 && mMetadataHeader != nullptr; }
size_t user_metadata_size() const { return mUserMetadataSize; }
size_t metadata_size() const {
return mUserMetadataSize + dvr::BufferHubDefs::kMetadataHeaderSize;
}
- const pdx::LocalHandle& ashmem_handle() const { return mAshmemHandle; }
+ const unique_fd& ashmem_fd() const { return mAshmemFd; }
dvr::BufferHubDefs::MetadataHeader* metadata_header() { return mMetadataHeader; }
private:
- BufferHubMetadata(size_t userMetadataSize, pdx::LocalHandle ashmemHandle,
+ BufferHubMetadata(size_t userMetadataSize, unique_fd ashmemFd,
dvr::BufferHubDefs::MetadataHeader* metadataHeader);
BufferHubMetadata(const BufferHubMetadata&) = delete;
void operator=(const BufferHubMetadata&) = delete;
size_t mUserMetadataSize = 0;
- pdx::LocalHandle mAshmemHandle;
+ unique_fd mAshmemFd;
dvr::BufferHubDefs::MetadataHeader* mMetadataHeader = nullptr;
};
diff --git a/libs/ui/tests/BufferHubMetadata_test.cpp b/libs/ui/tests/BufferHubMetadata_test.cpp
index 4209392..70f86b3 100644
--- a/libs/ui/tests/BufferHubMetadata_test.cpp
+++ b/libs/ui/tests/BufferHubMetadata_test.cpp
@@ -43,11 +43,11 @@
EXPECT_TRUE(m1.IsValid());
EXPECT_NE(m1.metadata_header(), nullptr);
- pdx::LocalHandle h2 = m1.ashmem_handle().Duplicate();
- EXPECT_TRUE(h2.IsValid());
+ unique_fd h2 = unique_fd(dup(m1.ashmem_fd().get()));
+ EXPECT_NE(h2.get(), -1);
BufferHubMetadata m2 = BufferHubMetadata::Import(std::move(h2));
- EXPECT_FALSE(h2.IsValid());
+ EXPECT_EQ(h2.get(), -1);
EXPECT_TRUE(m1.IsValid());
BufferHubDefs::MetadataHeader* mh1 = m1.metadata_header();
EXPECT_NE(mh1, nullptr);
@@ -71,31 +71,29 @@
BufferHubMetadata m1 = BufferHubMetadata::Create(sizeof(int));
EXPECT_TRUE(m1.IsValid());
EXPECT_NE(m1.metadata_header(), nullptr);
- EXPECT_TRUE(m1.ashmem_handle().IsValid());
+ EXPECT_NE(m1.ashmem_fd().get(), -1);
EXPECT_EQ(m1.user_metadata_size(), sizeof(int));
BufferHubMetadata m2 = std::move(m1);
- // After the move, the metadata header (a raw pointer) should be reset in the
- // older buffer.
+ // After the move, the metadata header (a raw pointer) should be reset in the older buffer.
EXPECT_EQ(m1.metadata_header(), nullptr);
EXPECT_NE(m2.metadata_header(), nullptr);
- EXPECT_FALSE(m1.ashmem_handle().IsValid());
- EXPECT_TRUE(m2.ashmem_handle().IsValid());
+ EXPECT_EQ(m1.ashmem_fd().get(), -1);
+ EXPECT_NE(m2.ashmem_fd().get(), -1);
EXPECT_EQ(m1.user_metadata_size(), 0U);
EXPECT_EQ(m2.user_metadata_size(), sizeof(int));
BufferHubMetadata m3{std::move(m2)};
- // After the move, the metadata header (a raw pointer) should be reset in the
- // older buffer.
+ // After the move, the metadata header (a raw pointer) should be reset in the older buffer.
EXPECT_EQ(m2.metadata_header(), nullptr);
EXPECT_NE(m3.metadata_header(), nullptr);
- EXPECT_FALSE(m2.ashmem_handle().IsValid());
- EXPECT_TRUE(m3.ashmem_handle().IsValid());
+ EXPECT_EQ(m2.ashmem_fd().get(), -1);
+ EXPECT_NE(m3.ashmem_fd().get(), -1);
EXPECT_EQ(m2.user_metadata_size(), 0U);
EXPECT_EQ(m3.user_metadata_size(), sizeof(int));