On 05/12/13 08:18, Dan Carpenter wrote:
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.
This patch moves code around so that some of it can be shared with the
compat layer. I agree that due to the abundance of debug code it is, at
times, hard to have a clear idea of what is actually happening and adds
extra obscurities to the logic.
Thanks,
Serban C
_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel