On Thu, Dec 05, 2013 at 11:18:22AM +0300, 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. I don't ever think the binder code will be moved out of staging for a variety of reasons, but that doesn't mean it shouldn't be cleaned up :) thanks, greg k-h _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel