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.