Received-SPF: pass (google.com: domain of linux-kernel+bounces-65458-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Message-ID: <cc997fd5-1478-43fc-8ba0-aba5e7b3bfdc@sdfg.com.ar>
Date: Wed, 14 Feb 2024 12:50:53 -0300
Precedence: bulk
MIME-Version: 1.0
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH 3/4] tools/nolibc: Fix strlcpy() return code and size
 usage
Content-Language: en-US
To: Willy Tarreau <w@1wt.eu>
Cc: =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <linux@weissschuh.net>,
 linux-kernel@vger.kernel.org
References: <20240129141516.198636-1-rodrigo@sdfg.com.ar>
 <20240129141516.198636-4-rodrigo@sdfg.com.ar> <20240211110814.GB19364@1wt.eu>
From: Rodrigo Campos <rodrigo@sdfg.com.ar>
In-Reply-To: <20240211110814.GB19364@1wt.eu>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit

On 2/11/24 12:08, Willy Tarreau wrote:
> Hi Rodrigo,
> 
> It's good, but for the same reason as the previous one, I'm getting
> smaller code by doing less in the loop. Also calling strlen() here
> looks expensive, I'm seeing that the compiler inlined it nevertheless
> and did it in a dep-optimized way due to the asm statement. That
> results in 67 bytes total while a simpler version gives 47.
> 
> If I explicitly mark strlen() __attribute__((noinline)) that prevents
> it from doing so starting with gcc-10, where it correctly places a jump
> from strlcpy() to strlen() and ends up with 50 bytes (vs 44 for the alt
> one). The other one I can propose is directly derived from the other
> strlcat() variant, which first performs the copy and starts to count:
> 
> size_t strlcpy(char *dst, const char *src, size_t size)
> {
>          size_t len;
> 
>          for (len = 0; len < size; len++) {
>                  if (!(dst[len] = src[len]))
>                          return len;
>          }
> 
>          /* end of src not found before size */
>          if (size)
>                  dst[size - 1] = '\0';
> 
>          while (src[len])
>                  len++;
> 
>          return len;
> }
> 
> Just let me know what you think. 

This is one is very nice, thanks!

Sorry I didn't think about the size at all when writing the functions :)

We can change the loop to be:

         for (len = 0; len < size; len++) {
                 dst[len] = src[len];
                 if (!dst[len])
                         break;
         }

That IMHO it is slightly more readable and makes it only 2 bytes longer 
here.

What do you think? I'm fine with both, of course.


If I resend, shall I add a suggested-by or directly you as the author?


Best,
Rodrigo