> look up Documentation/CodingStyle Chapter 7: Centralized exiting of functions > which says unconditional statements are easier to understand and follow, > and note that with this style nesting is reduced.
Hmmm, OK, I will re-phrase this patch to reduce nesting. thanks, - KK "Michael S. Tsirkin" <[EMAIL PROTECTED]> wrote on 10/17/2006 10:53:03 AM: > Quoting r. Krishna Kumar <[EMAIL PROTECTED]>: > > Subject: [PATCH] rdma_bind_addr() leaks a cma_dev reference count > > > > rdma_bind_addr leaks a cma_dev reference count in failure > > case. > > > > Signed-off-by: Krishna Kumar <[EMAIL PROTECTED]> > > -------- > > diff -ruNp org/drivers/infiniband/core/cma.c new/drivers/infiniband/core/cma.c > > --- org/drivers/infiniband/core/cma.c 2006-10-09 17:13:41.000000000 +0530 > > +++ new/drivers/infiniband/core/cma.c 2006-10-09 19:42:31.000000000 +0530 > > @@ -1749,6 +1749,7 @@ static int cma_get_port(struct rdma_id_p > > int rdma_bind_addr(struct rdma_cm_id *id, struct sockaddr *addr) > > { > > struct rdma_id_private *id_priv; > > + int did_acquire_dev = 0; > > int ret; > > > > if (addr->sa_family != AF_INET) > > @@ -1765,18 +1766,20 @@ int rdma_bind_addr(struct rdma_cm_id *id > > ret = cma_acquire_dev(id_priv); > > mutex_unlock(&lock); > > } > > - if (ret) > > - goto err; > > + if (!ret) > > + did_acquire_dev = 1; > > + else > > + goto out; > > } > > - > > memcpy(&id->route.addr.src_addr, addr, ip_addr_size(addr)); > > ret = cma_get_port(id_priv); > > - if (ret) > > - goto err; > > > > - return 0; > > -err: > > - cma_comp_exch(id_priv, CMA_ADDR_BOUND, CMA_IDLE); > > +out: > > + if (ret) { > > + if (did_acquire_dev) > > + cma_detach_from_dev(id_priv); > > + cma_comp_exch(id_priv, CMA_ADDR_BOUND, CMA_IDLE); > > + } > > return ret; > > } > > EXPORT_SYMBOL(rdma_bind_addr); > > Ugh, replacing two labels with an if statement is uglifying code: look how > nesting got 2-deep already. Please do the error handling the usual linux way > without testing flags: > > if (ret) > goto err > > .... > err: > cma_comp_exch(id_priv, CMA_ADDR_BOUND, CMA_IDLE); > out: > return ret; > > is better than > if (ret) > goto out; > > .... > > out: > if (ret) > cma_comp_exch(id_priv, CMA_ADDR_BOUND, CMA_IDLE); > > look up Documentation/CodingStyle Chapter 7: Centralized exiting of functions > which says unconditional statements are easier to understand and follow, > and note that with this style nesting is reduced. > > -- > MST _______________________________________________ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general