On (22/04/27 15:35), Péter Ujfalusi wrote: > >> 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. That's what I suspected. > >> 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. OK. I'll remove if from the backport, run another test and will call it a day. As you can guess I was puzzled by that ->data check but it's another very long day in the office for me and in the end I just dropped the ball and decide to suppress clang warning instead. Very smart! (NO). My bad. (YES).
