From 115d446fcb03d0d2e336e99cb2d90b323a0444bd Mon Sep 17 00:00:00 2001 From: Jeremy Gebben Date: Thu, 26 Jan 2012 12:33:18 -0700 Subject: [PATCH] libgralloc: fix libgralloc error codes Several ioctl calls were returning -1 instead of -errno, which may confuse upper layers, especially when the error is -ENOMEM. Also move some existing "err = -errno" calls to the top of if statements so that errno cannot change by libc calls in the error handling code. (cherry picked from commit a8eda532452651eb1fbae419319455de2a078ef0) Change-Id: I181f98d5a261e8e3e1b3f6ecd3ba288e7b4b5607 --- libgralloc/ionalloc.cpp | 31 ++++++++++++++++--------------- libgralloc/pmemalloc.cpp | 40 ++++++++++++++++++++++++---------------- 2 files changed, 40 insertions(+), 31 deletions(-) diff --git a/libgralloc/ionalloc.cpp b/libgralloc/ionalloc.cpp index ee14e90..dbd8546 100644 --- a/libgralloc/ionalloc.cpp +++ b/libgralloc/ionalloc.cpp @@ -56,7 +56,7 @@ int IonAlloc::open_device() void IonAlloc::close_device() { - if(mIonFd > 0) + if(mIonFd >= 0) close(mIonFd); mIonFd = FD_INIT; } @@ -95,8 +95,8 @@ int IonAlloc::alloc_buffer(alloc_data& data) iFd = mIonFd; } - err = ioctl(iFd, ION_IOC_ALLOC, &ionAllocData); - if(err) { + if(ioctl(iFd, ION_IOC_ALLOC, &ionAllocData)) { + err = -errno; LOGE("ION_IOC_ALLOC failed with error - %s", strerror(errno)); close_device(); if(ionSyncFd >= 0) @@ -107,8 +107,8 @@ int IonAlloc::alloc_buffer(alloc_data& data) fd_data.handle = ionAllocData.handle; handle_data.handle = ionAllocData.handle; - err = ioctl(iFd, ION_IOC_MAP, &fd_data); - if(err) { + if(ioctl(iFd, ION_IOC_MAP, &fd_data)) { + err = -errno; LOGE("%s: ION_IOC_MAP failed with error - %s", __FUNCTION__, strerror(errno)); ioctl(mIonFd, ION_IOC_FREE, &handle_data); @@ -124,9 +124,9 @@ int IonAlloc::alloc_buffer(alloc_data& data) base = mmap(0, ionAllocData.len, PROT_READ|PROT_WRITE, MAP_SHARED, fd_data.fd, 0); if(base == MAP_FAILED) { + err = -errno; LOGE("%s: Failed to map the allocated memory: %s", __FUNCTION__, strerror(errno)); - err = -errno; ioctl(mIonFd, ION_IOC_FREE, &handle_data); close_device(); ionSyncFd = FD_INIT; @@ -147,7 +147,7 @@ int IonAlloc::alloc_buffer(alloc_data& data) ioctl(mIonFd, ION_IOC_FREE, &handle_data); LOGD("ion: Allocated buffer base:%p size:%d fd:%d", data.base, ionAllocData.len, data.fd); - return err; + return 0; } @@ -180,9 +180,9 @@ int IonAlloc::map_buffer(void **pBase, size_t size, int offset, int fd) MAP_SHARED, fd, 0); *pBase = base; if(base == MAP_FAILED) { + err = -errno; LOGD("ion: Failed to map memory in the client: %s", strerror(errno)); - err = -errno; } else { LOGD("ion: Mapped buffer base:%p size:%d offset:%d fd:%d", base, size, offset, fd); @@ -193,8 +193,9 @@ int IonAlloc::map_buffer(void **pBase, size_t size, int offset, int fd) int IonAlloc::unmap_buffer(void *base, size_t size, int offset) { LOGD("ion: Unmapping buffer base:%p size:%d", base, size); - int err = munmap(base, size); - if(err) { + int err = 0; + if(munmap(base, size)) { + err = -errno; LOGE("ion: Failed to unmap memory at %p : %s", base, strerror(errno)); } @@ -214,8 +215,8 @@ int IonAlloc::clean_buffer(void *base, size_t size, int offset, int fd) return err; fd_data.fd = fd; - err = ioctl(mIonFd, ION_IOC_IMPORT, &fd_data); - if(err) { + if (ioctl(mIonFd, ION_IOC_IMPORT, &fd_data)) { + err = -errno; LOGE("%s: ION_IOC_IMPORT failed with error - %s", __FUNCTION__, strerror(errno)); close_device(); @@ -227,8 +228,8 @@ int IonAlloc::clean_buffer(void *base, size_t size, int offset, int fd) flush_data.vaddr = base; flush_data.offset = offset; flush_data.length = size; - err = ioctl(mIonFd, ION_IOC_CLEAN_INV_CACHES, &flush_data); - if(err) { + if(ioctl(mIonFd, ION_IOC_CLEAN_INV_CACHES, &flush_data)) { + err = -errno; LOGE("%s: ION_IOC_CLEAN_INV_CACHES failed with error - %s", __FUNCTION__, strerror(errno)); ioctl(mIonFd, ION_IOC_FREE, &handle_data); @@ -236,6 +237,6 @@ int IonAlloc::clean_buffer(void *base, size_t size, int offset, int fd) return err; } ioctl(mIonFd, ION_IOC_FREE, &handle_data); - return err; + return 0; } diff --git a/libgralloc/pmemalloc.cpp b/libgralloc/pmemalloc.cpp index 19a2ea8..0bf85e8 100644 --- a/libgralloc/pmemalloc.cpp +++ b/libgralloc/pmemalloc.cpp @@ -47,8 +47,8 @@ static int getPmemTotalSize(int fd, size_t* size) //XXX: 7x27 int err = 0; pmem_region region; - err = ioctl(fd, PMEM_GET_TOTAL_SIZE, ®ion); - if (err == 0) { + if (ioctl(fd, PMEM_GET_TOTAL_SIZE, ®ion)) { + err = -errno; *size = region.len; } return err; @@ -63,24 +63,32 @@ static int getOpenFlags(bool uncached) } static int connectPmem(int fd, int master_fd) { - return ioctl(fd, PMEM_CONNECT, master_fd); + if (ioctl(fd, PMEM_CONNECT, master_fd)) + return -errno; + return 0; } static int mapSubRegion(int fd, int offset, size_t size) { struct pmem_region sub = { offset, size }; - return ioctl(fd, PMEM_MAP, &sub); + if (ioctl(fd, PMEM_MAP, &sub)) + return -errno; + return 0; } static int unmapSubRegion(int fd, int offset, size_t size) { struct pmem_region sub = { offset, size }; - return ioctl(fd, PMEM_UNMAP, &sub); + if (ioctl(fd, PMEM_UNMAP, &sub)) + return -errno; + return 0; } static int alignPmem(int fd, size_t size, int align) { struct pmem_allocation allocation; allocation.size = size; allocation.align = align; - return ioctl(fd, PMEM_ALLOCATE_ALIGNED, &allocation); + if (ioctl(fd, PMEM_ALLOCATE_ALIGNED, &allocation)) + return -errno; + return 0; } static int cleanPmem(void *base, size_t size, int offset, int fd) { @@ -88,7 +96,9 @@ static int cleanPmem(void *base, size_t size, int offset, int fd) { pmem_addr.vaddr = (unsigned long) base; pmem_addr.offset = offset; pmem_addr.length = size; - return ioctl(fd, PMEM_CLEAN_INV_CACHES, &pmem_addr); + if (ioctl(fd, PMEM_CLEAN_INV_CACHES, &pmem_addr)) + return -errno; + return 0; } //-------------- PmemUserspaceAlloc-----------------------// @@ -123,9 +133,9 @@ int PmemUserspaceAlloc::init_pmem_area_locked() void* base = mmap(0, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); if (base == MAP_FAILED) { + err = -errno; LOGE("%s: Failed to map pmem master fd: %s", mPmemDev, strerror(errno)); - err = -errno; base = 0; close(fd); fd = -1; @@ -134,9 +144,9 @@ int PmemUserspaceAlloc::init_pmem_area_locked() mMasterBase = base; } } else { + err = -errno; LOGE("%s: Failed to open pmem device: %s", mPmemDev, strerror(errno)); - err = -errno; } return err; } @@ -193,7 +203,6 @@ int PmemUserspaceAlloc::alloc_buffer(alloc_data& data) if (err < 0) { LOGE("%s: Failed to initialize pmem sub-heap: %d", mPmemDev, err); - err = -errno; close(fd); mAllocator->deallocate(offset); fd = -1; @@ -245,9 +254,9 @@ int PmemUserspaceAlloc::map_buffer(void **pBase, size_t size, int offset, int fd MAP_SHARED, fd, 0); *pBase = base; if(base == MAP_FAILED) { + err = -errno; LOGE("%s: Failed to map buffer size:%d offset:%d fd:%d Error: %s", mPmemDev, size, offset, fd, strerror(errno)); - err = -errno; } else { LOGD("%s: Mapped buffer base:%p size:%d offset:%d fd:%d", mPmemDev, base, size, offset, fd); @@ -265,10 +274,10 @@ int PmemUserspaceAlloc::unmap_buffer(void *base, size_t size, int offset) LOGD("%s: Unmapping buffer base:%p size:%d offset:%d", mPmemDev , base, size, offset); if (munmap(base, size) < 0) { + err = -errno; LOGE("%s: Failed to unmap memory at %p :%s", mPmemDev, base, strerror(errno)); - err = -errno; } return err; @@ -314,9 +323,9 @@ int PmemKernelAlloc::alloc_buffer(alloc_data& data) } void* base = mmap(0, size, PROT_READ|PROT_WRITE, MAP_SHARED, fd, 0); if (base == MAP_FAILED) { + err = -errno; LOGE("%s: failed to map pmem fd: %s", mPmemDev, strerror(errno)); - err = -errno; close(fd); return err; } @@ -348,9 +357,9 @@ int PmemKernelAlloc::map_buffer(void **pBase, size_t size, int offset, int fd) MAP_SHARED, fd, 0); *pBase = base; if(base == MAP_FAILED) { + err = -errno; LOGE("%s: Failed to map memory in the client: %s", mPmemDev, strerror(errno)); - err = -errno; } else { LOGD("%s: Mapped buffer base:%p size:%d, fd:%d", mPmemDev, base, size, fd); @@ -362,8 +371,7 @@ int PmemKernelAlloc::map_buffer(void **pBase, size_t size, int offset, int fd) int PmemKernelAlloc::unmap_buffer(void *base, size_t size, int offset) { int err = 0; - munmap(base, size); - if (err < 0) { + if (munmap(base, size)) { err = -errno; LOGW("%s: Error unmapping memory at %p: %s", mPmemDev, base, strerror(err));