On 01/19/2017 11:08 PM, SF Markus Elfring wrote:
quoted
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.
quoted
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