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
