Re: Incorrect automatic ALSA card ID when unicode is at play

[Jaroslav Kysela <perex@perex.cz>]

On 02. 10. 24 18:54, Barnabás Pőcze wrote:
> Hi
> 
> 
> When `snd_card::id` is not specified explicitly it is determined automatically
> in `sound/core/init.c:snd_card_register()` as follows:
> 
>    if (*card->id) {
>      // ...
>    } else {
>      /* create an id from either shortname or longname */
>      const char *src;
> 
>      src = *card->shortname ? card->shortname : card->longname;
>      snd_card_set_id_no_lock(card, src,
>                                    retrieve_id_from_card_name(src));
>    }
> 
> However, `snd_card_set_id_no_lock()`, or more specifically the `copy_valid_id_string()`
> function that it calls does not seem to copy very well with utf-8.
> 
> For example, based on the report at https://gitlab.freedesktop.org/pipewire/pipewire/-/issues/4135
> where `card->shortname="Redmi 电脑音箱"`, `card->id` will be set to \xE7\xE8\xE9\xE7,
> which are the first bytes of the symbols in the suffix "电脑音箱" because only those
> bytes of the string satisfy the `isalnum()` check in `copy_valid_id_string()`.
> 
> I am not sure what kind of 8-bit character set `isalnum()` is basing these results on, but
> it certainly does not mix well with utf-8 in this scenario at least.

It seems that isalnum() also returns true for characters above 0x80 (see 
lib/ctype.c):

0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,                        /* 128-143 */
0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,                        /* 144-159 */
_S|_SP,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,    /* 160-175 */
_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,_P,        /* 176-191 */
_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,_U,        /* 192-207 */
_U,_U,_U,_U,_U,_U,_U,_P,_U,_U,_U,_U,_U,_U,_U,_L,        /* 208-223 */
_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,_L,        /* 224-239 */
_L,_L,_L,_L,_L,_L,_L,_P,_L,_L,_L,_L,_L,_L,_L,_L};       /* 240-255 */

We should probably add isacii() check like:

diff --git a/sound/core/init.c b/sound/core/init.c
index b92aa7103589..114fb87de990 100644
--- a/sound/core/init.c
+++ b/sound/core/init.c
@@ -654,13 +654,19 @@ void snd_card_free(struct snd_card *card)
  }
  EXPORT_SYMBOL(snd_card_free);

+/* check, if the character is in the valid ASCII range */
+static inline bool safe_ascii_char(char c)
+{
+	return isascii(c) && isalnum(c);
+}
+
  /* retrieve the last word of shortname or longname */
  static const char *retrieve_id_from_card_name(const char *name)
  {
  	const char *spos = name;

  	while (*name) {
-		if (isspace(*name) && isalnum(name[1]))
+		if (isspace(*name) && safe_ascii_char(name[1]))
  			spos = name + 1;
  		name++;
  	}
@@ -687,12 +693,12 @@ static void copy_valid_id_string(struct snd_card *card, 
const char *src,
  {
  	char *id = card->id;

-	while (*nid && !isalnum(*nid))
+	while (*nid && !safe_ascii_char(*nid))
  		nid++;
  	if (isdigit(*nid))
  		*id++ = isalpha(*src) ? *src : 'D';
  	while (*nid && (size_t)(id - card->id) < sizeof(card->id) - 1) {
-		if (isalnum(*nid))
+		if (safe_ascii_char(*nid))
  			*id++ = *nid;
  		nid++;
  	}
@@ -787,7 +793,7 @@ static ssize_t id_store(struct device *dev, struct 
device_attribute *attr,

  	for (idx = 0; idx < copy; idx++) {
  		c = buf[idx];
-		if (!isalnum(c) && c != '_' && c != '-')
+		if (!safe_ascii_char(c) && c != '_' && c != '-')
  			return -EINVAL;
  	}
  	memcpy(buf1, buf, copy);


Takashi, do you have a better idea?

				Jaroslav

-- 
Jaroslav Kysela <perex@perex.cz>
Linux Sound Maintainer; ALSA Project; Red Hat, Inc.