Re: [PATCH alsa-lib 2/8] topology: decode: Fix adding texts field to enum control

[Pierre-Louis Bossart <pierre-louis.bossart@linux.intel.com>]

On 6/25/20 6:03 AM, Piotr Maziarz wrote:
> Iterating over texts in tplg_decode_control_enum1 was an infinite loop,

that should probably be a separate fix?

> it needed to be fixed. Parsing values was removed since they are not added
> to the UML file.

What does this mean? first time I hear about UML for topology.

> Also texts are separate section and therefore it needs to be added as
> separate element.

separate patch?

> Signed-off-by: Piotr Maziarz <piotrx.maziarz@linux.intel.com>
> ---
>   src/topology/ctl.c | 19 ++++++++++++-------
>   1 file changed, 12 insertions(+), 7 deletions(-)
> 
> diff --git a/src/topology/ctl.c b/src/topology/ctl.c
> index c8c7e94..24eadc8 100644
> --- a/src/topology/ctl.c
> +++ b/src/topology/ctl.c
> @@ -1088,11 +1088,19 @@ int tplg_add_enum(snd_tplg_t *tplg, struct snd_tplg_enum_template *enum_ctl,
>   	}
>   
>   	if (enum_ctl->texts != NULL) {
> +		struct tplg_elem *texts = tplg_elem_new_common(tplg, NULL,
> +						enum_ctl->hdr.name, SND_TPLG_TYPE_TEXT);
> +
> +		texts->texts->num_items = num_items;
>   		for (i = 0; i < num_items; i++) {
> -			if (enum_ctl->texts[i] != NULL)
> -				snd_strlcpy(ec->texts[i], enum_ctl->texts[i],
> -					    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
> +			if (!enum_ctl->texts[i])
> +				continue;
> +			snd_strlcpy(ec->texts[i], enum_ctl->texts[i],
> +				    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
> +			snd_strlcpy(texts->texts->items[i], enum_ctl->texts[i],
> +				    SNDRV_CTL_ELEM_ID_NAME_MAXLEN);
>   		}
> +		tplg_ref_add(elem, SND_TPLG_TYPE_TEXT, enum_ctl->hdr.name);
>   	}
>   
>   	if (enum_ctl->values != NULL) {
> @@ -1367,11 +1375,8 @@ int tplg_decode_control_enum1(snd_tplg_t *tplg,
>   		et->texts = tplg_calloc(heap, sizeof(char *) * ec->items);
>   		if (!et->texts)
>   			return -ENOMEM;
> -		for (i = 0; ec->items; i++) {
> -			unsigned int j = i * sizeof(int) * ENUM_VAL_SIZE;
> +		for (i = 0; i < ec->items; i++)

This is the infinite loop, this should not be buried in unrelated changes.

>   			et->texts[i] = ec->texts[i];
> -			et->values[i] = (int *)&ec->values[j];
> -		}
>   	}
>   
>   	et->map = tplg_calloc(heap, sizeof(struct snd_tplg_channel_map_template));
>