Re: [ndctl patch 3/4] query_fw_finish_status: get rid of redundant variable

["Verma, Vishal L" <vishal.l.verma@intel.com>]

On Wed, 2019-10-23 at 22:28 +0000, Verma, Vishal L wrote:
> On Fri, 2019-10-18 at 17:06 -0400, Jeff Moyer wrote:
> > Ira Weiny <ira.weiny@intel.com> writes:
> > > On Fri, Oct 18, 2019 at 04:23:01PM -0400, Jeff Moyer wrote:
> > > > The 'done' variable only adds confusion.
> > > > 
> > > > Signed-off-by: Jeff Moyer <jmoyer@redhat.com>
> > > > ---
> > > >  ndctl/dimm.c | 7 +------
> > > >  1 file changed, 1 insertion(+), 6 deletions(-)
> > > > 
> > > > diff --git a/ndctl/dimm.c b/ndctl/dimm.c
> > > > index c8821d6..f28b9c1 100644
> > > > --- a/ndctl/dimm.c
> > > > +++ b/ndctl/dimm.c
> > > > @@ -682,7 +682,6 @@ static int query_fw_finish_status(struct ndctl_dimm *dimm,
> > > >  	struct ndctl_cmd *cmd;
> > > >  	int rc;
> > > >  	enum ND_FW_STATUS status;
> > > > -	bool done = false;
> > > >  	struct timespec now, before, after;
> > > >  	uint64_t ver;
> > > >  
> > > > @@ -716,7 +715,6 @@ static int query_fw_finish_status(struct ndctl_dimm *dimm,
> > > >  					ndctl_dimm_get_devname(dimm));
> > > >  			printf("Firmware version %#lx.\n", ver);
> > > >  			printf("Cold reboot to activate.\n");
> > > > -			done = true;
> > > >  			rc = 0;
> > > 
> > > Do we need "goto out" here?
> > 
> > Yes, I missed that one.  Thanks.
> 
> This actually looks fine, since there is a 'break' down below.
> 
> > > >  			break;
> > > >  		case FW_EBUSY:

(Watching the unit test run fall into an infinite loop..) Nope, the
break is in the switch scope, the while loop needs the 'goto out'.

Yes this bit definitely needs to be refactored :)
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-leave@lists.01.org