Re: [PATCH v2] IB/rdmavt: Fix variable shadowing issue in rvt_create_cq

[Ira Weiny]

On Tue, Jul 09, 2019 at 04:05:53PM -0700, Nathan Chancellor wrote:
> clang warns:
> 
> drivers/infiniband/sw/rdmavt/cq.c:260:7: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>                 if (err)
>                     ^~~
> drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs
> here
>         return err;
>                ^~~
> drivers/infiniband/sw/rdmavt/cq.c:260:3: note: remove the 'if' if its
> condition is always false
>                 if (err)
>                 ^~~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:253:7: warning: variable 'err' is used
> uninitialized whenever 'if' condition is true
> [-Wsometimes-uninitialized]
>                 if (!cq->ip) {
>                     ^~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:310:9: note: uninitialized use occurs
> here
>         return err;
>                ^~~
> drivers/infiniband/sw/rdmavt/cq.c:253:3: note: remove the 'if' if its
> condition is always false
>                 if (!cq->ip) {
>                 ^~~~~~~~~~~~~~
> drivers/infiniband/sw/rdmavt/cq.c:211:9: note: initialize the variable
> 'err' to silence this warning
>         int err;
>                ^
>                 = 0
> 2 warnings generated.

What version of the kernel was this found on?

I don't see the problem with 5.2.  AFAICS there is no 'err' in the function
scope and the if scoped 'err' is initialized properly on line 239.

Ira

> 
> The function scoped err variable is uninitialized when the flow jumps
> into the if statement. The if scoped err variable shadows the function
> scoped err variable, preventing the err assignments within the if
> statement to be reflected at the function level, which will cause
> uninitialized use when the goto statements are taken.
> 
> Just remove the if scoped err declaration so that there is only one
> copy of the err variable for this function.
> 
> Fixes: 239b0e52d8aa ("IB/hfi1: Move rvt_cq_wc struct into uapi directory")
> Link: https://github.com/ClangBuiltLinux/linux/issues/594
> Reviewed-by: Nick Desaulniers <ndesaulniers@xxxxxxxxxx>
> Signed-off-by: Nathan Chancellor <natechancellor@xxxxxxxxx>
> ---
> 
> v1 -> v2:
> 
> * Updated the wording of the commit message to use proper terms like
>   scoping and shadowing, thanks to review from Nick (let me know if the
>   wording isn't up to snuff).
> 
>  drivers/infiniband/sw/rdmavt/cq.c | 2 --
>  1 file changed, 2 deletions(-)
> 
> diff --git a/drivers/infiniband/sw/rdmavt/cq.c b/drivers/infiniband/sw/rdmavt/cq.c
> index fac87b13329d..a85571a4cf57 100644
> --- a/drivers/infiniband/sw/rdmavt/cq.c
> +++ b/drivers/infiniband/sw/rdmavt/cq.c
> @@ -247,8 +247,6 @@ int rvt_create_cq(struct ib_cq *ibcq, const struct ib_cq_init_attr *attr,
>  	 * See rvt_mmap() for details.
>  	 */
>  	if (udata && udata->outlen >= sizeof(__u64)) {
> -		int err;
> -
>  		cq->ip = rvt_create_mmap_info(rdi, sz, udata, u_wc);
>  		if (!cq->ip) {
>  			err = -ENOMEM;
> -- 
> 2.22.0
>