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

Sergey Senozhatsky
2022-04-27T12:42:11Z

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