[PATCH v2] ASoC: core: clarify the driver name initialization

[Nícolas F. R. A. Prado]

Hi,

On Thu, Sep 29, 2022 at 04:37:54PM +0200, Jaroslav Kysela wrote:
> The driver field in the struct snd_ctl_card_info is a valid
> user space identifier. Actually, many ASoC drivers do not care
> and let to initialize this field using a standard wrapping method.
> Unfortunately, in this way, this field becomes unusable and
> unreadable for the drivers with longer card names. Also,
> there is a possibility to have clashes (driver field has
> only limit of 15 characters).
> 
> This change will print an error when the wrapping is used.
> The developers of the affected drivers should fix the problem.
> 
> Signed-off-by: Jaroslav Kysela <perex at perex.cz>
> 
> v1..v2:
>   - remove the wrong DMI condition per Mark's review
> ---
>  sound/soc/soc-core.c | 37 +++++++++++++++++++++++--------------
>  1 file changed, 23 insertions(+), 14 deletions(-)
> 
> diff --git a/sound/soc/soc-core.c b/sound/soc/soc-core.c
> index e824ff1a9fc0..590143ebf6df 100644
> --- a/sound/soc/soc-core.c
> +++ b/sound/soc/soc-core.c
> @@ -1840,21 +1840,22 @@ static void soc_check_tplg_fes(struct snd_soc_card *card)
>  	}
>  }
>  
> -#define soc_setup_card_name(name, name1, name2, norm)		\
> -	__soc_setup_card_name(name, sizeof(name), name1, name2, norm)
> -static void __soc_setup_card_name(char *name, int len,
> -				  const char *name1, const char *name2,
> -				  int normalization)
> +#define soc_setup_card_name(card, name, name1, name2) \
> +	__soc_setup_card_name(card, name, sizeof(name), name1, name2)
> +static void __soc_setup_card_name(struct snd_soc_card *card,
> +				  char *name, int len,
> +				  const char *name1, const char *name2)
>  {
> +	const char *src = name1 ? name1 : name2;
>  	int i;
>  
> -	snprintf(name, len, "%s", name1 ? name1 : name2);
> +	snprintf(name, len, "%s", src);
>  
> -	if (!normalization)
> +	if (name != card->snd_card->driver)
>  		return;

The changes do more than the commit message implies by also reworking the need
for the normalization flag, and now that you're special casing most of the
function based on a particular pointer, I wonder if splitting the below logic to
a separate function would make more sense.

In any case,

Reviewed-by: Nícolas F. R. A. Prado <nfraprado at collabora.com>

Thanks,
Nícolas

>  
>  	/*
> -	 * Name normalization
> +	 * Name normalization (driver field)
>  	 *
>  	 * The driver name is somewhat special, as it's used as a key for
>  	 * searches in the user-space.
> @@ -1874,6 +1875,14 @@ static void __soc_setup_card_name(char *name, int len,
>  			break;
>  		}
>  	}
> +
> +	/*
> +	 * The driver field should contain a valid string from the user view.
> +	 * The wrapping usually does not work so well here. Set a smaller string
> +	 * in the specific ASoC driver.
> +	 */
> +	if (strlen(src) > len - 1)
> +		dev_err(card->dev, "ASoC: driver name too long '%s' -> '%s'\n", src, name);
>  }
>  
>  static void soc_cleanup_card_resources(struct snd_soc_card *card)
> @@ -2041,12 +2050,12 @@ static int snd_soc_bind_card(struct snd_soc_card *card)
>  	/* try to set some sane longname if DMI is available */
>  	snd_soc_set_dmi_name(card, NULL);
>  
> -	soc_setup_card_name(card->snd_card->shortname,
> -			    card->name, NULL, 0);
> -	soc_setup_card_name(card->snd_card->longname,
> -			    card->long_name, card->name, 0);
> -	soc_setup_card_name(card->snd_card->driver,
> -			    card->driver_name, card->name, 1);
> +	soc_setup_card_name(card, card->snd_card->shortname,
> +			    card->name, NULL);
> +	soc_setup_card_name(card, card->snd_card->longname,
> +			    card->long_name, card->name);
> +	soc_setup_card_name(card, card->snd_card->driver,
> +			    card->driver_name, card->name);
>  
>  	if (card->components) {
>  		/* the current implementation of snd_component_add() accepts */
> -- 
> 2.35.3
>