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.
PCM does not call ::free() if ::open() fails, same as device-driver
model does not care about ::remove() if ::probe() fails.
Czarek
