Skip to contentSkip to conversation

staging:android:fix line over 80 characters issue in binders.c this patch fixes line over 80 characters warning that was found using checkpatch.pl tool Signed-off-by:Anirudh Bhat <[email protected]>

PATCH In review anirudh bhat 2 participants 3 messages Apr 18, 2012 Staging

lore
Share & tools
lore

anirudh bhat

author
#0 lore raw

PATCH 2/2 staging:android:fix line over 80 characters issue in binders.c this patch fixes line over 80 characters warning that was found using checkpatch.pl tool Signed-off-by:Anirudh Bhat <[email protected]>

From: anirudh bhat <[email protected]>

---
 drivers/staging/android/binder.c |   59 +++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 29 deletions(-)
drivers/staging/android/binder.c +30 -29
index c283212..ae341e0 100644
@@ -644,8 +644,8 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
 		goto free_range;
 
 	if (vma == NULL) {
-		printk(KERN_ERR "binder: %d: binder_alloc_buf failed to "
-		       "map pages in userspace, no vma\n", proc->pid);
+		printk(KERN_ERR "binder: %d: binder_alloc_buf failed to map pages in userspace, no vma\n",
+			proc->pid);
 		goto err_no_vma;
 	}
 

@@ -657,8 +657,8 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
 		BUG_ON(*page);
 		*page = alloc_page(GFP_KERNEL | __GFP_ZERO);
 		if (*page == NULL) {
-			printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
-			       "for page at %p\n", proc->pid, page_addr);
+			printk(KERN_ERR "binder: %d: binder_alloc_buf failed for page at %p\n",
+				proc->pid, page_addr);
 			goto err_alloc_page_failed;
 		}
 		tmp_area.addr = page_addr;

@@ -666,18 +666,16 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
 		page_array_ptr = page;
 		ret = map_vm_area(&tmp_area, PAGE_KERNEL, &page_array_ptr);
 		if (ret) {
-			printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
-			       "to map page at %p in kernel\n",
-			       proc->pid, page_addr);
+			printk(KERN_ERR "binder: %d: binder_alloc_buf failed to map page at %p in kernel\n",
+				proc->pid, page_addr);
 			goto err_map_kernel_failed;
 		}
 		user_page_addr =
 			(uintptr_t)page_addr + proc->user_buffer_offset;
 		ret = vm_insert_page(vma, user_page_addr, page[0]);
 		if (ret) {
-			printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
-			       "to map page at %lx in userspace\n",
-			       proc->pid, user_page_addr);
+			printk(KERN_ERR "binder: %d: binder_alloc_buf failed to map page at %lx in userspace\n",
+				proc->pid, user_page_addr);
 			goto err_vm_insert_page_failed;
 		}
 		/* vm_insert_page does not seem to increment the refcount */

@@ -733,8 +731,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
 		ALIGN(offsets_size, sizeof(void *));
 
 	if (size < data_size || size < offsets_size) {
-		binder_user_error("binder: %d: got transaction with invalid "
-			"size %zd-%zd\n", proc->pid, data_size, offsets_size);
+		binder_user_error("binder:%d:transaction with invalid size %zd-%zd\n",
+					proc->pid, data_size, offsets_size);
 		return NULL;
 	}
 

