From: Sergey Senozhatsky <senozhatsky@chromium.org>
To: "Péter Ujfalusi" <peter.ujfalusi@linux.intel.com>
Cc: pierre-louis.bossart@linux.intel.com,
alsa-devel@alsa-project.org, kai.vehmanen@linux.intel.com,
cujomalainey@google.com, ranjani.sridharan@linux.intel.com,
lgirdwood@gmail.com,
Sergey Senozhatsky <senozhatsky@chromium.org>,
broonie@kernel.org
Subject: Re: [PATCH v2] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload
Date: Wed, 27 Apr 2022 21:31:09 +0900 [thread overview]
Message-ID: <Ymk3jYPd1lpvfjBE@google.com> (raw)
In-Reply-To: <03a5d9ee-90ef-e4b9-5117-e59e81407453@linux.intel.com>
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)
> 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.
next prev parent reply other threads:[~2022-04-27 12:32 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-27 10:52 [PATCH v2] ASoC: SOF: ipc3-topology: Correct get_control_data for non bytes payload Peter Ujfalusi
2022-04-27 11:20 ` Sergey Senozhatsky
2022-04-27 11:33 ` Péter Ujfalusi
2022-04-27 11:54 ` Sergey Senozhatsky
2022-04-27 12:08 ` Péter Ujfalusi
2022-04-27 12:31 ` Sergey Senozhatsky [this message]
2022-04-27 12:35 ` Péter Ujfalusi
2022-04-27 12:41 ` Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Ymk3jYPd1lpvfjBE@google.com \
--to=senozhatsky@chromium.org \
--cc=alsa-devel@alsa-project.org \
--cc=broonie@kernel.org \
--cc=cujomalainey@google.com \
--cc=kai.vehmanen@linux.intel.com \
--cc=lgirdwood@gmail.com \
--cc=peter.ujfalusi@linux.intel.com \
--cc=pierre-louis.bossart@linux.intel.com \
--cc=ranjani.sridharan@linux.intel.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox