[PATCH] Staging: android: binder: Fix over-80-char lines

[PATCH] Staging: android: binder: Fix over-80-char lines

[Daniel Dressler]

From: danieru <danieru.dressler@gmail.com>

Following Greg Kroah-Hartman's newbie guide to hacking
the linux kernel this patch addresses only coding style
issues.

Binder still has many too-long lines but I'm worried
doing too much work in a single patch is unfair to the
reviewers. So this patch address 20% of the file's
issues.

There is one change to take notice of: it merges two if
statement's conditionals together. Then it removes a
redundant else clause.

Signed-off-by: Daniel Dressler <danieru.dressler@gmail.com>
---
 drivers/staging/android/binder.c | 89 ++++++++++++++++++++++++----------------
 1 file changed, 54 insertions(+), 35 deletions(-)

diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
index cfe4bc8..6e80861a 100644
--- a/drivers/staging/android/binder.c
+++ b/drivers/staging/android/binder.c
@@ -2211,11 +2211,14 @@ retry:
 			binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
 		} break;
 		case BINDER_WORK_NODE: {
-			struct binder_node *node = container_of(w, struct binder_node, work);
+			struct binder_node *node = container_of(w,
+						struct binder_node, work);
 			uint32_t cmd = BR_NOOP;
 			const char *cmd_name;
-			int strong = node->internal_strong_refs || node->local_strong_refs;
-			int weak = !hlist_empty(&node->refs) || node->local_weak_refs || strong;
+			int strong = node->internal_strong_refs ||
+						node->local_strong_refs;
+			int weak = !hlist_empty(&node->refs) ||
+					node->local_weak_refs || strong;
 			if (weak && !node->has_weak_ref) {
 				cmd = BR_INCREFS;
 				cmd_name = "BR_INCREFS";
@@ -2311,8 +2314,10 @@ retry:
 				binder_stats_deleted(BINDER_STAT_DEATH);
 			} else
 				list_move(&w->entry, &proc->delivered_death);
+
+			/* DEAD_BINDER notifications can cause transactions */
 			if (cmd == BR_DEAD_BINDER)
-				goto done; /* DEAD_BINDER notifications can cause transactions */
+				goto done;
 		} break;
 		}
 
@@ -2321,7 +2326,8 @@ retry:
 
 		BUG_ON(t->buffer == NULL);
 		if (t->buffer->target_node) {
-			struct binder_node *target_node = t->buffer->target_node;
+			struct binder_node *target_node =
+							t->buffer->target_node;
 			tr.target.ptr = target_node->ptr;
 			tr.cookie =  target_node->cookie;
 			t->saved_priority = task_nice(current);
@@ -2344,7 +2350,7 @@ retry:
 		if (t->from) {
 			struct task_struct *sender = t->from->proc->tsk;
 			tr.sender_pid = task_tgid_nr_ns(sender,
-							task_active_pid_ns(current));
+						task_active_pid_ns(current));
 		} else {
 			tr.sender_pid = 0;
 		}
@@ -2575,11 +2581,13 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 	unsigned int size = _IOC_SIZE(cmd);
 	void __user *ubuf = (void __user *)arg;
 
-	/*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
+	/*pr_info("binder_ioctl: %d:%d %x %lx\n",
+					proc->pid, current->pid, cmd, arg);*/
 
 	trace_binder_ioctl(cmd, arg);
 
-	ret = wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
+	ret = wait_event_interruptible(binder_user_error_wait,
+						binder_stop_on_user_error < 2);
 	if (ret)
 		goto err_unlocked;
 
