Message-ID: <5bd05f4d-a427-4812-802e-f5adaa30b7ac@sdfg.com.ar>
Date: Wed, 14 Feb 2024 19:47:25 -0300
From: Rodrigo Campos <rodrigo@...g.com.ar>
To: Willy Tarreau <w@....eu>
Cc: Thomas Weißschuh <linux@...ssschuh.net>,
 linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/4] tools/nolibc: Fix strlcat() return code and size
 usage

On 2/14/24 23:03, Rodrigo Campos wrote:
> On 2/14/24 16:34, Rodrigo Campos wrote:
>> size_t strlcat_rata(char *dst, const char *src, size_t size)
>> {
>>          const char *orig_src = src;
>>          size_t len = 0;
>>          for (;len < size; len++) {
>>                  if (dst[len] == '\0')
>>                          break;
>>          }
> 
> If you think about it, this is strnlen() and what follows is strncat().
> 
>> size_t strlcat_willy_fixed(char *dst, const char *src, size_t size)
>> {
>>          const char *orig_src = src;
>>          size_t len = strlen(dst);
>>          if (size < len)
>>                  len = size;
> 
> Same here.
> 
> We can simplify the code by using them, but the size doesn't seem to be 
> smaller. Let me see what I can do.

Yeap, third option would be this:

size_t strlcat(char *dst, const char *src, size_t size)
{
         size_t len = strnlen(dst, size);
         if(size > len) {
                 strncpy(dst + len, src, size - len - 1);
                 dst[size - 1] = '\0';
         }
         return len + strlen(src);
}

I've tried to use strncpy return code to avoid doing the strlen(src), 
but that is only useful if we copy all of src. Otherwise, we still need 
to go till the end. And the function code ends-up being bigger because 
we can only do it inside the if (and return from there), so we have to 
add code there and keep the strlen(src) outside.

I'm not sure about the size of this variant, as different programs give 
me different sizes.

For example, if I use it in this program:

int main(void) {
         char buf[10] = "test123456";
         buf[4] = '\0';
         char *src = "bar";
         size_t ret = strlcat(buf, src, 9);

         printf("dst is: %s\n", buf);
         printf("ret is: %d\n", ret);

         printf("strlen(buf) is: %d\n", strlen(buf));
         printf("strlen(src) is: %d\n", strlen(src));

	return 0;
}

It is compiled to 74 bytes. This is smaller than the other two options I 
posted.

But if I just use it in a wrapper function, as you suggested, like:

size_t test_strlcat_nlen(char *dst, const char *src, size_t size)
{
         return strlcat(dst, src, size);
}

And compile like this:

	gcc -xc -c -Os -fno-asynchronous-unwind-tables -fno-ident -s -Os  -I 
sysroot/x86/include -include nolibc.h strlcat_sizes.c -o test.o

or even like this, the result is the same:

	gcc -xc -c -Os -I sysroot/x86/include -include nolibc.h strlcat_sizes.c 
-o test.o

The result is 86 bytes, which is similar to the previous versions (81 
and 90 bytes they were).

I haven't checked the assembler generated, I bet it is being smart and 
taking advantage of the set size param when it makes it so much smaller. 
I've tried calling it with more numbers than just "9", in lot of cases 
it is smaller than 86 bytes.

I have the gut-feeling that for the types of programs that us nolibc, 
all params might be known (dst, src, size) and we won't need to keep a 
generic version of the function, the compiler will optimize it. Even 
calling it several times in the compiled program, forcing it out and 
inside the "if size > len)" case, the optimized version is still in the 
76 bytes or so.

So, the code is so simple, the size is almost the same and it seems the 
compiler has a better time optimizing it. I lean towards this version here.


What do you think?


Best,
Rodrigo

Powered by blists - more mailing lists