[Sound-open-firmware] suspicious code in hda-ipc.c

[Guennadi Liakhovetski]

Hi,

This code in hda_dsp_ipc_get_reply() in hda-ipc.c looks suspicious to
me:

	hdr = msg->msg_data;
	if (hdr->cmd == (SOF_IPC_GLB_PM_MSG | SOF_IPC_PM_CTX_SAVE)) {
		...
	} else {
		/* get IPC reply from DSP in the mailbox */
		sof_mailbox_read(sdev, sdev->host_box.offset, &reply,
				 sizeof(reply));
	}
	if (reply.error < 0) {
		size = sizeof(reply);
		ret = reply.error;
	} else {
		...
	}

	/* read the message */
	if (msg->msg_data && size > 0)
		sof_mailbox_read(sdev, sdev->host_box.offset,
				 msg->reply_data, size);

So, as far as I understand, first of all the first part of the
"if (msg->msg_data && ...)" test is always true, since we've already
dereferenced "hdr" above. Secondly if reply contains a negative error
the reply struct will be read from the mailbox again, instead of just
using "memcpy(msg->reply_data, &reply, sizeof(reply));" to copy it?
The same pattern is also repeated in other IPC implementations.

Thanks
Guennadi