@@ -2602,10 +2610,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			goto err;
 		}
 		binder_debug(BINDER_DEBUG_READ_WRITE,
-			     "%d:%d write %lld at %016llx, read %lld at %016llx\n",
-			     proc->pid, thread->pid,
-			     (u64)bwr.write_size, (u64)bwr.write_buffer,
-			     (u64)bwr.read_size, (u64)bwr.read_buffer);
+			"%d:%d write %lld at %016llx, read %lld at %016llx\n",
+			proc->pid, thread->pid,
+			(u64)bwr.write_size, (u64)bwr.write_buffer,
+			(u64)bwr.read_size, (u64)bwr.read_buffer);
 
 		if (bwr.write_size > 0) {
 			ret = binder_thread_write(proc, thread,
@@ -2635,10 +2643,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			}
 		}
 		binder_debug(BINDER_DEBUG_READ_WRITE,
-			     "%d:%d wrote %lld of %lld, read return %lld of %lld\n",
-			     proc->pid, thread->pid,
-			     (u64)bwr.write_consumed, (u64)bwr.write_size,
-			     (u64)bwr.read_consumed, (u64)bwr.read_size);
+			"%d:%d wrote %lld of %lld, read return %lld of %lld\n",
+			proc->pid, thread->pid,
+			(u64)bwr.write_consumed, (u64)bwr.write_size,
+			(u64)bwr.read_consumed, (u64)bwr.read_size);
 		if (copy_to_user(ubuf, &bwr, sizeof(bwr))) {
 			ret = -EFAULT;
 			goto err;
@@ -2646,7 +2654,8 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		break;
 	}
 	case BINDER_SET_MAX_THREADS:
-		if (copy_from_user(&proc->max_threads, ubuf, sizeof(proc->max_threads))) {
+		if (copy_from_user(&proc->max_threads, ubuf,
+						sizeof(proc->max_threads))) {
 			ret = -EINVAL;
 			goto err;
 		}
@@ -2657,16 +2666,16 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			ret = -EBUSY;
 			goto err;
 		}
-		if (uid_valid(binder_context_mgr_uid)) {
-			if (!uid_eq(binder_context_mgr_uid, current->cred->euid)) {
-				pr_err("BINDER_SET_CONTEXT_MGR bad uid %d != %d\n",
-				       from_kuid(&init_user_ns, current->cred->euid),
-				       from_kuid(&init_user_ns, binder_context_mgr_uid));
-				ret = -EPERM;
-				goto err;
-			}
-		} else
-			binder_context_mgr_uid = current->cred->euid;
+		if (uid_valid(binder_context_mgr_uid) &&
+			!uid_eq(binder_context_mgr_uid, current->cred->euid)) {
+			pr_err("BINDER_SET_CONTEXT_MGR bad uid %d != %d\n",
+			       from_kuid(&init_user_ns, current->cred->euid),
+			       from_kuid(&init_user_ns, binder_context_mgr_uid)
+			       );
+			ret = -EPERM;
+			goto err;
+		}
+		binder_context_mgr_uid = current->cred->euid;
 		binder_context_mgr_node = binder_new_node(proc, 0, 0);
 		if (binder_context_mgr_node == NULL) {
 			ret = -ENOMEM;
@@ -2688,7 +2697,8 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 			ret = -EINVAL;
 			goto err;
 		}
