[alsa-devel] [PATCH v2 3/3] topology: A API calls to directly build topology data from templates

[Takashi Iwai]

On Tue, 11 Aug 2015 17:37:51 +0200,
Lin, Mengdong wrote:
> 
> > -----Original Message-----
> > From: Takashi Iwai [mailto:tiwai at 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.

It's error-prone.  You're calling tplg_elem_free() in other error
cases but only this doesn't.


Takashi