ASoC: SOF: ipc4: check return value #4577
Conversation
bardliao
requested review from
lgirdwood,
kv2019i,
dbaluta,
vijendarmukunda,
kuanhsuncheng and
yaochunhung
as code owners
sound/soc/sof/ipc4.c
Outdated
| snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr, ipc4_msg->data_size); | ||
| err = snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr, ipc4_msg->data_size); | ||
| if (err < 0) { | ||
| dev_warn(sdev->dev, "failed to read IPC reply data: %d\n", err); |
There was a problem hiding this comment.
this might be actually an error here?
There was a problem hiding this comment.
and the text is incorrect, this is not a reply, this is a notification.
There was a problem hiding this comment.
and the text is incorrect, this is not a reply, this is a notification.
ack
There was a problem hiding this comment.
this might be actually an error here?
I changed it to dev_err. To be honestly, I am not sure what is the impact if we don't get the data.
There was a problem hiding this comment.
@bardliao, it is an error for sure. Some messages must have data in payload, like the RESOURCE_EVENT and the will be added MODULE_NOTIFICATION.
What I'm not sure is that will it ever happen? I think not, but have not checked it.
sound/soc/sof/ipc4.c
Outdated
| } | ||
|
|
||
| sof_ipc4_log_header(sdev->dev, "ipc rx done ", ipc4_msg, true); | ||
|
|
||
| out: |
There was a problem hiding this comment.
Does this applies at all?
We are now dumping the payload if data_size is not NULL and we might dump invalid data if the snd_sof_ipc_msg_data() failed.
There was a problem hiding this comment.
Does this applies at all? We are now dumping the payload if data_size is not NULL and we might dump invalid data if the snd_sof_ipc_msg_data() failed.
Yes, it didn't conflict. But, yes, it might dump invalid data if the snd_sof_ipc_msg_data() failed. So, return err immediately.
| @@ -631,11 +631,15 @@ static void sof_ipc4_rx_msg(struct snd_sof_dev *sdev) | |||
| return; | |||
|
|
|||
| ipc4_msg->data_size = data_size; | |||
| snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr, ipc4_msg->data_size); | |||
| err = snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr, ipc4_msg->data_size); | |||
There was a problem hiding this comment.
and for the record: this cannot fail in this context as the second parameter is NULL.
sound/soc/sof/ipc4.c
Outdated
| err = snd_sof_ipc_msg_data(sdev, NULL, ipc4_msg->data_ptr, ipc4_msg->data_size); | ||
| if (err < 0) { | ||
| dev_err(sdev->dev, "failed to read IPC notification data: %d\n", err); | ||
| return err; |
There was a problem hiding this comment.
you still need to do the cleanup for the allocated buffer...
There was a problem hiding this comment.
you still need to do the cleanup for the allocated buffer...
fixed
snd_sof_ipc_msg_data could return error. Signed-off-by: Bard Liao <[email protected]>
plbossart
merged commit 776cbce
into
thesofproject:topic/sof-dev
snd_sof_ipc_msg_data could return error.