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

Reply via email to