recent change in seqmid.c is broken?

recent change in seqmid.c is broken?

[Takashi Iwai]

Hi Jaroslav,

I'm afraid that your recent fix for alsa-lib commit a4e47461eca1
doesn't work as expected:

```
@@ -664,15 +663,11 @@ static void update_group_ports(snd_seq_t *seq, snd_ump_endpoint_info_t *ep)
                                break;
                        }
 
-                       if (!*bp->name)
+                       if (bp->name[0] == '\0')
                                continue;
-                       len = strlen(blknames);
-                       if (len)
-                               snprintf(blknames + len, sizeof(blknames) - len,
-                                        ", %s", bp->name);
-                       else
-                               snd_strlcpy(blknames, (const char *)bp->name,
-                                           sizeof(blknames));
+                       if (blknames[0])
+                               snd_strlcpy(blknames, ", ", sizeof(blknames));
+                       snd_strlcpy(blknames, (const char *)bp->name, sizeof(blknames));
                }
 
                if (!*blknames)
```

The original code appended the new bp->name string with the prefix of
", " if blknames is already present, but the new code looks as if it
overwrites onto blknames with strlcpy() from scratch for each
bp->name.

FWIW, the code there used to be with strlcat(), but it was rewritten
in the way above because strlcat() isn't always available in commit
d9694398130c.


thanks,

Takashi

Re: recent change in seqmid.c is broken?

[Jaroslav Kysela]

On 14. 04. 25 17:44, Takashi Iwai wrote:
> Hi Jaroslav,
> 
> I'm afraid that your recent fix for alsa-lib commit a4e47461eca1
> doesn't work as expected:
> 
> ```
> @@ -664,15 +663,11 @@ static void update_group_ports(snd_seq_t *seq, snd_ump_endpoint_info_t *ep)
>                                  break;
>                          }
>   
> -                       if (!*bp->name)
> +                       if (bp->name[0] == '\0')
>                                  continue;
> -                       len = strlen(blknames);
> -                       if (len)
> -                               snprintf(blknames + len, sizeof(blknames) - len,
> -                                        ", %s", bp->name);
> -                       else
> -                               snd_strlcpy(blknames, (const char *)bp->name,
> -                                           sizeof(blknames));
> +                       if (blknames[0])
> +                               snd_strlcpy(blknames, ", ", sizeof(blknames));
> +                       snd_strlcpy(blknames, (const char *)bp->name, sizeof(blknames));
>                  }
>   
>                  if (!*blknames)
> ```
> 
> The original code appended the new bp->name string with the prefix of
> ", " if blknames is already present, but the new code looks as if it
> overwrites onto blknames with strlcpy() from scratch for each
> bp->name.
> 
> FWIW, the code there used to be with strlcat(), but it was rewritten
> in the way above because strlcat() isn't always available in commit
> d9694398130c.

Oops.... I am at the end of 1.2.14 release procedure.

I'm trying to put the correct fix, could you check quickly latest two commits?

		Thanks,
				Jaroslav


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

Re: recent change in seqmid.c is broken?

[Jaroslav Kysela]

On 14. 04. 25 18:45, Jaroslav Kysela wrote:
> On 14. 04. 25 17:44, Takashi Iwai wrote:
>> Hi Jaroslav,
>>
>> I'm afraid that your recent fix for alsa-lib commit a4e47461eca1
>> doesn't work as expected:
>>
>> ```
>> @@ -664,15 +663,11 @@ static void update_group_ports(snd_seq_t *seq, snd_ump_endpoint_info_t *ep)
>>                                   break;
>>                           }
>>    
>> -                       if (!*bp->name)
>> +                       if (bp->name[0] == '\0')
>>                                   continue;
>> -                       len = strlen(blknames);
>> -                       if (len)
>> -                               snprintf(blknames + len, sizeof(blknames) - len,
>> -                                        ", %s", bp->name);
>> -                       else
>> -                               snd_strlcpy(blknames, (const char *)bp->name,
>> -                                           sizeof(blknames));
>> +                       if (blknames[0])
>> +                               snd_strlcpy(blknames, ", ", sizeof(blknames));
>> +                       snd_strlcpy(blknames, (const char *)bp->name, sizeof(blknames));
>>                   }
>>    
>>                   if (!*blknames)
>> ```
>>
>> The original code appended the new bp->name string with the prefix of
>> ", " if blknames is already present, but the new code looks as if it
>> overwrites onto blknames with strlcpy() from scratch for each
>> bp->name.
>>
>> FWIW, the code there used to be with strlcat(), but it was rewritten
>> in the way above because strlcat() isn't always available in commit
>> d9694398130c.
> 
> Oops.... I am at the end of 1.2.14 release procedure.
> 
> I'm trying to put the correct fix, could you check quickly latest two commits?

The fix is in 1.2.14 alsa-lib release.

				Jaroslav

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

Re: recent change in seqmid.c is broken?

[Takashi Iwai]

On Mon, 14 Apr 2025 18:45:04 +0200,
Jaroslav Kysela wrote:
> 
> On 14. 04. 25 17:44, Takashi Iwai wrote:
> > Hi Jaroslav,
> > 
> > I'm afraid that your recent fix for alsa-lib commit a4e47461eca1
> > doesn't work as expected:
> > 
> > ```
> > @@ -664,15 +663,11 @@ static void update_group_ports(snd_seq_t *seq, snd_ump_endpoint_info_t *ep)
> >                                  break;
> >                          }
> >   -                       if (!*bp->name)
> > +                       if (bp->name[0] == '\0')
> >                                  continue;
> > -                       len = strlen(blknames);
> > -                       if (len)
> > -                               snprintf(blknames + len, sizeof(blknames) - len,
> > -                                        ", %s", bp->name);
> > -                       else
> > -                               snd_strlcpy(blknames, (const char *)bp->name,
> > -                                           sizeof(blknames));
> > +                       if (blknames[0])
> > +                               snd_strlcpy(blknames, ", ", sizeof(blknames));
> > +                       snd_strlcpy(blknames, (const char *)bp->name, sizeof(blknames));
> >                  }
> >                    if (!*blknames)
> > ```
> > 
> > The original code appended the new bp->name string with the prefix of
> > ", " if blknames is already present, but the new code looks as if it
> > overwrites onto blknames with strlcpy() from scratch for each
> > bp->name.
> > 
> > FWIW, the code there used to be with strlcat(), but it was rewritten
> > in the way above because strlcat() isn't always available in commit
> > d9694398130c.
> 
> Oops.... I am at the end of 1.2.14 release procedure.
> 
> I'm trying to put the correct fix, could you check quickly latest two commits?

Looks good to me.  Thanks!


Takashi