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.