On 4/22/24 00:43, Krzysztof Kozlowski wrote:
On 15/04/2024 16:19, Pierre-Louis Bossart wrote:
The core code does not modify the 'struct snd_sof_dsp_ops' passed via pointer in various places, so this can be made pointer to const.
The structure itself is NOT always const - the initialization itself does have platform-specific changes, so what do we really gain from all this?
In the context of these patches, the structure is *always* const. In other drivers, it is not, but they are not really relevant here.
some commit messages say the code is "a bit safer", but I personally find the 'const' more confusing since the information that the structure can be modified during initialization is lost.
Functions which take some data and do not modify it are easier to read if the pointed data is marked as const. Then it is obvious that functions for example is re-entrant. Or that it does not affect the state of other devices/core structures.
Additionally, the static data is safer when is const, because it cannot be used in some attacks.
agree, but here you are marking as 'const' non-static data.
I really do not understand which information you lost here? Core does not change the ops, so the data should be passed as const as often as possible. If anyone wants to write a driver which does not use static ops, but somehow dynamically allocated and changed, nothing stops him. This patch did not make it less readable/doable.
The point is that these ops do not differ from other ops or some other driver-passed structures, which we have around 100 already in checkpatch.
I am so old that I remember times where we had to put things in ROM. That's what 'const' means to me: a dedicated memory space for immutable values.
that's a different interpretation to the 'software' view you're describing. "this structure will not modified by this function" is not the same thing as "this structure CANNOT be modified".
I am not going to lay on the tracks, if Mark wants to apply the patches that's fine. I just wanted to highlight that the reason we did not use 'const' was that the data is dynamically allocated/modified and not constant at all.