[PATCH v2 0/4] drivers: staging: checkpatch fixes

[PATCH v2 0/4] drivers: staging: checkpatch fixes

[Tobin C Harding]

drivers/staging/android/ion/ion.c produces 2 warnings and 29 checks.

This patch set fixes both warnings and all but 4 of the checks.  All
except the last patch are trivial.

Signed-off-by: Tobin C Harding <me@tobin.cc>
---

Patch 1/4 is whitespace only.

This is my first Linux kernel patch.

thanks

Tobin C Harding (4):
  staging: android: ion: fixed checkpatch alignment warnings
  staging: android: ion: fixed checkpatch long line warnings
  staging: android: ion: fixed checkpatch comparison to NULL warnings
  staging: android: ion: changed BUG_ON to WARN[_ON]

 drivers/staging/android/ion/ion.c | 88 +++++++++++++++++++++-------------
 1 file changed, 48 insertions(+), 40 deletions(-)

-- 
2.7.4

[PATCH v2 1/4] drivers: staging: fix parameter alignment

[Tobin C Harding]

drivers/staging/android/ion/ion.c checkpatch produces alignment checks.

This patch is whitespace only and fixes these checks.

Signed-off-by: Tobin C Harding <me@tobin.cc>
---
 drivers/staging/android/ion/ion.c | 64 +++++++++++++++++++--------------------
 1 file changed, 32 insertions(+), 32 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index d4c6207..c4a8aef 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -174,10 +174,10 @@ static void ion_buffer_add(struct ion_device *dev,
 
 /* this function should only be called while dev->lock is held */
 static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
-				     struct ion_device *dev,
-				     unsigned long len,
-				     unsigned long align,
-				     unsigned long flags)
+					    struct ion_device *dev,
+					    unsigned long len,
+					    unsigned long align,
+					    unsigned long flags)
 {
 	struct ion_buffer *buffer;
 	struct sg_table *table;
@@ -210,7 +210,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
 
 	table = heap->ops->map_dma(heap, buffer);
 	if (WARN_ONCE(table == NULL,
-			"heap->ops->map_dma should return ERR_PTR on error"))
+		      "heap->ops->map_dma should return ERR_PTR on error"))
 		table = ERR_PTR(-EINVAL);
 	if (IS_ERR(table)) {
 		ret = -EINVAL;
@@ -337,7 +337,7 @@ static void ion_buffer_remove_from_handle(struct ion_buffer *buffer)
 }
 
 static struct ion_handle *ion_handle_create(struct ion_client *client,
-				     struct ion_buffer *buffer)
+					    struct ion_buffer *buffer)
 {
 	struct ion_handle *handle;
 
@@ -427,7 +427,7 @@ static struct ion_handle *ion_handle_lookup(struct ion_client *client,
 }
 
 static struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
-						int id)
+						      int id)
 {
 	struct ion_handle *handle;
 
@@ -439,7 +439,7 @@ static struct ion_handle *ion_handle_get_by_id_nolock(struct ion_client *client,
 }
 
 struct ion_handle *ion_handle_get_by_id(struct ion_client *client,
-						int id)
+					int id)
 {
 	struct ion_handle *handle;
 
@@ -592,7 +592,7 @@ int ion_phys(struct ion_client *client, struct ion_handle *handle,
 
 	if (!buffer->heap->ops->phys) {
 		pr_err("%s: ion_phys is not implemented by this heap (name=%s, type=%d).\n",
-			__func__, buffer->heap->name, buffer->heap->type);
+		       __func__, buffer->heap->name, buffer->heap->type);
 		mutex_unlock(&client->lock);
 		return -ENODEV;
 	}
@@ -612,7 +612,7 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
 	}
 	vaddr = buffer->heap->ops->map_kernel(buffer->heap, buffer);
 	if (WARN_ONCE(vaddr == NULL,
-			"heap->ops->map_kernel should return ERR_PTR on error"))
+		      "heap->ops->map_kernel should return ERR_PTR on error"))
 		return ERR_PTR(-EINVAL);
 	if (IS_ERR(vaddr))
 		return vaddr;
