Re: [PATCH v1 1/9] staging: android: binder: Move some of the logic into subfunction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Dec 04, 2013 at 06:09:33PM +0000, Serban Constantinescu wrote:
> +static void bc_increfs_done(struct binder_proc *proc,
> +		struct binder_thread *thread, uint32_t cmd,
> +		void __user *node_ptr, void __user *cookie)
> +{
> +	struct binder_node *node;
> +
> +	node = binder_get_node(proc, node_ptr);
> +	if (node == NULL) {
> +		binder_user_error("%d:%d %s u%p no match\n",
> +			proc->pid, thread->pid,
> +			cmd == BC_INCREFS_DONE ?
> +			"BC_INCREFS_DONE" :
> +			"BC_ACQUIRE_DONE",
> +			node_ptr);
> +		return;
> +	}
> +	if (cookie != node->cookie) {
> +		binder_user_error("%d:%d %s u%p node %d cookie mismatch %p != %p\n",
> +			proc->pid, thread->pid,
> +			cmd == BC_INCREFS_DONE ?
> +			"BC_INCREFS_DONE" : "BC_ACQUIRE_DONE",
> +			node_ptr, node->debug_id,
> +			cookie, node->cookie);
> +		return;
> +	}
> +	if (cmd == BC_ACQUIRE_DONE) {
> +		if (node->pending_strong_ref == 0) {
> +			binder_user_error("%d:%d BC_ACQUIRE_DONE node %d has no pending acquire request\n",
> +				proc->pid, thread->pid,
> +				node->debug_id);
> +			return;
> +		}
> +		node->pending_strong_ref = 0;
> +	} else {
> +		if (node->pending_weak_ref == 0) {
> +			binder_user_error("%d:%d BC_INCREFS_DONE node %d has no pending increfs request\n",
> +				proc->pid, thread->pid,
> +				node->debug_id);
> +			return;
> +		}
> +		node->pending_weak_ref = 0;
> +	}
> +	binder_dec_node(node, cmd == BC_ACQUIRE_DONE, 0);
> +	binder_debug(BINDER_DEBUG_USER_REFS,
> +		     "%d:%d %s node %d ls %d lw %d\n",
> +		     proc->pid, thread->pid,
> +		     cmd == BC_INCREFS_DONE ?
> +		     "BC_INCREFS_DONE" :
> +		     "BC_ACQUIRE_DONE",
> +		     node->debug_id, node->local_strong_refs,
> +		     node->local_weak_refs);
> +	return;
> +}

This function is 52 lines long but at least 32 of those lines are
debug code.

Just extra of lines of code means you have increased the number of bugs
150%.  But what I know is that from a static analysis perspective, debug
code has more defects per line then regular code it's worse than that.
Plus all the extra noise obscures the actual logic and makes the real
code annoying to look at so it's worse still.

If you want to move this stuff out of staging, then remove all the extra
crap so it doesn't look like barf.

regards,
dan carpenter
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel




[Index of Archives]     [Linux Driver Backports]     [DMA Engine]     [Linux GPIO]     [Linux SPI]     [Video for Linux]     [Linux USB Devel]     [Linux Coverity]     [Linux Audio Users]     [Linux Kernel]     [Linux SCSI]     [Yosemite Backpacking]
  Powered by Linux