Re: [PATCH alsa-lib 2/4] seq: Add API helper functions for creating UMP Endpoint and Blocks

["Amadeusz Sławiński" <amadeuszx.slawinski@linux.intel.com>]

On 7/31/2024 11:21 AM, Takashi Iwai wrote:
> On Wed, 31 Jul 2024 10:46:08 +0200,
> Amadeusz Sławiński wrote:
>>
>> On 6/19/2024 5:28 PM, Takashi Iwai wrote:
>>> For making it easer for applications to create a virtual UMP Endpoint
>>> and UMP blocks, add two API helper functions.
>>>
>>> snd_seq_create_ump_endpoint() creates (unsurprisingly) a UMP Endpoint,
>>> based on the given snd_ump_endpoint_info_t information.  The number of
>>> (max) UMP groups belonging to this Endpoint has to be specified.
>>> This function sets up the Endpoint info on the sequencer client, and
>>> creates a MIDI 2.0 UMP port as well as UMP Group ports automatically.
>>> The name of the sequencer client is updated from the Endpoint name,
>>> too.
>>>
>>> After creating a UMP Endpoint, create each UMP Block via
>>> snd_seq_create_ump_block() function with a snd_ump_block_info_t info.
>>> The associated groups for each block have to be specified there.
>>> The port names and capability bits are updated accordingly after
>>> setting each block information.
>>>
>>> Signed-off-by: Takashi Iwai <tiwai@suse.de>
>>> ---
>>
>> ...
>>
>>> +			if (*blknames) {
>>> +				strlcat(blknames, ", ", sizeof(blknames));
>>> +				strlcat(blknames, (const char *)bp->name,
>>> +					sizeof(blknames));
>>
>> FYI, this seems to introduce build problems on systems that do not
>> have strlcpy:
>>
>> During build:
>> seqmid.c: In function ‘update_group_ports’:
>> seqmid.c:672:33: warning: implicit declaration of function
>> ‘strlcat’; did you mean ‘strncat’?
>> [-Wimplicit-function-declaration]
>>    672 |                                 strlcat(blknames, ", ",
>> sizeof(blknames));
>>        |                                 ^~~~~~~
>>        |                                 strncat
>>
>> And then during linking:
>> /usr/bin/ld: seq/.libs/libseq.a(seqmid.o): in function `update_group_ports':
>> /home/amade/workdir/avs/alsa-lib/src/seq/seqmid.c:672: undefined
>> reference to `strlcat'
>> /usr/bin/ld: /home/amade/workdir/avs/alsa-lib/src/seq/seqmid.c:673:
>> undefined reference to `strlcat'
>> collect2: error: ld returned 1 exit status
> 
> Thanks, I'll modify it to avoid strlcat() like below.
> 
> 
> Takashi
> 
> -- 8< --
> Subject: [PATCH] seq: Avoid strlcat()
> 
> strlcat() isn't available in every system, so better to avoid it.
> Rewrite the code without strlcat().
> 
> Fixes: 6167b8ce3e80 ("seq: Add API helper functions for creating UMP Endpoint and Blocks")
> Link: https://lore.kernel.org/0796c157-1ac3-47a3-9d54-ba86f59d64d5@linux.intel.com
> Signed-off-by: Takashi Iwai <tiwai@suse.de>
> ---
>   src/seq/seqmid.c | 12 ++++++------
>   1 file changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/src/seq/seqmid.c b/src/seq/seqmid.c
> index 08c62d5c24b8..b30db4075254 100644
> --- a/src/seq/seqmid.c
> +++ b/src/seq/seqmid.c
> @@ -635,6 +635,7 @@ static void update_group_ports(snd_seq_t *seq, snd_ump_endpoint_info_t *ep)
>   		char blknames[64];
>   		char name[64];
>   		unsigned int caps = 0;
> +		int len;
>   
>   		blknames[0] = 0;
>   		for (b = 0; b < ep->num_blocks; b++) {
> @@ -668,14 +669,13 @@ static void update_group_ports(snd_seq_t *seq, snd_ump_endpoint_info_t *ep)
>   
>   			if (!*bp->name)
>   				continue;
> -			if (*blknames) {
> -				strlcat(blknames, ", ", sizeof(blknames));
> -				strlcat(blknames, (const char *)bp->name,
> -					sizeof(blknames));
> -			} else {
> +			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)

Builds now, but still gives warning:

seqmid.c: In function ‘update_group_ports’:
seqmid.c:675:45: warning: ‘%s’ directive output may be truncated writing 
up to 127 bytes into a region of size 61 [-Wformat-truncation=]
   675 |                                          ", %s", bp->name);
       |                                             ^~
In file included from /usr/include/stdio.h:894,
                  from ../../include/local.h:28,
                  from seq_local.h:26,
                  from seqmid.c:23:
In function ‘snprintf’,
     inlined from ‘update_group_ports’ at seqmid.c:674:5:
/usr/include/x86_64-linux-gnu/bits/stdio2.h:71:10: note: 
‘__builtin___snprintf_chk’ output between 3 and 130 bytes into a 
destination of size 63
    71 |   return __builtin___snprintf_chk (__s, __n, 
__USE_FORTIFY_LEVEL - 1,
       | 
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    72 |                                    __glibc_objsize (__s), __fmt,
       |                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    73 |                                    __va_arg_pack ());
       |                                    ~~~~~~~~~~~~~~~~~