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

On Tue, 11 Aug 2015 17:37:51 +0200, Lin, Mengdong wrote:

...

-----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.

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

Takashi