gralloc4: Only free allocated handles in error path am: c8ee87ed1c

Original change: https://googleplex-android-review.googlesource.com/c/platform/hardware/google/gchips/+/24437750

Change-Id: I798dbc6850c2a8fc441d90572f06b0f824e0fd8d
Signed-off-by: Automerger Merge Worker <android-build-automerger-merge-worker@system.gserviceaccount.com>

2 files changed, 39 insertions, 39 deletions

diff --git a/gralloc4/src/allocator/mali_gralloc_ion.cpp b/gralloc4/src/allocator/mali_gralloc_ion.cpp
index da9aba5..2d63a70 100644
--- a/gralloc4/src/allocator/mali_gralloc_ion.cpp
+++ b/gralloc4/src/allocator/mali_gralloc_ion.cpp
@@ -48,6 +48,7 @@
 #include "mali_gralloc_ion.h"
 
 #include <array>
+#include <cassert>
 #include <string>
 
 static const char kDmabufSensorDirectHeapName[] = "sensor_direct_heap";
@@ -369,61 +370,57 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors,
 	unsigned int priv_heap_flag = 0;
 	uint64_t usage;
 	uint32_t i;
-	int fds[MAX_FDS];
-	std::fill(fds, fds + MAX_FDS, -1);
 
 	for (i = 0; i < numDescriptors; i++)
 	{
-		buffer_descriptor_t *bufDescriptor = (buffer_descriptor_t *)(descriptors[i]);
-		usage = bufDescriptor->consumer_usage | bufDescriptor->producer_usage;
-
-		for (int fidx = 0; fidx < bufDescriptor->fd_count; fidx++)
-		{
-			if (ion_fd >= 0 && fidx == 0) {
-				fds[fidx] = ion_fd;
-			} else {
-				fds[fidx] = alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], bufDescriptor->name);
-			}
-			if (fds[fidx] < 0)
-			{
-				MALI_GRALLOC_LOGE("ion_alloc failed");
-
-				for (int cidx = 0; cidx < fidx; cidx++)
-				{
-					close(fds[cidx]);
-				}
-
-				/* need to free already allocated memory. not just this one */
-				mali_gralloc_ion_free_internal(pHandle, numDescriptors);
-
-				return -1;
-			}
-		}
+		buffer_descriptor_t *bufDescriptor = reinterpret_cast<buffer_descriptor_t *>(descriptors[i]);
+		assert(bufDescriptor);
+		assert(bufDescriptor->fd_count >= 0);
+		assert(bufDescriptor->fd_count <= MAX_FDS);
 
-		private_handle_t *hnd = new private_handle_t(
+		auto hnd = new private_handle_t(
 		    priv_heap_flag,
 		    bufDescriptor->alloc_sizes,
 		    bufDescriptor->consumer_usage, bufDescriptor->producer_usage,
-		    fds, bufDescriptor->fd_count,
+		    nullptr, bufDescriptor->fd_count,
 		    bufDescriptor->hal_format, bufDescriptor->alloc_format,
 		    bufDescriptor->width, bufDescriptor->height, bufDescriptor->pixel_stride,
 		    bufDescriptor->layer_count, bufDescriptor->plane_info);
 
-		if (NULL == hnd)
+		/* Reset the number of valid filedescriptors, we will increment
+		 * it each time a valid fd is added, so we can rely on the
+		 * cleanup functions to close open fds. */
+		hnd->set_numfds(0);
+
+		if (nullptr == hnd)
 		{
 			MALI_GRALLOC_LOGE("Private handle could not be created for descriptor:%d in non-shared usecase", i);
+			mali_gralloc_ion_free_internal(pHandle, i);
+			return -1;
+		}
+
+		pHandle[i] = hnd;
+		usage = bufDescriptor->consumer_usage | bufDescriptor->producer_usage;
+
+		for (uint32_t fidx = 0; fidx < bufDescriptor->fd_count; fidx++)
+		{
+			int& fd = hnd->fds[fidx];
 
-			/* Close the obtained shared file descriptor for the current handle */
-			for (int j = 0; j < bufDescriptor->fd_count; j++)
+			if (ion_fd >= 0 && fidx == 0) {
+				fd = ion_fd;
+			} else {
+				fd = alloc_from_dmabuf_heap(usage, bufDescriptor->alloc_sizes[fidx], bufDescriptor->name);
+			}
+
+			if (fd < 0)
 			{
-				close(fds[j]);
+				MALI_GRALLOC_LOGE("ion_alloc failed for fds[%u] = %d", fidx, fd);
+				mali_gralloc_ion_free_internal(pHandle, i + 1);
+				return -1;
 			}
 
-			mali_gralloc_ion_free_internal(pHandle, numDescriptors);
-			return -1;
+			hnd->incr_numfds(1);
 		}
-
-		pHandle[i] = hnd;
 	}
 
 #if defined(GRALLOC_INIT_AFBC) && (GRALLOC_INIT_AFBC == 1)
@@ -431,8 +428,8 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors,
 	unsigned char *cpu_ptr = NULL;
 	for (i = 0; i < numDescriptors; i++)
 	{
-		buffer_descriptor_t *bufDescriptor = (buffer_descriptor_t *)(descriptors[i]);
-		private_handle_t *hnd = (private_handle_t *)(pHandle[i]);
+		buffer_descriptor_t *bufDescriptor = reinterpret_cast<buffer_descriptor_t *>(descriptors[i]);
+		const private_handle_t *hnd = static_cast<const private_handle_t *>(pHandle[i]);
 
 		usage = bufDescriptor->consumer_usage | bufDescriptor->producer_usage;
 
@@ -459,6 +456,7 @@ int mali_gralloc_ion_allocate(const gralloc_buffer_descriptor_t *descriptors,
 				ATRACE_NAME("data init");
 				/* For separated plane YUV, there is a header to initialise per plane. */
 				const plane_info_t *plane_info = bufDescriptor->plane_info;
+				assert(plane_info);
 				const bool is_multi_plane = hnd->is_multi_plane();
 				for (int i = 0; i < MAX_PLANES && (i == 0 || plane_info[i].byte_stride != 0); i++)
 				{
diff --git a/gralloc4/src/mali_gralloc_buffer.h b/gralloc4/src/mali_gralloc_buffer.h
index 9cc7920..c4db9fc 100644
--- a/gralloc4/src/mali_gralloc_buffer.h
+++ b/gralloc4/src/mali_gralloc_buffer.h
@@ -286,6 +286,8 @@ struct private_handle_t
 
 		if (_fds)
 			memcpy(fds, _fds, sizeof(fds));
+		else
+			memset(fds, -1, sizeof(fds));
 
 		if (_alloc_sizes)
 			memcpy(alloc_sizes, _alloc_sizes, sizeof(alloc_sizes));