Messages in this thread | ![/](/images/icornerl.gif) | | Date | Wed, 14 Feb 2024 16:55:45 +0100 | From | Willy Tarreau <> | Subject | Re: [PATCH 3/4] tools/nolibc: Fix strlcpy() return code and size usage |
| |
On Wed, Feb 14, 2024 at 12:50:53PM -0300, Rodrigo Campos wrote: > 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 :)
Never be sorry, low-level user code like this is never trivial and that's the goal of the nolibc-test in the first place ;-)
> 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.
It's not exactly the same, it will always write a zero at dst[size-1] due to the break statement. As much as I hate returns in the middle, this one made sense for this case. A goto to the final return statement is fine as well.
> What do you think? I'm fine with both, of course.
I'm fine with the more readable part (I also prefer it) but not the use of break here.
> If I resend, shall I add a suggested-by or directly you as the author?
No need for either, it's your work, my part was just a review and an addictive temptation to look at asm code ;-)
Cheers, Willy
| ![\](/images/icornerr.gif) |