>From aa06cc53c7214a044fbc220872aa6210c09608d3 Mon Sep 17 00:00:00 2001
From: Daeseok Youn <[email protected]>
Date: Mon, 10 Feb 2014 14:27:40 +0900
Subject: [PATCH] staging : ion : Fix some checkpatch warnings and an error
Warning:
- Unnecessary space after function pointer name
- Prefer seq_puts to seq_printf
- quoted string split across lines
Error:
- return is not a function, parentheses are not required
Signed-off-by: Daeseok Youn <[email protected]>
---
drivers/staging/android/ion/ion.c | 19 +++++++++----------
drivers/staging/android/ion/ion_priv.h | 16 ++++++++--------
2 files changed, 17 insertions(+), 18 deletions(-)
diff --git a/drivers/staging/android/ion/ion.c b/drivers/staging/android/ion/ion.c
index 574066f..d81f231 100644
--- a/drivers/staging/android/ion/ion.c
+++ b/drivers/staging/android/ion/ion.c
@@ -55,7 +55,7 @@ struct ion_device {
struct mutex buffer_lock;
struct rw_semaphore lock;
struct plist_head heaps;
- long (*custom_ioctl) (struct ion_client *client, unsigned int cmd,
+ long (*custom_ioctl)(struct ion_client *client, unsigned int cmd,
unsigned long arg);
struct rb_root clients;
struct dentry *debug_root;
@@ -429,7 +429,7 @@ static bool ion_handle_validate(struct ion_client *client,
struct ion_handle *handle)
{
WARN_ON(!mutex_is_locked(&client->lock));
- return (idr_find(&client->idr, handle->id) == handle);
+ return idr_find(&client->idr, handle->id) == handle;
}
static int ion_handle_add(struct ion_client *client, struct ion_handle *handle)
@@ -1338,7 +1338,7 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused)
size_t total_orphaned_size = 0;
seq_printf(s, "%16.s %16.s %16.s\n", "client", "pid", "size");
- seq_printf(s, "----------------------------------------------------\n");
+ seq_puts(s, "----------------------------------------------------\n");
for (n = rb_first(&dev->clients); n; n = rb_next(n)) {
struct ion_client *client = rb_entry(n, struct ion_client,
@@ -1357,9 +1357,9 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused)
client->pid, size);
}
}
- seq_printf(s, "----------------------------------------------------\n");
- seq_printf(s, "orphaned allocations (info is from last known client):"
- "\n");
+ seq_puts(s, "----------------------------------------------------\n");
+ seq_puts(s, "orphaned allocations (info is from last known client):\n");
+
mutex_lock(&dev->buffer_lock);
for (n = rb_first(&dev->buffers); n; n = rb_next(n)) {
struct ion_buffer *buffer = rb_entry(n, struct ion_buffer,
@@ -1376,14 +1376,14 @@ static int ion_debug_heap_show(struct seq_file *s, void *unused)
}
}
mutex_unlock(&dev->buffer_lock);
- seq_printf(s, "----------------------------------------------------\n");
+ seq_puts(s, "----------------------------------------------------\n");
seq_printf(s, "%16.s %16zu\n", "total orphaned",
total_orphaned_size);
seq_printf(s, "%16.s %16zu\n", "total ", total_size);
if (heap->flags & ION_HEAP_FLAG_DEFER_FREE)
seq_printf(s, "%16.s %16zu\n", "deferred free",
heap->free_list_size);
- seq_printf(s, "----------------------------------------------------\n");
+ seq_puts(s, "----------------------------------------------------\n");
if (heap->debug_show)
heap->debug_show(heap, s, unused);
@@ -1527,8 +1527,7 @@ void __init ion_reserve(struct ion_platform_data *data)
data->heaps[i].align,
MEMBLOCK_ALLOC_ANYWHERE);
if (!paddr) {
- pr_err("%s: error allocating memblock for "
- "heap %d\n",
+ pr_err("%s: error allocating memblock for heap %d\n",
__func__, i);
continue;
}
diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
index d986739..10f315a 100644
--- a/drivers/staging/android/ion/ion_priv.h
+++ b/drivers/staging/android/ion/ion_priv.h
@@ -100,18 +100,18 @@ void ion_buffer_destroy(struct ion_buffer *buffer);
* map_dma and map_kernel return pointer on success, ERR_PTR on error.
*/
struct ion_heap_ops {
- int (*allocate) (struct ion_heap *heap,
+ int (*allocate)(struct ion_heap *heap,
struct ion_buffer *buffer, unsigned long len,
unsigned long align, unsigned long flags);
- void (*free) (struct ion_buffer *buffer);
- int (*phys) (struct ion_heap *heap, struct ion_buffer *buffer,
+ void (*free)(struct ion_buffer *buffer);
+ int (*phys)(struct ion_heap *heap, struct ion_buffer *buffer,
ion_phys_addr_t *addr, size_t *len);
- struct sg_table *(*map_dma) (struct ion_heap *heap,
+ struct sg_table * (*map_dma)(struct ion_heap *heap,
struct ion_buffer *buffer);
- void (*unmap_dma) (struct ion_heap *heap, struct ion_buffer *buffer);
- void * (*map_kernel) (struct ion_heap *heap, struct ion_buffer *buffer);
- void (*unmap_kernel) (struct ion_heap *heap, struct ion_buffer *buffer);
- int (*map_user) (struct ion_heap *mapper, struct ion_buffer *buffer,
+ void (*unmap_dma)(struct ion_heap *heap, struct ion_buffer *buffer);
+ void * (*map_kernel)(struct ion_heap *heap, struct ion_buffer *buffer);
+ void (*unmap_kernel)(struct ion_heap *heap, struct ion_buffer *buffer);
+ int (*map_user)(struct ion_heap *mapper, struct ion_buffer *buffer,
struct vm_area_struct *vma);
};
--
1.7.9.5
---
On Mon, 2014-02-10 at 14:56 +0900, Daeseok Youn wrote:
>
Hello.
> diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
[]
> - struct sg_table *(*map_dma) (struct ion_heap *heap,
> + struct sg_table * (*map_dma)(struct ion_heap *heap,
The message about space required after "struct sg_table *"
is a checkpatch defect. This is better as:
struct sg_table *(*map_dma)(struct ion_heap *heap, etc...)
I'll fix checkpatch in a little bit.
I think It looks better than reported by checkpatch.pl.
But I have a qeustion, if your patch is applied to checkpatch.pl file,
a return type of "void *" line will be changed.
for example,
void * (*map_kernel)(struct ion_heap *heap, struct ion_buffer *buffer);
=> void *(*map_kernel)(struct ion_heap *heap, struct ion_buffer
*buffer); in same file.
right?
If you confirm my question, I re-send my patch after fixing that line.
And I have a patch which is same fix. (driver/staging/android/sync.h)
diff --git a/drivers/staging/android/sync.h b/drivers/staging/android/sync.h
index 62e2255b..6ee8d69 100644
--- a/drivers/staging/android/sync.h
+++ b/drivers/staging/android/sync.h
@@ -53,7 +53,7 @@ struct sync_timeline_ops {
const char *driver_name;
/* required */
- struct sync_pt *(*dup)(struct sync_pt *pt);
+ struct sync_pt * (*dup)(struct sync_pt *pt);
/* required */
int (*has_signaled)(struct sync_pt *pt);
2014-02-10 15:07 GMT+09:00 Joe Perches <[email protected]>:
> On Mon, 2014-02-10 at 14:56 +0900, Daeseok Youn wrote:
>>
>
> Hello.
>
>> diff --git a/drivers/staging/android/ion/ion_priv.h b/drivers/staging/android/ion/ion_priv.h
> []
>> - struct sg_table *(*map_dma) (struct ion_heap *heap,
>> + struct sg_table * (*map_dma)(struct ion_heap *heap,
>
> The message about space required after "struct sg_table *"
> is a checkpatch defect. This is better as:
>
> struct sg_table *(*map_dma)(struct ion_heap *heap, etc...)
>
> I'll fix checkpatch in a little bit.
>
>
Check for some function pointer return styles are too strict. Fix them.
Multiple spaces after function pointer return types are allowed.
int (*foo)(int bar)
Spaces after function pointer returns of pointer types are not required.
int *(*foo)(int bar)
Signed-off-by: Joe Perches <[email protected]>
---
scripts/checkpatch.pl | 31 +++++++++++++++++++++----------
1 file changed, 21 insertions(+), 10 deletions(-)
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 7d3bc2f..19591af 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -2848,10 +2848,7 @@ sub process {
# Function pointer declarations
# check spacing between type, funcptr, and args
# canonical declaration is "type (*funcptr)(args...)"
-#
-# the $Declare variable will capture all spaces after the type
-# so check it for trailing missing spaces or multiple spaces
- if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)$Ident(\s*)\)(\s*)\(/) {
+ if ($line =~ /^.\s*($Declare)\((\s*)\*(\s*)($Ident)(\s*)\)(\s*)\(/) {
my $declare = $1;
my $pre_pointer_space = $2;
my $post_pointer_space = $3;
@@ -2859,16 +2856,30 @@ sub process {
my $post_funcname_space = $5;
my $pre_args_space = $6;
- if ($declare !~ /\s$/) {
+# the $Declare variable will capture all spaces after the type
+# so check it for a missing trailing missing space but pointer return types
+# don't need a space so don't warn for those.
+ my $post_declare_space = "";
+ if ($declare =~ /(\s+)$/) {
+ $post_declare_space = $1;
+ $declare = rtrim($declare);
+ }
+ if ($declare !~ /\*$/ && $post_declare_space =~ /^$/) {
WARN("SPACING",
"missing space after return type\n" . $herecurr);
+ $post_declare_space = " ";
}
# unnecessary space "type (*funcptr)(args...)"
- elsif ($declare =~ /\s{2,}$/) {
- WARN("SPACING",
- "Multiple spaces after return type\n" . $herecurr);
- }
+# This test is not currently implemented because these declarations are
+# equivalent to
+# int foo(int bar, ...)
+# and this is form shouldn't/doesn't generate a checkpatch warning.
+#
+# elsif ($declare =~ /\s{2,}$/) {
+# WARN("SPACING",
+# "Multiple spaces after return type\n" . $herecurr);
+# }
# unnecessary space "type ( *funcptr)(args...)"
if (defined $pre_pointer_space &&
@@ -2900,7 +2911,7 @@ sub process {
if (show_type("SPACING") && $fix) {
$fixed[$linenr - 1] =~
- s/^(.\s*$Declare)\(\s*\*\s*($Ident)\s*\)\s*\(/rtrim($1) . " " . "\(\*$2\)\("/ex;
+ s/^(.\s*)$Declare\s*\(\s*\*\s*$Ident\s*\)\s*\(/$1 . $declare . $post_declare_space . '(*' . $funcname . ')('/ex;
}
}
--
1.8.1.2.459.gbcd45b4.dirty