Re: [PATCH 1/1] string: Add stpecpy(3)

[Alejandro Colomar <alx.manpages@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 4175 bytes --]



On 12/24/22 01:05, Alejandro Colomar wrote:
> 
> 
> On 12/24/22 00:24, Wilco Dijkstra wrote:
>> Hi Alex,
>>
>>> For that, we'd first need to discuss what is a typical scenario.
>>
>> Like copying/concatenating strings that fit within the buffer. >
>>> And also, it depends a lot on what the compiler can optimize.  If I call
>>> strlcat(3) in a loop, I know that stpecpy(3) is going to be orders of magnitude
>>> faster.
>>
>> If you're trying to say that the 'strcat' variant is bad then yes absolutely -
> 
> I must admit that it has good things, if you have a compiler that does the magic 
> for you.  GCC optimizes strcat(3) into stpcpy(3), so if you know that it will be 
> optimized, it's not so bad, and the source code is cleaner to the eye.
> 
>> it's better to inline in the compiler or avoid the 'strcat' versions altogether
>> (that's also why I would strongly suggest never to add more 'cat' variants).
> 
> inlining in the compiler is a good solution.  And yes, I agree on not adding cat 
> variants, but at the same time, str functions are problematic in that they can't 
> be chained, as opposed to stp functions.  That problem shows itself in the 
> snprintf(3) bugs I mentioned.  Users need a way to catenate easily, even if not 
> with a 'cat' function.
> 
>> But that doesn't say anything about whether stpecpy is better than strlcpy.
>>
>>> If I call strlcpy(3) in a loop, doing what an ideal compiler might do, that
>>> might be something to benchmark, but we'd also need to discuss what is a good
>>> input for the benchmark.
>>
>> The typical case would be copying or concatenating smallish strings to a buffer.
> 
> Okay, I'll try to prepare a benchmark.
> 
>>
>>> In the OpenBSD definition of strlcpy(), I count 4 branches, and one of them is
>>> inside a while loop.  So, I'd find it very surprising if strlcpy(3) outperformed
>>> stpecpy(3).
>>
>> If that really is the OpenBSD implementation then this proves my point that
>> non-standard string functions are often totally unoptimized.
> 
> And not only that, but I find your version much more readable.  I don't 
> understand how the OpenBSD version was written that way, and hasn't been fixed 
> so far.
> 
>>
>> A basic implementation of strlcpy would use strlen and memcpy so it is fast
>> on every system without requiring any optimization:
>>
>> size_t
>> strlcpy (char *dst, const char *src, size_t size)
>> {
>>    size_t len = strlen (src);
>>
>>    if (size == 0)
>>      return len;
>>    size = len >= size ? size - 1 : len;
> 
> I'd use a separate variable dlen, to differentiate it from size.  Otherwise, it 
> looks like an off-by-one bug just below, since writing at a [size] usually means 
> writing past the array.
> 
>>    dst[size] = 0;
>>    memcpy (dst, src, size);
>>    return len;
>> }
> 
> Then, to compare oranges to oranges, I'll provide the equivalently optimized 
> stpecpy(3):
> 
> char *
> stpecpy (char *dst, char *end, const char *restrict src)
> {
>    size_t dsize;
>    size_t dlen;
>    size_t slen;
> 
>    slen = strlen(src);
> 
>    if (dst == end)
>      return NULL;
>    if (unlikely(dst == NULL))
>      return NULL;
>    if (dst > end)
>      unreachable();
>    dsize = end - dst;
>    dlen = slen >= dsize ? dsize - 1 : slen;
>    dst[dlen] = 0;
>    return mempcpy(dst, src, dlen);
> }

Sorry, I wrote a bug while optimizing: I forgot about the sentinel 'end' return. 
  Now I think it should be fine (anyway, I'll test it soon):

char *
stpecpy (char *dst, char *end, const char *restrict src)
{
   size_t dsize;
   size_t dlen;
   size_t slen = strlen (src);
   bool   trunc = false;

   if (dst == end)
     return NULL;
   if (dst == NULL)
     return NULL;
   dsize = end - dst;
   trunc = (slen >= dsize);
   dlen = trunc ? dsize - 1 : slen;
   dst[dlen] = 0;
   return mempcpy(dst, src, dlen) + trunc;
}

This adds a '+' operation, so the difference compared to your strlcpy(3) is 
smaller, but stpecpy() still wins, I think.


-- 
<http://www.alejandro-colomar.es/>

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]