[openib-general] [PATCH] rdma_bind_addr() leaks a cma_dev reference count

[Michael S. Tsirkin]

Quoting r. Krishna Kumar <krkumar2 at in.ibm.com>:
> 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 <krkumar2 at in.ibm.com>
> --------
> 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