@@ -781,14 +781,14 @@ static const struct file_operations debug_client_fops = {
 };
 
 static int ion_get_client_serial(const struct rb_root *root,
-					const unsigned char *name)
+				 const unsigned char *name)
 {
 	int serial = -1;
 	struct rb_node *node;
 
 	for (node = rb_first(root); node; node = rb_next(node)) {
 		struct ion_client *client = rb_entry(node, struct ion_client,
-						node);
+						     node);
 
 		if (strcmp(client->name, name))
 			continue;
@@ -863,14 +863,14 @@ struct ion_client *ion_client_create(struct ion_device *dev,
 	rb_insert_color(&client->node, &dev->clients);
 
 	client->debug_root = debugfs_create_file(client->display_name, 0664,
-						dev->clients_debug_root,
-						client, &debug_client_fops);
+						 dev->clients_debug_root,
+						 client, &debug_client_fops);
 	if (!client->debug_root) {
 		char buf[256], *path;
 
 		path = dentry_path(dev->clients_debug_root, buf, 256);
 		pr_err("Failed to create client debugfs at %s/%s\n",
-			path, client->display_name);
+		       path, client->display_name);
 	}
 
 	up_write(&dev->lock);
@@ -958,7 +958,7 @@ static void ion_unmap_dma_buf(struct dma_buf_attachment *attachment,
 }
 
 void ion_pages_sync_for_device(struct device *dev, struct page *page,
-		size_t size, enum dma_data_direction dir)
+			       size_t size, enum dma_data_direction dir)
 {
 	struct scatterlist sg;
 
@@ -998,7 +998,7 @@ static void ion_buffer_sync_for_device(struct ion_buffer *buffer,
 
 		if (ion_buffer_page_is_dirty(page))
 			ion_pages_sync_for_device(dev, ion_buffer_page(page),
-							PAGE_SIZE, dir);
+						  PAGE_SIZE, dir);
 
 		ion_buffer_page_clean(buffer->pages + i);
 	}
@@ -1076,13 +1076,13 @@ static int ion_mmap(struct dma_buf *dmabuf, struct vm_area_struct *vma)
 
 	if (!buffer->heap->ops->map_user) {
 		pr_err("%s: this heap does not define a method for mapping to userspace\n",
-			__func__);
+		       __func__);
 		return -EINVAL;
 	}
 
 	if (ion_buffer_fault_user_mappings(buffer)) {
 		vma->vm_flags |= VM_IO | VM_PFNMAP | VM_DONTEXPAND |
-							VM_DONTDUMP;
+			VM_DONTDUMP;
 		vma->vm_private_data = buffer;
 		vma->vm_ops = &ion_vma_ops;
 		ion_vm_open(vma);
@@ -1167,7 +1167,7 @@ static struct dma_buf_ops dma_buf_ops = {
 };
 
 struct dma_buf *ion_share_dma_buf(struct ion_client *client,
-						struct ion_handle *handle)
+				  struct ion_handle *handle)
 {
 	DEFINE_DMA_BUF_EXPORT_INFO(exp_info);
 	struct ion_buffer *buffer;
@@ -1342,9 +1342,9 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		struct ion_handle *handle;
 
 		handle = ion_alloc(client, data.allocation.len,
-						data.allocation.align,
-						data.allocation.heap_id_mask,
-						data.allocation.flags);
+				   data.allocation.align,
+				   data.allocation.heap_id_mask,
+				   data.allocation.flags);
 		if (IS_ERR(handle))
 			return PTR_ERR(handle);
 
@@ -1403,7 +1403,7 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		if (!dev->custom_ioctl)
 			return -ENOTTY;
 		ret = dev->custom_ioctl(client, data.custom.cmd,
-						data.custom.arg);
+					data.custom.arg);
 		break;
 	}
 	default:
@@ -1528,7 +1528,7 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused)
 	seq_printf(s, "%16s %16zu\n", "total ", total_size);
 	if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
 		seq_printf(s, "%16s %16zu\n", "deferred free",
-				heap->free_list_size);
+			   heap->free_list_size);
 	seq_puts(s, "----------------------------------------------------\n");
 
 	if (heap->debug_show)
@@ -1611,15 +1611,15 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 	plist_node_init(&heap->node, -heap->id);
 	plist_add(&heap->node, &dev->heaps);
 	debug_file = debugfs_create_file(heap->name, 0664,
-					dev->heaps_debug_root, heap,
-					&debug_heap_fops);
+					 dev->heaps_debug_root, heap,
+					 &debug_heap_fops);
 
 	if (!debug_file) {
 		char buf[256], *path;
 
 		path = dentry_path(dev->heaps_debug_root, buf, 256);
 		pr_err("Failed to create heap debugfs at %s/%s\n",
-			path, heap->name);
+		       path, heap->name);
 	}
 
 	if (heap->shrinker.count_objects && heap->shrinker.scan_objects) {
@@ -1634,7 +1634,7 @@ void ion_device_add_heap(struct ion_device *dev, struct ion_heap *heap)
 
 			path = dentry_path(dev->heaps_debug_root, buf, 256);
 			pr_err("Failed to create heap shrinker debugfs at %s/%s\n",
-				path, debug_name);
+			       path, debug_name);
 		}
 	}
 
