* [PATCH] staging : ion : Fix some checkpatch warnings and an error
@ 2014-02-10 5:56 Daeseok Youn
2014-02-10 6:07 ` Joe Perches
0 siblings, 1 reply; 4+ messages in thread
From: Daeseok Youn @ 2014-02-10 5:56 UTC (permalink / raw)
To: gregkh, swetland
Cc: john.stultz, rebecca, ccross, ohaugan, romlem, linux-kernel, devel
>From aa06cc53c7214a044fbc220872aa6210c09608d3 Mon Sep 17 00:00:00 2001
From: Daeseok Youn <daeseok.youn@gmail.com>
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 <daeseok.youn@gmail.com>
---
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
---
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] staging : ion : Fix some checkpatch warnings and an error
2014-02-10 5:56 [PATCH] staging : ion : Fix some checkpatch warnings and an error Daeseok Youn
@ 2014-02-10 6:07 ` Joe Perches
2014-02-10 6:39 ` DaeSeok Youn
2014-02-10 23:59 ` [PATCH] checkpatch: Don't warn on some function pointer return styles Joe Perches
0 siblings, 2 replies; 4+ messages in thread
From: Joe Perches @ 2014-02-10 6:07 UTC (permalink / raw)
To: Daeseok Youn
Cc: gregkh, swetland, john.stultz, rebecca, ccross, ohaugan, romlem,
linux-kernel, devel
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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] staging : ion : Fix some checkpatch warnings and an error
2014-02-10 6:07 ` Joe Perches
@ 2014-02-10 6:39 ` DaeSeok Youn
2014-02-10 23:59 ` [PATCH] checkpatch: Don't warn on some function pointer return styles Joe Perches
1 sibling, 0 replies; 4+ messages in thread
From: DaeSeok Youn @ 2014-02-10 6:39 UTC (permalink / raw)
To: Joe Perches
Cc: Greg KH, swetland, John Stultz, Rebecca Zavin, ccross, ohaugan,
Rom Lemarchand, linux-kernel, devel
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 <joe@perches.com>:
> 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.
>
>
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH] checkpatch: Don't warn on some function pointer return styles
2014-02-10 6:07 ` Joe Perches
2014-02-10 6:39 ` DaeSeok Youn
@ 2014-02-10 23:59 ` Joe Perches
1 sibling, 0 replies; 4+ messages in thread
From: Joe Perches @ 2014-02-10 23:59 UTC (permalink / raw)
To: Andrew Morton; +Cc: Daeseok Youn, LKML, Dan Carpenter
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 <joe@perches.com>
---
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
^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-02-10 23:59 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-02-10 5:56 [PATCH] staging : ion : Fix some checkpatch warnings and an error Daeseok Youn
2014-02-10 6:07 ` Joe Perches
2014-02-10 6:39 ` DaeSeok Youn
2014-02-10 23:59 ` [PATCH] checkpatch: Don't warn on some function pointer return styles Joe Perches
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.