Ira Weiny <ira.we...@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 <jmo...@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.
>> break;
>> case FW_EBUSY:
>> @@ -753,7 +751,6 @@ static int query_fw_finish_status(struct ndctl_dimm
>> *dimm,
>> ndctl_dimm_get_devname(dimm));
>> case FW_EINVAL_CTX:
>> case FW_ESEQUENCE:
>> - done = true;
>> rc = -ENXIO;
>> goto out;
>> case FW_ENORES:
>> @@ -761,17 +758,15 @@ static int query_fw_finish_status(struct ndctl_dimm
>> *dimm,
>> "Firmware update sequence timed out: %s\n",
>> ndctl_dimm_get_devname(dimm));
>> rc = -ETIMEDOUT;
>> - done = true;
>> goto out;
>> default:
>> fprintf(stderr,
>> "Unknown update status: %#x on DIMM %s\n",
>> status, ndctl_dimm_get_devname(dimm));
>> rc = -EINVAL;
>> - done = true;
>> goto out;
>> }
>> - } 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.
Thanks for looking, Ira!
-Jeff
_______________________________________________
Linux-nvdimm mailing list -- linux-nvdimm@lists.01.org
To unsubscribe send an email to linux-nvdimm-le...@lists.01.org