-		if (put_user(BINDER_CURRENT_PROTOCOL_VERSION, &((struct binder_version *)ubuf)->protocol_version)) {
+		if (put_user(BINDER_CURRENT_PROTOCOL_VERSION,
+			&((struct binder_version *)ubuf)->protocol_version)) {
 			ret = -EINVAL;
 			goto err;
 		}
@@ -2702,9 +2712,11 @@ err:
 	if (thread)
 		thread->looper &= ~BINDER_LOOPER_STATE_NEED_RETURN;
 	binder_unlock(__func__);
-	wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
+	wait_event_interruptible(binder_user_error_wait,
+						binder_stop_on_user_error < 2);
 	if (ret && ret != -ERESTARTSYS)
-		pr_info("%d:%d ioctl %x %lx returned %d\n", proc->pid, current->pid, cmd, arg, ret);
+		pr_info("%d:%d ioctl %x %lx returned %d\n", proc->pid,
+						current->pid, cmd, arg, ret);
 err_unlocked:
 	trace_binder_ioctl_done(ret);
 	return ret;
@@ -2784,13 +2796,18 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
 
 #ifdef CONFIG_CPU_CACHE_VIPT
 	if (cache_is_vipt_aliasing()) {
-		while (CACHE_COLOUR((vma->vm_start ^ (uint32_t)proc->buffer))) {
-			pr_info("binder_mmap: %d %lx-%lx maps %p bad alignment\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer);
+		uint32_t proc_buffer = (uint32_t)proc->buffer;
+		while (CACHE_COLOUR((vma->vm_start ^ proc_buffer))) {
+			pr_info(
+			"binder_mmap: %d %lx-%lx maps %p bad alignment\n",
+						proc->pid, vma->vm_start,
+						vma->vm_end, proc->buffer);
 			vma->vm_start += PAGE_SIZE;
 		}
 	}
 #endif
-	proc->pages = kzalloc(sizeof(proc->pages[0]) * ((vma->vm_end - vma->vm_start) / PAGE_SIZE), GFP_KERNEL);
+	size_t num_pages = (vma->vm_end - vma->vm_start) / PAGE_SIZE;
+	proc->pages = kzalloc(sizeof(proc->pages[0]) * num_pages, GFP_KERNEL);
 	if (proc->pages == NULL) {
 		ret = -ENOMEM;
 		failure_string = "alloc page array";
@@ -2801,7 +2818,8 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
 	vma->vm_ops = &binder_vm_ops;
 	vma->vm_private_data = proc;
 
-	if (binder_update_page_range(proc, 1, proc->buffer, proc->buffer + PAGE_SIZE, vma)) {
+	if (binder_update_page_range(proc, 1, proc->buffer,
+					proc->buffer + PAGE_SIZE, vma)) {
 		ret = -ENOMEM;
 		failure_string = "alloc small buf";
 		goto err_alloc_small_buf_failed;
@@ -2887,7 +2905,8 @@ static void binder_deferred_flush(struct binder_proc *proc)
 	struct rb_node *n;
 	int wake_count = 0;
 	for (n = rb_first(&proc->threads); n != NULL; n = rb_next(n)) {
-		struct binder_thread *thread = rb_entry(n, struct binder_thread, rb_node);
+		struct binder_thread *thread = rb_entry(n, struct binder_thread,
+								rb_node);
 		thread->looper |= BINDER_LOOPER_STATE_NEED_RETURN;
 		if (thread->looper & BINDER_LOOPER_STATE_WAITING) {
 			wake_up_interruptible(&thread->wait);
-- 
1.9.1

Re: [PATCH] Staging: android: binder: Fix over-80-char lines

[NeilBrown]

[-- Attachment #1: Type: text/plain, Size: 10516 bytes --]

On Fri, 30 May 2014 22:31:20 -0600 Daniel Dressler
<danieru.dressler@gmail.com> wrote:

> From: danieru <danieru.dressler@gmail.com>
> 
> Following Greg Kroah-Hartman's newbie guide to hacking
> the linux kernel this patch addresses only coding style
> issues.
> 
> Binder still has many too-long lines but I'm worried
> doing too much work in a single patch is unfair to the
> reviewers. So this patch address 20% of the file's
> issues.
> 
> There is one change to take notice of: it merges two if
> statement's conditionals together. Then it removes a
> redundant else clause.
> 
> Signed-off-by: Daniel Dressler <danieru.dressler@gmail.com>
> ---
>  drivers/staging/android/binder.c | 89 ++++++++++++++++++++++++----------------
>  1 file changed, 54 insertions(+), 35 deletions(-)
> 
> diff --git a/drivers/staging/android/binder.c b/drivers/staging/android/binder.c
> index cfe4bc8..6e80861a 100644
> --- a/drivers/staging/android/binder.c
> +++ b/drivers/staging/android/binder.c
> @@ -2211,11 +2211,14 @@ retry:
>  			binder_stats_deleted(BINDER_STAT_TRANSACTION_COMPLETE);
>  		} break;
>  		case BINDER_WORK_NODE: {
> -			struct binder_node *node = container_of(w, struct binder_node, work);
> +			struct binder_node *node = container_of(w,
> +						struct binder_node, work);
>  			uint32_t cmd = BR_NOOP;
>  			const char *cmd_name;
> -			int strong = node->internal_strong_refs || node->local_strong_refs;
> -			int weak = !hlist_empty(&node->refs) || node->local_weak_refs || strong;
> +			int strong = node->internal_strong_refs ||
> +						node->local_strong_refs;
> +			int weak = !hlist_empty(&node->refs) ||
> +					node->local_weak_refs || strong;
>  			if (weak && !node->has_weak_ref) {
>  				cmd = BR_INCREFS;
>  				cmd_name = "BR_INCREFS";
> @@ -2311,8 +2314,10 @@ retry:
>  				binder_stats_deleted(BINDER_STAT_DEATH);
>  			} else
>  				list_move(&w->entry, &proc->delivered_death);
> +
> +			/* DEAD_BINDER notifications can cause transactions */
>  			if (cmd == BR_DEAD_BINDER)
> -				goto done; /* DEAD_BINDER notifications can cause transactions */
> +				goto done;
>  		} break;
>  		}
>  
> @@ -2321,7 +2326,8 @@ retry:
>  
>  		BUG_ON(t->buffer == NULL);
>  		if (t->buffer->target_node) {
> -			struct binder_node *target_node = t->buffer->target_node;
> +			struct binder_node *target_node =
> +							t->buffer->target_node;
>  			tr.target.ptr = target_node->ptr;
>  			tr.cookie =  target_node->cookie;
>  			t->saved_priority = task_nice(current);
> @@ -2344,7 +2350,7 @@ retry:
>  		if (t->from) {
>  			struct task_struct *sender = t->from->proc->tsk;
>  			tr.sender_pid = task_tgid_nr_ns(sender,
> -							task_active_pid_ns(current));
> +						task_active_pid_ns(current));

If it were my code, I'd say the cure is much worse than the disease.

Stuff inside brackets (any sort) should always be to the right of the opening
bracket, unless that bracket is at the end of the line.  Then the stuff
inside the brackets should be indented a single tab.

So the only alternative to the original here (which I don't personally think
is worth fixing) is:

>  			tr.sender_pid = task_tgid_nr_ns(
>                               sender,	task_active_pid_ns(current));

That way the full list of arguments is still a well defined block that is
easy to see.



>  		} else {
>  			tr.sender_pid = 0;
>  		}
> @@ -2575,11 +2581,13 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  	unsigned int size = _IOC_SIZE(cmd);
>  	void __user *ubuf = (void __user *)arg;
>  
> -	/*pr_info("binder_ioctl: %d:%d %x %lx\n", proc->pid, current->pid, cmd, arg);*/
> +	/*pr_info("binder_ioctl: %d:%d %x %lx\n",
> +					proc->pid, current->pid, cmd, arg);*/
>  
>  	trace_binder_ioctl(cmd, arg);
>  
> -	ret = wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
> +	ret = wait_event_interruptible(binder_user_error_wait,
> +						binder_stop_on_user_error < 2);
>  	if (ret)
>  		goto err_unlocked;
>  
> @@ -2602,10 +2610,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			goto err;
>  		}
>  		binder_debug(BINDER_DEBUG_READ_WRITE,
> -			     "%d:%d write %lld at %016llx, read %lld at %016llx\n",
> -			     proc->pid, thread->pid,
> -			     (u64)bwr.write_size, (u64)bwr.write_buffer,
> -			     (u64)bwr.read_size, (u64)bwr.read_buffer);
> +			"%d:%d write %lld at %016llx, read %lld at %016llx\n",
> +			proc->pid, thread->pid,
> +			(u64)bwr.write_size, (u64)bwr.write_buffer,
> +			(u64)bwr.read_size, (u64)bwr.read_buffer);
>  
>  		if (bwr.write_size > 0) {
>  			ret = binder_thread_write(proc, thread,
> @@ -2635,10 +2643,10 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			}
>  		}
>  		binder_debug(BINDER_DEBUG_READ_WRITE,
> -			     "%d:%d wrote %lld of %lld, read return %lld of %lld\n",
> -			     proc->pid, thread->pid,
> -			     (u64)bwr.write_consumed, (u64)bwr.write_size,
> -			     (u64)bwr.read_consumed, (u64)bwr.read_size);
> +			"%d:%d wrote %lld of %lld, read return %lld of %lld\n",
> +			proc->pid, thread->pid,
> +			(u64)bwr.write_consumed, (u64)bwr.write_size,
> +			(u64)bwr.read_consumed, (u64)bwr.read_size);
>  		if (copy_to_user(ubuf, &bwr, sizeof(bwr))) {
>  			ret = -EFAULT;
>  			goto err;
> @@ -2646,7 +2654,8 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  		break;
>  	}
>  	case BINDER_SET_MAX_THREADS:
> -		if (copy_from_user(&proc->max_threads, ubuf, sizeof(proc->max_threads))) {
> +		if (copy_from_user(&proc->max_threads, ubuf,
> +						sizeof(proc->max_threads))) {
>  			ret = -EINVAL;
>  			goto err;
>  		}
> @@ -2657,16 +2666,16 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			ret = -EBUSY;
>  			goto err;
>  		}
> -		if (uid_valid(binder_context_mgr_uid)) {
> -			if (!uid_eq(binder_context_mgr_uid, current->cred->euid)) {
> -				pr_err("BINDER_SET_CONTEXT_MGR bad uid %d != %d\n",
> -				       from_kuid(&init_user_ns, current->cred->euid),
> -				       from_kuid(&init_user_ns, binder_context_mgr_uid));
> -				ret = -EPERM;
> -				goto err;
> -			}
> -		} else
> -			binder_context_mgr_uid = current->cred->euid;
> +		if (uid_valid(binder_context_mgr_uid) &&
> +			!uid_eq(binder_context_mgr_uid, current->cred->euid)) {

This also looks wrong.  visually, the second line above links with the line
below, but conceptually it links with the next line above.
So:
> +		if (uid_valid(binder_context_mgr_uid) &&
> +		    !uid_eq(binder_context_mgr_uid, current->cred->euid)) {

is much more visually consistent.

Apply these rules throughout the patch and, for me at least, the patch will be much better for it.

(I use emacs C-mode with (c-set-style "K&R"), and it always gets the indenting right)

Thanks,
NeilBrown



> +			pr_err("BINDER_SET_CONTEXT_MGR bad uid %d != %d\n",
> +			       from_kuid(&init_user_ns, current->cred->euid),
> +			       from_kuid(&init_user_ns, binder_context_mgr_uid)
> +			       );
> +			ret = -EPERM;
> +			goto err;
> +		}
> +		binder_context_mgr_uid = current->cred->euid;
>  		binder_context_mgr_node = binder_new_node(proc, 0, 0);
>  		if (binder_context_mgr_node == NULL) {
>  			ret = -ENOMEM;
> @@ -2688,7 +2697,8 @@ static long binder_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  			ret = -EINVAL;
>  			goto err;
>  		}
> -		if (put_user(BINDER_CURRENT_PROTOCOL_VERSION, &((struct binder_version *)ubuf)->protocol_version)) {
> +		if (put_user(BINDER_CURRENT_PROTOCOL_VERSION,
> +			&((struct binder_version *)ubuf)->protocol_version)) {
>  			ret = -EINVAL;
>  			goto err;
>  		}
> @@ -2702,9 +2712,11 @@ err:
>  	if (thread)
>  		thread->looper &= ~BINDER_LOOPER_STATE_NEED_RETURN;
>  	binder_unlock(__func__);
> -	wait_event_interruptible(binder_user_error_wait, binder_stop_on_user_error < 2);
> +	wait_event_interruptible(binder_user_error_wait,
> +						binder_stop_on_user_error < 2);
>  	if (ret && ret != -ERESTARTSYS)
> -		pr_info("%d:%d ioctl %x %lx returned %d\n", proc->pid, current->pid, cmd, arg, ret);
> +		pr_info("%d:%d ioctl %x %lx returned %d\n", proc->pid,
> +						current->pid, cmd, arg, ret);
>  err_unlocked:
>  	trace_binder_ioctl_done(ret);
>  	return ret;
> @@ -2784,13 +2796,18 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>  
>  #ifdef CONFIG_CPU_CACHE_VIPT
>  	if (cache_is_vipt_aliasing()) {
> -		while (CACHE_COLOUR((vma->vm_start ^ (uint32_t)proc->buffer))) {
> -			pr_info("binder_mmap: %d %lx-%lx maps %p bad alignment\n", proc->pid, vma->vm_start, vma->vm_end, proc->buffer);
> +		uint32_t proc_buffer = (uint32_t)proc->buffer;
> +		while (CACHE_COLOUR((vma->vm_start ^ proc_buffer))) {
> +			pr_info(
> +			"binder_mmap: %d %lx-%lx maps %p bad alignment\n",
> +						proc->pid, vma->vm_start,
> +						vma->vm_end, proc->buffer);
>  			vma->vm_start += PAGE_SIZE;
>  		}
>  	}
>  #endif
> -	proc->pages = kzalloc(sizeof(proc->pages[0]) * ((vma->vm_end - vma->vm_start) / PAGE_SIZE), GFP_KERNEL);
> +	size_t num_pages = (vma->vm_end - vma->vm_start) / PAGE_SIZE;
> +	proc->pages = kzalloc(sizeof(proc->pages[0]) * num_pages, GFP_KERNEL);
>  	if (proc->pages == NULL) {
>  		ret = -ENOMEM;
>  		failure_string = "alloc page array";
> @@ -2801,7 +2818,8 @@ static int binder_mmap(struct file *filp, struct vm_area_struct *vma)
>  	vma->vm_ops = &binder_vm_ops;
>  	vma->vm_private_data = proc;
>  
> -	if (binder_update_page_range(proc, 1, proc->buffer, proc->buffer + PAGE_SIZE, vma)) {
> +	if (binder_update_page_range(proc, 1, proc->buffer,
> +					proc->buffer + PAGE_SIZE, vma)) {
>  		ret = -ENOMEM;
>  		failure_string = "alloc small buf";
>  		goto err_alloc_small_buf_failed;
> @@ -2887,7 +2905,8 @@ static void binder_deferred_flush(struct binder_proc *proc)
>  	struct rb_node *n;
>  	int wake_count = 0;
>  	for (n = rb_first(&proc->threads); n != NULL; n = rb_next(n)) {
> -		struct binder_thread *thread = rb_entry(n, struct binder_thread, rb_node);
> +		struct binder_thread *thread = rb_entry(n, struct binder_thread,
> +								rb_node);
>  		thread->looper |= BINDER_LOOPER_STATE_NEED_RETURN;
>  		if (thread->looper & BINDER_LOOPER_STATE_WAITING) {
>  			wake_up_interruptible(&thread->wait);


[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 828 bytes --]

Re: [PATCH] Staging: android: binder: Fix over-80-char lines

[Dan Carpenter]

What Neil said.

On Fri, May 30, 2014 at 10:31:20PM -0600, Daniel Dressler wrote:
> From: danieru <danieru.dressler@gmail.com>

This is wrong and we also get your name and email address from the
email headers so leave this line out.

> 
> Following Greg Kroah-Hartman's newbie guide to hacking
> the linux kernel this patch addresses only coding style
> issues.
> 
> Binder still has many too-long lines but I'm worried
> doing too much work in a single patch is unfair to the
> reviewers. So this patch address 20% of the file's
> issues.
> 
> There is one change to take notice of: it merges two if
> statement's conditionals together. Then it removes a
> redundant else clause.

Thanks for pointing this out, but the else statement wasn't redundant.
Your change introduces a bug.

Your patch doesn't apply.  You need to work against linux-next.

regards,
dan carpenter

Re: [PATCH] Staging: android: binder: Fix over-80-char lines

[Daniel Dressler]

Thank you Neil

I was not sure if space-based alignment was allowed so thank you for
demonstrating the change. It does make the code much easier to read.

Similar thank you for how to indent over-long function calls. I had
mis-understood the coding standard to mean overflow needed to be on
the far right margin. A single indent before the first argument looks
much nicer and consistent.

Thank you Dan

You are right! It does break the code. Thank you for finding that and
the name issue. I'll rewrite the patch on top of linux-next this time
with the proper style as Neil suggested. I think git send-email added
the danieru. I had tried fixing the name my git config and then git
amending but send-email continued adding the danieru address so I
assumed that it always took the current user's username. Now it sounds
like that was some metadata git did not redo upon amend.

Daniel

2014-05-31 0:51 GMT-06:00 Dan Carpenter <dan.carpenter@oracle.com>:
> What Neil said.
>
> On Fri, May 30, 2014 at 10:31:20PM -0600, Daniel Dressler wrote:
>> From: danieru <danieru.dressler@gmail.com>
>
> This is wrong and we also get your name and email address from the
> email headers so leave this line out.
>
>>
>> Following Greg Kroah-Hartman's newbie guide to hacking
>> the linux kernel this patch addresses only coding style
>> issues.
>>
>> Binder still has many too-long lines but I'm worried
>> doing too much work in a single patch is unfair to the
>> reviewers. So this patch address 20% of the file's
>> issues.
>>
>> There is one change to take notice of: it merges two if
>> statement's conditionals together. Then it removes a
>> redundant else clause.
>
> Thanks for pointing this out, but the else statement wasn't redundant.
> Your change introduces a bug.
>
> Your patch doesn't apply.  You need to work against linux-next.
>
> regards,
> dan carpenter
>