Re: [PATCH 4/4] cxl/region: Fix region commit uninitialized variable warning

[Ira Weiny <ira.weiny@intel.com>]

On Wed, Aug 03, 2022 at 12:24:41AM -0700, Dan Williams wrote:
> 0day robot reports:
> 
> drivers/cxl/core/region.c:196 cxl_region_decode_commit() error: uninitialized symbol 'rc'.
> 
> The re-checking of loop termination conditions to determine "success"
> makes it hard to see that @rc is initialized in all cases. Remove those
> to make it explicit that @rc reflects a commit error and that the rest
> of logic is concerned with unwinding committed decoders.
> 
> Fixes: 176baefb2eb5 ("cxl/hdm: Commit decoder state to hardware")
> Reported-by: kernel test robot <lkp@intel.com>
> Signed-off-by: Dan Williams <dan.j.williams@intel.com>
> ---
>  drivers/cxl/core/region.c |   12 +++++-------
>  1 file changed, 5 insertions(+), 7 deletions(-)
> 
> diff --git a/drivers/cxl/core/region.c b/drivers/cxl/core/region.c
> index 5c931b6eb4e7..a68e4e0cf169 100644
> --- a/drivers/cxl/core/region.c
> +++ b/drivers/cxl/core/region.c
> @@ -159,7 +159,7 @@ static int cxl_region_decode_reset(struct cxl_region *cxlr, int count)
>  static int cxl_region_decode_commit(struct cxl_region *cxlr)
>  {
>  	struct cxl_region_params *p = &cxlr->params;
> -	int i, rc;
> +	int i, rc = 0;
>  
>  	for (i = 0; i < p->nr_targets; i++) {
>  		struct cxl_endpoint_decoder *cxled = p->targets[i];
> @@ -180,7 +180,7 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
>  		}
>  
>  		/* success, all decoders up to the root are programmed */
> -		if (is_cxl_root(iter))
> +		if (rc == 0)
>  			continue;
>  
>  		/* programming @iter failed, teardown */
> @@ -192,14 +192,12 @@ static int cxl_region_decode_commit(struct cxl_region *cxlr)
>  		}
>  
>  		cxled->cxld.reset(&cxled->cxld);
> -		if (i == 0)
> -			return rc;
> -		break;
> +		goto err;
>  	}
>  
> -	if (i >= p->nr_targets)
> -		return 0;
> +	return 0;
>  
> +err:
>  	/* undo the targets that were successfully committed */
>  	cxl_region_decode_reset(cxlr, i);

Doesn't this take care of teardown now?  Does the for loop even need to do
teardown now?

Ira

>  	return rc;
>