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: [..] > > > - } while (!done); > > > + } while (true); > > > > I'm not a fan of "while (true)". But I'm not the maintainer. The Logic > > seems > > fine otherwise. > > The way things stand today is a mashup of goto vs. break. I'll > follow-up with fixed up patch next week if there is consensus on the > change. If you have a suggestion for a better way, that's welcome as > well. > I've applied this as is for v67, we can look at a refactoring for the while (true) later. _______________________________________________ Linux-nvdimm mailing list -- [email protected] To unsubscribe send an email to [email protected]
