-----Original Message----- From: Takashi Iwai [mailto:tiwai@suse.de] Sent: Tuesday, August 11, 2015 3:58 PM
On Mon, 10 Aug 2015 20:13:49 +0200, Liam Girdwood wrote:
+int tplg_add_bytes(snd_tplg_t *tplg, struct snd_tplg_bytes_template
*bytes_ctl,
- struct tplg_elem **e)
+{
- struct snd_soc_tplg_bytes_control *be;
- struct tplg_elem *elem;
- int ret;
- tplg_dbg(" Control Bytes: %s\n", bytes_ctl->hdr.name);
- if (bytes_ctl->hdr.type != SND_SOC_TPLG_TYPE_BYTES) {
SNDERR("error: invalid bytes type %d\n", bytes_ctl->hdr.type);return -EINVAL;- }
- elem = tplg_elem_new_common(tplg, NULL, bytes_ctl->hdr.name,
SND_TPLG_TYPE_BYTES);- if (!elem)
return -ENOMEM;- be = elem->bytes_ext;
- be->size = elem->size;
- ret = init_ctl_hdr(&be->hdr, &bytes_ctl->hdr);
- if (ret < 0) {
tplg_elem_free(elem);return ret;- }
- be->max = bytes_ctl->max;
- be->mask = bytes_ctl->mask;
- be->base = bytes_ctl->base;
- be->num_regs = bytes_ctl->num_regs;
- be->ext_ops.put = bytes_ctl->ext_ops.put;
- be->ext_ops.get = bytes_ctl->ext_ops.get;
- if (bytes_ctl->priv != NULL) {
be = realloc(be,elem->size + bytes_ctl->priv->size);if (!be) {tplg_elem_free(elem);return -ENOMEM;}elem->bytes_ext = be;elem->size += bytes_ctl->priv->size;memcpy(be->priv.data, bytes_ctl->priv->data,bytes_ctl->priv->size);be->priv.size = bytes_ctl->priv->size;- }
- /* check on TLV bytes control */
- if (be->hdr.access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) {
if (be->hdr.access & SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE!= SNDRV_CTL_ELEM_ACCESS_TLV_READWRITE) {SNDERR("error: Invalid TLV bytes control access 0x%x\n",be->hdr.access);return -EINVAL;Missing tplg_elem_free()?
tplg_elem_free() is not needed here. tplg_elem_new_common() has inserted the element into a specific list according to the element type. And snd_tplg_free() will be called at last even on error, and it will scan all lists and free the elements.
+struct tplg_elem* tplg_elem_new_route(snd_tplg_t *tplg) {
- struct tplg_elem *elem;
- struct snd_soc_tplg_dapm_graph_elem *line;
- elem = tplg_elem_new();
- if (!elem)
return NULL;- list_add_tail(&elem->list, &tplg->route_list);
- strcpy(elem->id, "line");
- elem->type = SND_TPLG_TYPE_DAPM_GRAPH;
- elem->size = sizeof(*line);
- line = calloc(1, sizeof(*line));
- if (!line)
return NULL;Missing free of elem in the error path. Also, tplg->route_list still contains the stray elem.
I guess this was a bug in the original code, so you can address it before this patch.
Same as above. snd_tplg_free() is expected to do the final cleanup.
+int tplg_add_widget_object(snd_tplg_t *tplg, snd_tplg_obj_template_t +*t) {
- struct snd_tplg_widget_template *wt = t->widget;
- struct snd_soc_tplg_dapm_widget *w;
- struct tplg_elem *elem;
- int i, ret = 0;
- tplg_dbg("Widget: %s\n", wt->name);
- elem = tplg_elem_new_common(tplg, NULL, wt->name,
SND_TPLG_TYPE_DAPM_WIDGET);- if (!elem)
return -ENOMEM;- /* init new widget */
- w = elem->widget;
- w->size = elem->size;
- w->id = wt->id;
- elem_copy_text(w->name, wt->name,
SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
- if (wt->sname)
elem_copy_text(w->sname, wt->sname,SNDRV_CTL_ELEM_ID_NAME_MAXLEN);Indentation problem.
- /* add controls to the widget's reference list */
- for (i = 0 ; i < wt->num_ctls; i++) {
struct snd_tplg_ctl_template *ct = wt->ctl[i];struct tplg_elem *elem_ctl;struct snd_tplg_mixer_template *mt;struct snd_tplg_bytes_template *bt;struct snd_tplg_enum_template *et;if (!ct) {tplg_elem_free(elem);return -EINVAL;}switch (ct->type) {case SND_SOC_TPLG_TYPE_MIXER:mt = container_of(ct, struct snd_tplg_mixer_template, hdr);ret = tplg_add_mixer(tplg, mt, &elem_ctl);break;case SND_SOC_TPLG_TYPE_BYTES:bt = container_of(ct, struct snd_tplg_bytes_template, hdr);ret = tplg_add_bytes(tplg, bt, &elem_ctl);break;case SND_SOC_TPLG_TYPE_ENUM:et = container_of(ct, struct snd_tplg_enum_template, hdr);ret = tplg_add_enum(tplg, bt, &elem_ctl);et instead of bt?
Yes. It should be et here.
Thanks Mengdong