@@ -1676,7 +1676,7 @@ struct ion_device *ion_device_create(long (*custom_ioctl)
 		goto debugfs_done;
 	}
 	idev->clients_debug_root = debugfs_create_dir("clients",
-						idev->debug_root);
+						      idev->debug_root);
 	if (!idev->clients_debug_root)
 		pr_err("ion: failed to create debugfs clients directory.\n");
 
@@ -1719,13 +1719,13 @@ void __init ion_reserve(struct ion_platform_data *data)
 						    MEMBLOCK_ALLOC_ANYWHERE);
 			if (!paddr) {
 				pr_err("%s: error allocating memblock for heap %d\n",
-					__func__, i);
+				       __func__, i);
 				continue;
 			}
 			data->heaps[i].base = paddr;
 		} else {
 			int ret = memblock_reserve(data->heaps[i].base,
-					       data->heaps[i].size);
+						   data->heaps[i].size);
 			if (ret)
 				pr_err("memblock reserve of %zx@%lx failed\n",
 				       data->heaps[i].size,
-- 
2.7.4

Re: [PATCH v2 1/4] drivers: staging: fix parameter alignment

[Greg KH]

On Mon, Apr 18, 2016 at 10:35:28AM +1000, Tobin C Harding wrote:
> drivers/staging/android/ion/ion.c checkpatch produces alignment checks.
> 
> This patch is whitespace only and fixes these checks.
> 
> Signed-off-by: Tobin C Harding <me@tobin.cc>
> ---
>  drivers/staging/android/ion/ion.c | 64 +++++++++++++++++++--------------------
>  1 file changed, 32 insertions(+), 32 deletions(-)

Please put the driver's name in the subject line to make it easier to
see.  For this one it would be:
	Subject: [PATCH v2 1/4] staging: android: ion: fix parameter alignment

Please fix up and resend the series after dropping the one patch.

thanks,

greg k-h

[PATCH v2 2/4] drivers: staging: fix line length

[Tobin C Harding]

drivers/staging/android/ion/ion.c checkpatch produces line over 80
character warnings.

This patch is whitespace only and fixes these warnings.

Signed-off-by: Tobin C Harding <me@tobin.cc>
---
 drivers/staging/android/ion/ion.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index c4a8aef..4e02209 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -551,7 +551,8 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
 }
 EXPORT_SYMBOL(ion_alloc);
 
