Re: [PATCH 3/5] acpi: cleanup acpi_nfit_ctl calling xlat_status

[Dave Jiang <dave.jiang@intel.com>]

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 <dave.jiang@intel.com>
>> ---
>>  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
>> Linux-nvdimm@lists.01.org
>> https://lists.01.org/mailman/listinfo/linux-nvdimm
>>
> 
_______________________________________________
Linux-nvdimm mailing list
Linux-nvdimm@lists.01.org
https://lists.01.org/mailman/listinfo/linux-nvdimm