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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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



[Index of Archives]     [ALSA User]     [Linux Audio Users]     [Pulse Audio]     [Kernel Archive]     [Asterisk PBX]     [Photo Sharing]     [Linux Sound]     [Video 4 Linux]     [Gimp]     [Yosemite News]

  Powered by Linux