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 <[email protected]> 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 <[email protected]> > > > > --- > > > > 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 -- [email protected] To unsubscribe send an email to [email protected]
