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

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

On Wed, Aug 03, 2022 at 02:49:37PM -0700, Ira wrote:
> 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?

Never mind I think I see how this logic is the same.

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> Ira
> 
> >  	return rc;
> >