Re: powerpc/nvram: Move an assignment for the variable "ret" in dev_nvram_write()

From: Tyrel Datwyler
Date: Fri Jan 20 2017 - 15:54:24 EST


On 01/19/2017 11:08 PM, SF Markus Elfring wrote:
>> I think you really could have squashed patches 1-3 into a single patch
>> that returns directly after any failure.
>
> Thanks for your constructive feedback.
>
> I have got software development concerns around such patch squashing.
>
>
>> At this point you might as well remove that label and move the kfree(tmp) call up
>> and return directly after the failure and at the nvram_write() call site
>> doing away completely with the "ret" variable.
>
> Your idea might look nice at first glance. But I would interpret the previous
> implementation of the discussed function in the way that the memory which was
> dynamically allocated here should always (not only in the failure case) be released
> before returning here.

You are correct. I did muck that part up. However, I do still believe it
is cleaner to squash your three patches together. There is no functional
change here and it is clearer in a single patch that you are modifying
the function to return directly in the simple error cases.

-Tyrel

>
> Would you really like to change the life time for this âtemporaryâ data item?
>
> Regards,
> Markus
>