[PATCH 02/14] drivers/firmware/sdei: Common block for failing path in sdei_event_create()

Gavin Shan gshan at redhat.com
Tue Jul 21 22:12:31 EDT 2020


Hi James,

On 7/22/20 6:40 AM, James Morse wrote:
> On 06/07/2020 06:47, Gavin Shan wrote:
>> The failing path in sdei_event_create() is to free SDEI event and
>> return the corresponding error. This introduces common block of
>> code for that to avoid duplicated logics.
> 
> By replacing it with slightly different duplicated logic?
> If there were some kind of state to unwind here, I'd agree a single block to return from
> is less likely to have bugs. Until then, this change is just churn.
> 

The problem is there are multiple calls to kfree(event) in sdei_event_create().
It's prone to cause memory leakage when more code is added to the failing
path. To have common/single block to free @event and return errno can avoid
the situation as you mentioned. I will adjust the commit log to make it more
explicit in v2.

> 
>> By the way, the function
>> scoped variables are also reordered according to their importance.
> 
> Please: Never do this.
> 
> The 'order' is usually to make it easier to resolve merge conflicts. If two people add an
> entry, at the same time. Its normally a 'fir' tree, and all that really matters is its
> consistent. This change is just churn.
> 
> 
> No Thanks,
> 

Ok. I will drop this part of change in v2.

Thanks,
Gavin




More information about the linux-arm-kernel mailing list