Hi,
I submitted a new v4 patch to solve this issue, which will return
-EINVAL right after a failure to find an element's reference. I hope
will make the code more clear to read. So there is no need to modify
tplg_copy_data() now, checking its return value is enough.
The new patch name is [PATCH v4] topology: Return -EINVAL at once on
failure to find a reference. Please review.
Thanks
Mengdong
On 07/22/2016 11:28 AM, Takashi Sakamoto wrote:
On Jul 22 2016 11:39, Lin, Mengdong wrote:
-----Original Message-----
From: Takashi Sakamoto [mailto:o-takashi@xxxxxxxxxxxxx]
Sent: Friday, July 22, 2016 10:16 AM
To: mengdong.lin@xxxxxxxxxxxxxxx; alsa-devel@xxxxxxxxxxxxxxxx;
broonie@xxxxxxxxxx
Cc: tiwai@xxxxxxx; Lin, Mengdong; Girdwood, Liam R; Nc, Shreyas
Subject: Re: [PATCH v2 2/2] topology: Fix the missing referenced elem
ptr
when merging private data
On Jul 22 2016 10:47, mengdong.lin@xxxxxxxxxxxxxxx wrote:
From: Mengdong Lin <mengdong.lin@xxxxxxxxxxxxxxx>
tplg_copy_data() should set the valid referenced data element pointer
on success. The caller will double check this pointer for all kinds of
references, including controls and data.
Signed-off-by: Mengdong Lin <mengdong.lin@xxxxxxxxxxxxxxx>
diff --git a/src/topology/data.c b/src/topology/data.c index
768fc27..e7793b2 100644
--- a/src/topology/data.c
+++ b/src/topology/data.c
@@ -1078,6 +1078,8 @@ int tplg_copy_data(snd_tplg_t *tplg, struct
tplg_elem *elem,
ref_elem->compound_elem = 1;
memcpy(priv->data + old_priv_data_size,
ref_elem->data->data, priv_data_size);
+
+ ref->elem = ref_elem;
return 0;
}
Is this really OK when the found topology element has no private data?
In this case, ref->elem has no assignment at return.
No. This is my mistake. Thanks for finding this bug.
ref->elem should still be assigned for this case. I will make it like
this:
/* overlook empty private data */
if (!ref_elem->data || !ref_elem->data->size) {
ref->elem = ref_elem;
return 0;
}
Let us split data merging code to an explicit function? Then:
if (ref_elem->data != NULL && ref_elem->data->size > 0) {
err = merge_private_data(elem, ref_elem);
if (err < 0)
return err;
}
ref->elem = ref_elem;
return 0;
(Here, I expect compiler optimization to merge the new function to
caller implicitly.)
Regards
Takashi Sakamoto
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
_______________________________________________
Alsa-devel mailing list
Alsa-devel@xxxxxxxxxxxxxxxx
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
