ion: remove IS_ERR_OR_NULL

IS_ERR_OR_NULL is often part of a bad pattern that can accidentally
return 0 on error:
if (IS_ERR_OR_NULL(ptr))
    return PTR_ERR(ptr);

It also usually means that the errors of a function are not well
defined.  Replace all uses in ion.c by ensure that the return
type of any function in ion is an ERR_PTR.

Specify that the expected return value from map_kernel or map_dma
heap ops is ERR_PTR, and warn if a heap returns NULL.

Change-Id: I6e7ea0d2e62fa08d4e372a7ef6da649f7a62289c
Signed-off-by: Colin Cross <ccross@android.com>

diff --git a/drivers/gpu/ion/ion.c b/drivers/gpu/ion/ion.c
index 48652ddd49489a15c9700568869e9ab87084b555..d6700d0eb3fa4474a84acc2e74439cc4a8298ef1 100644 (file)
--- a/drivers/gpu/ion/ion.c
+++ b/drivers/gpu/ion/ion.c
@@ -197,7 +197,9 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
        buffer->size = len;
 
        table = heap->ops->map_dma(heap, buffer);
-       if (IS_ERR_OR_NULL(table)) {
+       if (WARN_ONCE(table == NULL, "heap->ops->map_dma should return ERR_PTR on error"))
+               table = ERR_PTR(-EINVAL);
+       if (IS_ERR(table)) {
                heap->ops->free(buffer);
                kfree(buffer);
                return ERR_PTR(PTR_ERR(table));
@@ -389,7 +391,7 @@ static struct ion_handle *ion_handle_lookup(struct ion_client *client,
                if (handle->buffer == buffer)
                        return handle;
        }
-       return NULL;
+       return ERR_PTR(-EINVAL);
 }
 
 static bool ion_handle_validate(struct ion_client *client, struct ion_handle *handle)
@@ -459,7 +461,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
                if (!((1 << heap->id) & heap_id_mask))
                        continue;
                buffer = ion_buffer_create(heap, dev, len, align, flags);
-               if (!IS_ERR_OR_NULL(buffer))
+               if (!IS_ERR(buffer))
                        break;
        }
        up_read(&dev->lock);
@@ -543,7 +545,9 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
                return buffer->vaddr;
        }
        vaddr = buffer->heap->ops->map_kernel(buffer->heap, buffer);
-       if (IS_ERR_OR_NULL(vaddr))
+       if (WARN_ONCE(vaddr == NULL, "heap->ops->map_kernel should return ERR_PTR on error"))
+               return ERR_PTR(-EINVAL);
+       if (IS_ERR(vaddr))
                return vaddr;
        buffer->vaddr = vaddr;
        buffer->kmap_cnt++;
@@ -560,7 +564,7 @@ static void *ion_handle_kmap_get(struct ion_handle *handle)
                return buffer->vaddr;
        }
        vaddr = ion_buffer_kmap_get(buffer);
-       if (IS_ERR_OR_NULL(vaddr))
+       if (IS_ERR(vaddr))
                return vaddr;
        handle->kmap_cnt++;
        return vaddr;
@@ -954,8 +958,6 @@ static int ion_dma_buf_begin_cpu_access(struct dma_buf *dmabuf, size_t start,
        mutex_unlock(&buffer->lock);
        if (IS_ERR(vaddr))
                return PTR_ERR(vaddr);
-       if (!vaddr)
-               return -ENOMEM;
        return 0;
 }
 
@@ -1034,7 +1036,7 @@ struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd)
        struct ion_handle *handle;
 
        dmabuf = dma_buf_get(fd);
-       if (IS_ERR_OR_NULL(dmabuf))
+       if (IS_ERR(dmabuf))
                return ERR_PTR(PTR_ERR(dmabuf));
        /* if this memory came from ion */
 
@@ -1049,12 +1051,12 @@ struct ion_handle *ion_import_dma_buf(struct ion_client *client, int fd)
        mutex_lock(&client->lock);
        /* if a handle exists for this buffer just take a reference to it */
        handle = ion_handle_lookup(client, buffer);
-       if (!IS_ERR_OR_NULL(handle)) {
+       if (!IS_ERR(handle)) {
                ion_handle_get(handle);
                goto end;
        }
        handle = ion_handle_create(client, buffer);
-       if (IS_ERR_OR_NULL(handle))
+       if (IS_ERR(handle))
                goto end;
        ion_handle_add(client, handle);
 end:
@@ -1070,7 +1072,7 @@ static int ion_sync_for_device(struct ion_client *client, int fd)
        struct ion_buffer *buffer;
 
        dmabuf = dma_buf_get(fd);
-       if (IS_ERR_OR_NULL(dmabuf))
+       if (IS_ERR(dmabuf))
                return PTR_ERR(dmabuf);
 
        /* if this memory came from ion */
@@ -1204,7 +1206,7 @@ static int ion_open(struct inode *inode, struct file *file)
 
        pr_debug("%s: %d\n", __func__, __LINE__);
        client = ion_client_create(dev, "user");
-       if (IS_ERR_OR_NULL(client))
+       if (IS_ERR(client))
                return PTR_ERR(client);
        file->private_data = client;
 
@@ -1400,7 +1402,7 @@ struct ion_device *ion_device_create(long (*custom_ioctl)
        }
 
        idev->debug_root = debugfs_create_dir("ion", NULL);
-       if (IS_ERR_OR_NULL(idev->debug_root))
+       if (!idev->debug_root)
                pr_err("ion: failed to create debug files.\n");
 
        idev->custom_ioctl = custom_ioctl;

diff --git a/drivers/gpu/ion/ion_priv.h b/drivers/gpu/ion/ion_priv.h
index 1155994fc8ec5f39d05790d47468283d8fa3a4b6..32461e94673561cc82021e152ed85b83a23cb041 100644 (file)
--- a/drivers/gpu/ion/ion_priv.h
+++ b/drivers/gpu/ion/ion_priv.h
@@ -93,6 +93,9 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
  * @map_kernel         map memory to the kernel
  * @unmap_kernel       unmap memory to the kernel
  * @map_user           map memory to userspace
+ *
+ * allocate, phys, and map_user return 0 on success, -errno on error.
+ * map_dma and map_kernel return pointer on success, ERR_PTR on error.
  */
 struct ion_heap_ops {
        int (*allocate) (struct ion_heap *heap,