On 03/07/2017 05:20 PM, Linda Knippers wrote:
> On 03/06/2017 03:32 PM, Dave Jiang wrote:
>> Make sure that xlat_status is unconditionally called.
> 
> Is this just code cleanup or is it fixing something?

This is code clean up requested by Dan.

> 
>>
>> Signed-off-by: Dave Jiang <[email protected]>
>> ---
>>  drivers/acpi/nfit/core.c |   15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c
>> index 7361d00..9d4f461 100644
>> --- a/drivers/acpi/nfit/core.c
>> +++ b/drivers/acpi/nfit/core.c
>> @@ -199,7 +199,7 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor *nd_desc, 
>> struct nvdimm *nvdimm,
>>      acpi_handle handle;
>>      unsigned int func;
>>      const u8 *uuid;
>> -    int rc, i;
>> +    int rc = 0, xlat_rc, i;
>>  
>>      func = cmd;
>>      if (cmd == ND_CMD_CALL) {
>> @@ -343,21 +343,20 @@ int acpi_nfit_ctl(struct nvdimm_bus_descriptor 
>> *nd_desc, struct nvdimm *nvdimm,
>>                       * unfilled in the output buffer
>>                       */
>>                      rc = buf_len - offset - in_buf.buffer.length;
>> -                    if (cmd_rc)
>> -                            *cmd_rc = xlat_status(nvdimm, buf, cmd,
>> -                                            fw_status);
>>              } else {
>>                      dev_err(dev, "%s:%s underrun cmd: %s buf_len: %d 
>> out_len: %d\n",
>>                                      __func__, dimm_name, cmd_name, buf_len,
>>                                      offset);
>>                      rc = -ENXIO;
>> +                    goto out;
>>              }
>> -    } else {
>> -            rc = 0;
>> -            if (cmd_rc)
>> -                    *cmd_rc = xlat_status(nvdimm, buf, cmd, fw_status);
>>      }
>>  
>> +    xlat_rc = xlat_status(nvdimm, buf, cmd, fw_status);
>> +
>> +    if (cmd_rc)
>> +            *cmd_rc = xlat_rc;
> 
> Is there some benefit of calling xlat_status and then throwing away the 
> result?

It's not exactly thrown away. I tried to preserve the original code path
as much as possible. So in the current case, if xlat_status() fails,
then *cmd_rc gets it like in the original code. If it's succeeds, then
the forget_poison provides the status instead.

> 
>> +
>>   out:
>>      ACPI_FREE(out_obj);
>>  
>>
>> _______________________________________________
>> Linux-nvdimm mailing list
>> [email protected]
>> https://lists.01.org/mailman/listinfo/linux-nvdimm
>>
> 
_______________________________________________
Linux-nvdimm mailing list
[email protected]
https://lists.01.org/mailman/listinfo/linux-nvdimm

Reply via email to