-static void ion_free_nolock(struct ion_client *client, struct ion_handle *handle)
+static void ion_free_nolock(struct ion_client *client,
+			    struct ion_handle *handle)
 {
 	bool valid_handle;
 
@@ -1358,7 +1359,8 @@ static long ion_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		struct ion_handle *handle;
 
 		mutex_lock(&client->lock);
-		handle = ion_handle_get_by_id_nolock(client, data.handle.handle);
+		handle = ion_handle_get_by_id_nolock(client,
+						     data.handle.handle);
 		if (IS_ERR(handle)) {
 			mutex_unlock(&client->lock);
 			return PTR_ERR(handle);
-- 
2.7.4

[PATCH v2 3/4] drivers: staging: remove comparison to NULL

[Tobin C Harding]

drivers/staging/android/ion/ion.c checkpatch produces
'Comparison to NULL could be written ...' checks. 

This patch replaces comparison to NULL with !var as suggested by checkpatch.

Signed-off-by: Tobin C Harding <me@tobin.cc>
---
 drivers/staging/android/ion/ion.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 4e02209..234848f 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -209,7 +209,7 @@ static struct ion_buffer *ion_buffer_create(struct ion_heap *heap,
 	buffer->size = len;
 
 	table = heap->ops->map_dma(heap, buffer);
-	if (WARN_ONCE(table == NULL,
+	if (WARN_ONCE(!table,
 		      "heap->ops->map_dma should return ERR_PTR on error"))
 		table = ERR_PTR(-EINVAL);
 	if (IS_ERR(table)) {
@@ -522,7 +522,7 @@ struct ion_handle *ion_alloc(struct ion_client *client, size_t len,
 	}
 	up_read(&dev->lock);
 
-	if (buffer == NULL)
+	if (!buffer)
 		return ERR_PTR(-ENODEV);
 
 	if (IS_ERR(buffer))
@@ -612,7 +612,7 @@ static void *ion_buffer_kmap_get(struct ion_buffer *buffer)
 		return buffer->vaddr;
 	}
 	vaddr = buffer->heap->ops->map_kernel(buffer->heap, buffer);
-	if (WARN_ONCE(vaddr == NULL,
+	if (WARN_ONCE(!vaddr,
 		      "heap->ops->map_kernel should return ERR_PTR on error"))
 		return ERR_PTR(-EINVAL);
 	if (IS_ERR(vaddr))
-- 
2.7.4

[PATCH v2 4/4] drivers: staging: remove BUG_ON

[Tobin C Harding]

drivers/staging/android/ion/ion.c calls BUG_ON in places where WARN_ON will
suffice.

This patch replaces two such occurences. Two other occurences remain.

Signed-off-by: Tobin C Harding <me@tobin.cc>
---

Changing the remaining two BUG_ON's causes changes to the programm logic.

This is my first patch set to the kernel, I am as such, not comfortable
changeing the logic of this file at this early stage of the game. :)

 drivers/staging/android/ion/ion.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 234848f..1e37f52 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -325,7 +325,7 @@ static void ion_buffer_remove_from_handle(struct ion_buffer *buffer)
 	 */
 	mutex_lock(&buffer->lock);
 	buffer->handle_count--;
-	BUG_ON(buffer->handle_count < 0);
+	WARN_ON(buffer->handle_count < 0);
 	if (!buffer->handle_count) {
 		struct task_struct *task;
 
@@ -556,7 +556,10 @@ static void ion_free_nolock(struct ion_client *client,
 {
 	bool valid_handle;
 
-	BUG_ON(client != handle->client);
+	if (client != handle->client) {
+		WARN(1, "%s: client != handle->client.\n", __func__);
+		return;
+	}
 
 	valid_handle = ion_handle_validate(client, handle);
 
@@ -569,7 +572,10 @@ static void ion_free_nolock(struct ion_client *client,
 
 void ion_free(struct ion_client *client, struct ion_handle *handle)
 {
-	BUG_ON(client != handle->client);
+	if (client != handle->client) {
+		WARN(1, "%s: client != handle->client.\n", __func__);
+		return;
+	}
 
 	mutex_lock(&client->lock);
 	ion_free_nolock(client, handle);
-- 
2.7.4

Re: [PATCH v2 4/4] drivers: staging: remove BUG_ON

[Laura Abbott]

On 04/17/2016 05:35 PM, Tobin C Harding wrote:
> drivers/staging/android/ion/ion.c calls BUG_ON in places where WARN_ON will
> suffice.
>
> This patch replaces two such occurences. Two other occurences remain.
>

You got the logic flow correct but I'd prefer to keep the BUG_ON.
It's catching actual bugs in the framework.

> Signed-off-by: Tobin C Harding <me@tobin.cc>
> ---
>
> Changing the remaining two BUG_ON's causes changes to the programm logic.
>
> This is my first patch set to the kernel, I am as such, not comfortable
> changeing the logic of this file at this early stage of the game. :)
>
>   drivers/staging/android/ion/ion.c | 12 +++++++++---
>   1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
> index 234848f..1e37f52 100644
> --- a/drivers/staging/android/ion/ion.c
> +++ b/drivers/staging/android/ion/ion.c
> @@ -325,7 +325,7 @@ static void ion_buffer_remove_from_handle(struct ion_buffer *buffer)
>   	 */
>   	mutex_lock(&buffer->lock);
>   	buffer->handle_count--;
> -	BUG_ON(buffer->handle_count < 0);
> +	WARN_ON(buffer->handle_count < 0);
>   	if (!buffer->handle_count) {
>   		struct task_struct *task;
>
> @@ -556,7 +556,10 @@ static void ion_free_nolock(struct ion_client *client,
>   {
>   	bool valid_handle;
>
> -	BUG_ON(client != handle->client);
> +	if (client != handle->client) {
> +		WARN(1, "%s: client != handle->client.\n", __func__);
> +		return;
> +	}
>
>   	valid_handle = ion_handle_validate(client, handle);
>
> @@ -569,7 +572,10 @@ static void ion_free_nolock(struct ion_client *client,
>
>   void ion_free(struct ion_client *client, struct ion_handle *handle)
>   {
> -	BUG_ON(client != handle->client);
> +	if (client != handle->client) {
> +		WARN(1, "%s: client != handle->client.\n", __func__);
> +		return;
> +	}
>
>   	mutex_lock(&client->lock);
>   	ion_free_nolock(client, handle);
>