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

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