Re: [PATCH v2] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload

[Sergey Senozhatsky <senozhatsky@chromium.org>]

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).