@@ -762,8 +760,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
 		}
 	}
 	if (best_fit == NULL) {
-		printk(KERN_ERR "binder: %d: binder_alloc_buf size %zd failed, "
-		       "no address space\n", proc->pid, size);
+		printk(KERN_ERR "binder: %d: binder_alloc_buf size %zd failed,no address space\n",
+			proc->pid, size);
 		return NULL;
 	}
 	if (n == NULL) {

@@ -997,8 +995,8 @@ static int binder_inc_node(struct binder_node *node, int strong, int internal,
 			    node->internal_strong_refs == 0 &&
 			    !(node == binder_context_mgr_node &&
 			    node->has_strong_ref)) {
-				printk(KERN_ERR "binder: invalid inc strong "
-					"node for %d\n", node->debug_id);
+				printk(KERN_ERR "binder:invalid inc strong node for %d\n",
+					node->debug_id);
 				return -EINVAL;
 			}
 			node->internal_strong_refs++;

@@ -1013,8 +1011,8 @@ static int binder_inc_node(struct binder_node *node, int strong, int internal,
 			node->local_weak_refs++;
 		if (!node->has_weak_ref && list_empty(&node->work.entry)) {
 			if (target_list == NULL) {
-				printk(KERN_ERR "binder: invalid inc weak node "
-					"for %d\n", node->debug_id);
+				printk(KERN_ERR "binder: invalid inc weak node for %d\n",
+					node->debug_id);
 				return -EINVAL;
 			}
 			list_add_tail(&node->work.entry, target_list);

@@ -1206,10 +1204,11 @@ static int binder_dec_ref(struct binder_ref *ref, int strong)
 {
 	if (strong) {
 		if (ref->strong == 0) {
-			binder_user_error("binder: %d invalid dec strong, "
-					  "ref %d desc %d s %d w %d\n",
-					  ref->proc->pid, ref->debug_id,
-					  ref->desc, ref->strong, ref->weak);
+			binder_user_error("binder:%d invalid dec strong,ref %d desc %d s %d w %d\n",
+						ref->proc->pid, ref->debug_id,
+						ref->desc,
+						ref->strong,
+						ref->weak);
 			return -EINVAL;
 		}
 		ref->strong--;

@@ -1221,10 +1220,12 @@ static int binder_dec_ref(struct binder_ref *ref, int strong)
 		}
 	} else {
 		if (ref->weak == 0) {
-			binder_user_error("binder: %d invalid dec weak, "
-					  "ref %d desc %d s %d w %d\n",
-					  ref->proc->pid, ref->debug_id,
-					  ref->desc, ref->strong, ref->weak);
+			binder_user_error("binder: %d invalid dec weak,ref %d desc %d s %d w %d\n",
+						ref->proc->pid,
+						ref->debug_id,
+						ref->desc,
+						ref->strong,
+						ref->weak);
 			return -EINVAL;
 		}
 		ref->weak--;

@@ -3303,7 +3304,7 @@ static void print_binder_proc(struct seq_file *m,
 		m->count = start_pos;
 }
 
-static const char *binder_return_strings[] = {
+static const char *const binder_return_strings[] = {
 	"BR_ERROR",
 	"BR_OK",
 	"BR_TRANSACTION",

@@ -3324,7 +3325,7 @@ static const char *binder_return_strings[] = {
 	"BR_FAILED_REPLY"
 };
 
-static const char *binder_command_strings[] = {
+static const char *const binder_command_strings[] = {
 	"BC_TRANSACTION",
 	"BC_REPLY",
 	"BC_ACQUIRE_RESULT",

@@ -3344,7 +3345,7 @@ static const char *binder_command_strings[] = {
 	"BC_DEAD_BINDER_DONE"
 };
 
-static const char *binder_objstat_strings[] = {
+static const char *const binder_objstat_strings[] = {
 	"proc",
 	"thread",
 	"node",
Signature · 1 line
1.7.9.5
#1 lore raw
Quoted 2 lines · anirudh bhat

From: anirudh bhat <[email protected]>

Unless you are sending a patch on behalf of someone else, and need to preserve that person as the author of the patch, there's usually no need for a From: in the body of your patch submission email - the From: from the email headers will be used.

This is where your Signed-off-by: should have been, *not* in the patch subject that will usually be used as the initial/summary line of the git commit message. Which is also why it should usually be kept fairly short - yours is extremely long - 164 characters if we exclude the misplaced Signed-off-by.

To quote Documentation/SubmittingPatches (which I suggest you read in full):

"For these reasons, the "summary" must be no more than 70-75
characters, and it must describe both what the patch changes, as well as why the patch might be necessary. It is challenging to be both succinct and descriptive, but that is what a well-written summary should do."

Ohh and one other little detail about that Signed-off-by:
in the Subject - there should be a space between the colon and name - as in "Signed-off-by: Anirudh Bhat <[email protected]>".

And finally, your subject is simply wrong.
It does not fix any checkpatch.pl warnings about lines over 80 characters as it claims. What it actually does is address 10 out of the current 46 warnings about "quoted string split across lines" and the 3 warnings about "static const char * array should probably be static const char * const". But that's not what the Subject says at all.

Quoted 54 lines
---
 drivers/staging/android/binder.c |   59 +++++++++++++++++++-------------------
 1 file changed, 30 insertions(+), 29 deletions(-)
drivers/staging/android/binder.c +8 -10
index c283212..ae341e0 100644
@@ -644,8 +644,8 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
 		goto free_range;
 
 	if (vma == NULL) {
-		printk(KERN_ERR "binder: %d: binder_alloc_buf failed to "
-		       "map pages in userspace, no vma\n", proc->pid);
+		printk(KERN_ERR "binder: %d: binder_alloc_buf failed to map pages in userspace, no vma\n",
+			proc->pid);
 		goto err_no_vma;
 	}
 

@@ -657,8 +657,8 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
 		BUG_ON(*page);
 		*page = alloc_page(GFP_KERNEL | __GFP_ZERO);
 		if (*page == NULL) {
-			printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
-			       "for page at %p\n", proc->pid, page_addr);
+			printk(KERN_ERR "binder: %d: binder_alloc_buf failed for page at %p\n",
+				proc->pid, page_addr);
 			goto err_alloc_page_failed;
 		}
 		tmp_area.addr = page_addr;

@@ -666,18 +666,16 @@ static int binder_update_page_range(struct binder_proc *proc, int allocate,
 		page_array_ptr = page;
 		ret = map_vm_area(&tmp_area, PAGE_KERNEL, &page_array_ptr);
 		if (ret) {
-			printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
-			       "to map page at %p in kernel\n",
-			       proc->pid, page_addr);
+			printk(KERN_ERR "binder: %d: binder_alloc_buf failed to map page at %p in kernel\n",
+				proc->pid, page_addr);
 			goto err_map_kernel_failed;
 		}
 		user_page_addr =
 			(uintptr_t)page_addr + proc->user_buffer_offset;
 		ret = vm_insert_page(vma, user_page_addr, page[0]);
 		if (ret) {
-			printk(KERN_ERR "binder: %d: binder_alloc_buf failed "
-			       "to map page at %lx in userspace\n",
-			       proc->pid, user_page_addr);
+			printk(KERN_ERR "binder: %d: binder_alloc_buf failed to map page at %lx in userspace\n",
+				proc->pid, user_page_addr);
 			goto err_vm_insert_page_failed;
 		}
 		/* vm_insert_page does not seem to increment the refcount */

drivers/staging/android/binder.c static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc, · line 735

Quoted 7 lines
drivers/staging/android/binder.c +2 -2
@@ -733,8 +731,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
 		ALIGN(offsets_size, sizeof(void *));
 
 	if (size < data_size || size < offsets_size) {
-		binder_user_error("binder: %d: got transaction with invalid "
-			"size %zd-%zd\n", proc->pid, data_size, offsets_size);
+		binder_user_error("binder:%d:transaction with invalid size %zd-%zd\n",
+					proc->pid, data_size, offsets_size);

The space that used to be after "binder:" is gone in your version :(

drivers/staging/android/binder.c static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc, · line 764

Quoted 11 lines
drivers/staging/android/binder.c +2 -2
 		return NULL;
 	}
 

@@ -762,8 +760,8 @@ static struct binder_buffer *binder_alloc_buf(struct binder_proc *proc,
 		}
 	}
 	if (best_fit == NULL) {
-		printk(KERN_ERR "binder: %d: binder_alloc_buf size %zd failed, "
-		       "no address space\n", proc->pid, size);
+		printk(KERN_ERR "binder: %d: binder_alloc_buf size %zd failed,no address space\n",
+			proc->pid, size);

You have killed the space after the comma in the message printed :-(

drivers/staging/android/binder.c static int binder_inc_node(struct binder_node *node, int strong, int internal, · line 999

Quoted 11 lines
drivers/staging/android/binder.c +2 -2
 		return NULL;
 	}
 	if (n == NULL) {

@@ -997,8 +995,8 @@ static int binder_inc_node(struct binder_node *node, int strong, int internal,
 			    node->internal_strong_refs == 0 &&
 			    !(node == binder_context_mgr_node &&
 			    node->has_strong_ref)) {
-				printk(KERN_ERR "binder: invalid inc strong "
-					"node for %d\n", node->debug_id);
+				printk(KERN_ERR "binder:invalid inc strong node for %d\n",
+					node->debug_id);

This used to print "binder: invalid inc..."
but now it prints "binder:invalid inc..."

Quoted 15 lines
drivers/staging/android/binder.c +2 -2
 				return -EINVAL;
 			}
 			node->internal_strong_refs++;

@@ -1013,8 +1011,8 @@ static int binder_inc_node(struct binder_node *node, int strong, int internal,
 			node->local_weak_refs++;
 		if (!node->has_weak_ref && list_empty(&node->work.entry)) {
 			if (target_list == NULL) {
-				printk(KERN_ERR "binder: invalid inc weak node "
-					"for %d\n", node->debug_id);
+				printk(KERN_ERR "binder: invalid inc weak node for %d\n",
+					node->debug_id);
 				return -EINVAL;
 			}
 			list_add_tail(&node->work.entry, target_list);

drivers/staging/android/binder.c static int binder_dec_ref(struct binder_ref *ref, int strong) · line 1207

Quoted 8 lines
drivers/staging/android/binder.c +1 -4
@@ -1206,10 +1204,11 @@ static int binder_dec_ref(struct binder_ref *ref, int strong)
 {
 	if (strong) {
 		if (ref->strong == 0) {
-			binder_user_error("binder: %d invalid dec strong, "
-					  "ref %d desc %d s %d w %d\n",
-					  ref->proc->pid, ref->debug_id,
-					  ref->desc, ref->strong, ref->weak);
+			binder_user_error("binder:%d invalid dec strong,ref %d desc %d s %d w %d\n",

Before your patch the line printed had a space between the comma after "strong" and before "ref" as in "strong, ref".
After your patch that space is gone :-(

drivers/staging/android/binder.c static int binder_dec_ref(struct binder_ref *ref, int strong) · line 1211

Quoted 4 lines
drivers/staging/android/binder.c +4 -0
+						ref->proc->pid, ref->debug_id,
+						ref->desc,
+						ref->strong,
+						ref->weak);

Why are you keeping "ref->proc->pid, ref->debug_id," on one line but the rest on individual lines?

Why not this:

						ref->proc->pid, ref->debug_id,
						ref->desc, ref->strong,
						ref->weak);

all 3 lines stay below the 80 char limit and it looks nicer.

Quoted 7 lines
drivers/staging/android/binder.c +0 -0
 			return -EINVAL;
 		}
 		ref->strong--;

@@ -1221,10 +1220,12 @@ static int binder_dec_ref(struct binder_ref *ref, int strong)
 		}
 	} else {
 		if (ref->weak == 0) {

drivers/staging/android/binder.c static int binder_dec_ref(struct binder_ref *ref, int strong) · line 1228

Quoted 10 lines
drivers/staging/android/binder.c +6 -4
-			binder_user_error("binder: %d invalid dec weak, "
-					  "ref %d desc %d s %d w %d\n",
-					  ref->proc->pid, ref->debug_id,
-					  ref->desc, ref->strong, ref->weak);
+			binder_user_error("binder: %d invalid dec weak,ref %d desc %d s %d w %d\n",
+						ref->proc->pid,
+						ref->debug_id,
+						ref->desc,
+						ref->strong,
+						ref->weak);

Same comments here as above.

drivers/staging/android/binder.c static void print_binder_proc(struct seq_file *m, · line 3307

Quoted 9 lines
drivers/staging/android/binder.c +1 -1
 			return -EINVAL;
 		}
 		ref->weak--;

@@ -3303,7 +3304,7 @@ static void print_binder_proc(struct seq_file *m,
 		m->count = start_pos;
 }
 
-static const char *binder_return_strings[] = {
+static const char *const binder_return_strings[] = {

If you read the checkpatch warning this addresses you'll see that it says "static const char * array should probably be static const char * const" But you changed it to "static const char *const".

A quick check for usage of the two forms:

[jj@arch linux]$ git grep -E "static +const +char *\* +const" | wc -l 673
[jj@arch linux]$ git grep -E "static +const +char *\*const" | wc -l 151

Suggests that the vast majority prefer the style that has a space before the final "const" and since a uniform style is preferable I'd say you should probably go with that instead.

Quoted 13 lines
drivers/staging/android/binder.c +1 -1
 	"BR_ERROR",
 	"BR_OK",
 	"BR_TRANSACTION",

@@ -3324,7 +3325,7 @@ static const char *binder_return_strings[] = {
 	"BR_FAILED_REPLY"
 };
 
-static const char *binder_command_strings[] = {
+static const char *const binder_command_strings[] = {
 	"BC_TRANSACTION",
 	"BC_REPLY",
 	"BC_ACQUIRE_RESULT",

drivers/staging/android/binder.c static const char *binder_command_strings[] = { · line 3351

Quoted 8 lines
drivers/staging/android/binder.c +1 -1
@@ -3344,7 +3345,7 @@ static const char *binder_command_strings[] = {
 	"BC_DEAD_BINDER_DONE"
 };
 
-static const char *binder_objstat_strings[] = {
+static const char *const binder_objstat_strings[] = {
 	"proc",
 	"thread",
 	"node",

Same comment as above.

Signature · 3 lines
Jesper Juhl <[email protected]>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.
#2 lore raw
Quoted 9 lines · Jesper Juhl
Quoted 2 lines · anirudh bhat

From: anirudh bhat <[email protected]>

Unless you are sending a patch on behalf of someone else, and need to preserve that person as the author of the patch, there's usually no need for a From: in the body of your patch submission email - the From: from the email headers will be used.

Ohh and I forgot to mention, that in case you do put a From: line (or any other) you want to use a *real* email address.

The same goes for the email Cc: list - "[email protected]" doesn't work so great there either, yet that is one of the addresses you Cc'ed the original mail to..

Signature · 3 lines
Jesper Juhl <[email protected]>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.