[PATCH 4/7] ASoC: Intel: Skylake: Shield against no-NHLT configurations

[Pierre-Louis Bossart]

On 3/9/20 12:38 PM, Cezary Rojewski wrote:
> On 2020-03-09 18:01, Pierre-Louis Bossart wrote:
>> On 3/9/20 8:03 AM, Cezary Rojewski wrote:
>>> On 2020-03-06 22:03, Pierre-Louis Bossart wrote:
>>>>
>>>>
>>>>> -    intel_nhlt_free(skl->nhlt);
>>>>> +    if (skl->nhlt)
>>>>> +        intel_nhlt_free(skl->nhlt);
>>>>
>>>> we could alternatively move the test in intel_nhlt_free, which seems 
>>>> like a more robust thing to do?
>>>
>>> Depends. In general kernel-internal API trusts its caller and 
>>> appending 'ifs' everywhere would unnecessarily slow entire kernel 
>>> down. While intel_nhlt_free is called rarely, I'd still argue caller 
>>> should be sane about its invocation.
>>>
>>> 'if' in skl_probe could be avoided had the function's structure been 
>>> better. 'if' in skl_remove is just fine, though.
>>>
>>> Let's leave it as is.
>>
>> it's also used in SOF:
>>
>> sound/soc/sof/intel/hda.c:              intel_nhlt_free(nhlt);
>>
>> that's why I suggested to factor the test so that both users don't 
>> need to add the if.
> 
> I understand, and my explanation still applies.
> 
> SOF's intel_nhlt_free usage is great example actually. Caller is sane 
> about its doings as it should be. Internal API needs not to suffer from 
> callers irresponsibility.

ok, indeed looking at the code there's no need for the test.