[PATCH v2] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload
Péter Ujfalusi
peter.ujfalusi at linux.intel.com
Wed Apr 27 14:35:08 CEST 2022
On 27/04/2022 15:31, Sergey Senozhatsky wrote:
> On (22/04/27 15:08), Péter Ujfalusi wrote:
>>> clang appears to be unhappy otherwise.
>>>
>>> error: comparison of array 'cdata->data' equal to a null pointer is always false
>>>
>>> Changing this into `if (!cdata->data)` is a little bit better as now
>>> 'always false' becomes 'always true'
>>>
>>> error: address of array 'cdata->data' will always evaluate to 'true'
>>
>> Hrm, uhm. clang is right. The check is (and was) bogus...
>>
>> cdata->data is a pointer (to cdata->data[0]) which is always:
>> cdata + sizeof(struct sof_ipc_ctrl_data).
>> Checking if it is NULL or not is irrelevant and wrong. If we do not have
>> additional data then cdata->data points to memory which is outside of
>> the struct and it can be random data (might be 0, might not be).
>
> Yeah to be honest that's what I'm thinking too.
>
> Does sof_ipc_ctrl_data have to be a var-sized structure? Or can that union
> hold pointers that are allocated separately?
>
> scontrol->data = kzalloc(sizeof sof_ipc_ctrl_data);
> scontrol->data->chan = kzalloc(sizeof chan * mc->num_channels)
Unfortunately no, the data/chanv/compv needs to be flexible array as it
is the IPC message itself.
>
>> I think we can just drop this check as we would not be here if
>> additional data was not allocated for the payload prior?
>
> I don't have enough knowledge of this code. ->data check doesn't do what
> it is expected to do so removing it shouldn't do harm.
Let me quickly send v3 with dropped cdata->data check.
--
Péter
More information about the Alsa-devel
mailing list