Merge "Remove null support in SharedFileRegion" (11b45e9f) · Commits · e / os / android_frameworks_av

media/libshmem/Android.bp

+1 −0

Original line number	Original line	Diff line number	Diff line
	@@ -41,6 +41,7 @@ cc_test {
	srcs: ["ShmemTest.cpp"],		srcs: ["ShmemTest.cpp"],
	shared_libs: [		shared_libs: [
	"libbinder",		"libbinder",
			"libcutils",
	"libshmemcompat",		"libshmemcompat",
	"libshmemutil",		"libshmemutil",
	"libutils",		"libutils",

media/libshmem/ShmemCompat.cpp

+34 −10

Original line number	Original line	Diff line number	Diff line
	@@ -24,15 +24,12 @@ namespace media {

	bool convertSharedFileRegionToIMemory(const SharedFileRegion& shmem,		bool convertSharedFileRegionToIMemory(const SharedFileRegion& shmem,
	sp<IMemory>* result) {		sp<IMemory>* result) {
			assert(result != nullptr);

	if (!validateSharedFileRegion(shmem)) {		if (!validateSharedFileRegion(shmem)) {
	return false;		return false;
	}		}

	if (shmem.fd.get() < 0) {
	*result = nullptr;
	return true;
	}

	// Heap offset and size must be page aligned.		// Heap offset and size must be page aligned.
	const size_t pageSize = getpagesize();		const size_t pageSize = getpagesize();
	const size_t pageMask = ~(pageSize - 1);		const size_t pageMask = ~(pageSize - 1);
	@@ -62,16 +59,19 @@ bool convertSharedFileRegionToIMemory(const SharedFileRegion& shmem,

	bool convertIMemoryToSharedFileRegion(const sp<IMemory>& mem,		bool convertIMemoryToSharedFileRegion(const sp<IMemory>& mem,
	SharedFileRegion* result) {		SharedFileRegion* result) {
			assert(mem != nullptr);
			assert(result != nullptr);

	*result = SharedFileRegion();		*result = SharedFileRegion();
	if (mem == nullptr) {
	return true;
	}

	ssize_t offset;		ssize_t offset;
	size_t size;		size_t size;

	sp<IMemoryHeap> heap = mem->getMemory(&offset, &size);		sp<IMemoryHeap> heap = mem->getMemory(&offset, &size);
	if (heap != nullptr) {		if (size > 0) {
			if (heap == nullptr) {
			return false;
			}
	// Make sure the offset and size do not overflow from int64 boundaries.		// Make sure the offset and size do not overflow from int64 boundaries.
	if (size > std::numeric_limits<int64_t>::max() \|\|		if (size > std::numeric_limits<int64_t>::max() \|\|
	offset > std::numeric_limits<int64_t>::max() \|\|		offset > std::numeric_limits<int64_t>::max() \|\|
	@@ -90,9 +90,33 @@ bool convertIMemoryToSharedFileRegion(const sp<IMemory>& mem,
	result->size = size;		result->size = size;
	result->offset = heap->getOffset() + offset;		result->offset = heap->getOffset() + offset;
	}		}
			return true;
			}

			bool convertNullableSharedFileRegionToIMemory(const std::optional<SharedFileRegion>& shmem,
			sp<IMemory>* result) {
			assert(result != nullptr);

			if (!shmem.has_value()) {
			result->clear();
	return true;		return true;
	}		}

			return convertSharedFileRegionToIMemory(shmem.value(), result);
			}

			bool convertNullableIMemoryToSharedFileRegion(const sp<IMemory>& mem,
			std::optional<SharedFileRegion>* result) {
			assert(result != nullptr);

			if (mem == nullptr) {
			result->reset();
			return true;
			}

			result->emplace();
			return convertIMemoryToSharedFileRegion(mem, &result->value());
			}

	} // namespace media		} // namespace media
	} // namespace android		} // namespace android

media/libshmem/ShmemTest.cpp

+18 −9

Original line number	Original line	Diff line number	Diff line
	@@ -17,6 +17,7 @@

	#include "binder/MemoryBase.h"		#include "binder/MemoryBase.h"
	#include "binder/MemoryHeapBase.h"		#include "binder/MemoryHeapBase.h"
			#include "cutils/ashmem.h"
	#include "media/ShmemCompat.h"		#include "media/ShmemCompat.h"
	#include "media/ShmemUtil.h"		#include "media/ShmemUtil.h"

	@@ -24,8 +25,19 @@ namespace android {
	namespace media {		namespace media {
	namespace {		namespace {

	// Creates a SharedFileRegion instance with a null FD.		// Creates a SharedFileRegion instance.
	SharedFileRegion makeSharedFileRegion(int64_t offset, int64_t size) {		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;		SharedFileRegion shmem;
	shmem.offset = offset;		shmem.offset = offset;
	shmem.size = size;		shmem.size = size;
	@@ -46,9 +58,7 @@ TEST(ShmemTest, Validate) {
	EXPECT_TRUE(validateSharedFileRegion(makeSharedFileRegion(1, 2)));		EXPECT_TRUE(validateSharedFileRegion(makeSharedFileRegion(1, 2)));
	EXPECT_FALSE(validateSharedFileRegion(makeSharedFileRegion(-1, 2)));		EXPECT_FALSE(validateSharedFileRegion(makeSharedFileRegion(-1, 2)));
	EXPECT_FALSE(validateSharedFileRegion(makeSharedFileRegion(2, -1)));		EXPECT_FALSE(validateSharedFileRegion(makeSharedFileRegion(2, -1)));
	EXPECT_TRUE(validateSharedFileRegion(makeSharedFileRegion(		EXPECT_FALSE(validateSharedFileRegion(makeInvalidSharedFileRegion(1, 2)));
	std::numeric_limits<int64_t>::max(),
	std::numeric_limits<int64_t>::max())));
	}		}

	TEST(ShmemTest, Conversion) {		TEST(ShmemTest, Conversion) {
	@@ -72,12 +82,11 @@ TEST(ShmemTest, Conversion) {
	TEST(ShmemTest, NullConversion) {		TEST(ShmemTest, NullConversion) {
	sp<IMemory> reconstructed;		sp<IMemory> reconstructed;
	{		{
	SharedFileRegion shmem;		std::optional<SharedFileRegion> shmem;
	sp<IMemory> imem;		sp<IMemory> imem;
	ASSERT_TRUE(convertIMemoryToSharedFileRegion(imem, &shmem));		ASSERT_TRUE(convertNullableIMemoryToSharedFileRegion(imem, &shmem));
	ASSERT_EQ(0, shmem.size);		ASSERT_FALSE(shmem.has_value());
	ASSERT_LT(shmem.fd.get(), 0);		ASSERT_TRUE(convertNullableSharedFileRegionToIMemory(shmem, &reconstructed));
	ASSERT_TRUE(convertSharedFileRegionToIMemory(shmem, &reconstructed));
	}		}
	ASSERT_EQ(nullptr, reconstructed);		ASSERT_EQ(nullptr, reconstructed);
	}		}

media/libshmem/ShmemUtil.cpp

+5 −0

Original line number	Original line	Diff line number	Diff line
	@@ -19,6 +19,11 @@ namespace android {
	namespace media {		namespace media {

	bool validateSharedFileRegion(const SharedFileRegion& shmem) {		bool validateSharedFileRegion(const SharedFileRegion& shmem) {
			// FD must be valid.
			if (shmem.fd.get() < 0) {
			return false;
			}

	// Size and offset must be non-negative.		// Size and offset must be non-negative.
	if (shmem.size < 0 \|\| shmem.offset < 0) {		if (shmem.size < 0 \|\| shmem.offset < 0) {
	return false;		return false;

media/libshmem/aidl/android/media/SharedFileRegion.aidl

+4 −2

Original line number	Original line	Diff line number	Diff line
	@@ -20,13 +20,15 @@ package android.media;
	* A shared file region.		* A shared file region.
	*		*
	* This type contains the required information to share a region of a file between processes over		* This type contains the required information to share a region of a file between processes over
	* AIDL. An invalid (null) region may be represented using a negative file descriptor.		* AIDL.
			* An instance of this type represents a valid FD. For representing a null SharedFileRegion, use a
			* @nullable SharedFileRegion.
	* Primarily, this is intended for shared memory blocks.		* Primarily, this is intended for shared memory blocks.
	*		*
	* @hide		* @hide
	*/		*/
	parcelable SharedFileRegion {		parcelable SharedFileRegion {
	/** File descriptor of the region. */		/** File descriptor of the region. Must be valid. */
	ParcelFileDescriptor fd;		ParcelFileDescriptor fd;
	/** Offset, in bytes within the file of the start of the region. Must be non-negative. */		/** Offset, in bytes within the file of the start of the region. Must be non-negative. */
	long offset;		long offset;