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

[Noah Goldstein]

On Sun, Dec 25, 2022 at 6:37 AM Alejandro Colomar
<alx.manpages@gmail.com> wrote:
>
> Hi Noah,
>
> On 12/25/22 02:52, Noah Goldstein wrote:
>
> >> char *
> >> stpecpy (char *dst, char *end, const char *restrict src)
> >> {
> >>     size_t dsize;
> >>     size_t dlen;
> >>     size_t slen = strlen (src);
> >
> > Imo move `dst == end` and `dst == NULL` check before strlen
>
>
> That's a valid optimization.  Having strlen(3) before has the advantage that you
> make sure that your strings are strings, as strlcpy(3) does.  But since we're
> inventing stpecpy(3), we can choose to be faster.  If anyone wants to instrument
> their code, they can add a temporary wrapper that does that.
>
> > and change strlen to `strnlen(src, dsize)`.
>
> About strnlen(3), I have doubts.  Isn't strlen(3) faster for the common case of
> no truncation or little truncation?  strnlen(3) would optimize for the case
> where you truncate by a large difference.

It's faster if strlen(s) <= strnlen(s, N) (maybe up to N + 32).

But generally I think of it like `qsort`. Most data gets n * log(n) behavior
but still it's worth preventing the worst case for minor constant cost.


>
> >>     bool   trunc = false;
> >>
> >>     if (dst == end)
> >
> > Out of curiosity what if `end < dst`?
>
> The behavior is undefined.  That's by design.  In the definition of stpecpy(3) I
> have currently in libstp, I even tell the compiler to optimize on that condition:
> <http://www.alejandro-colomar.es/src/alx/alx/libstp.git/tree/include/stp/stpe/stpecpy.h#n33>
>

You could probably optimize out one of the branches along the line of:
if((dst - 1UL) >= (end - 1UL)) {
    // if dst == NULL, then dst - 1UL -> SIZE_MAX and must be >= any value.
    // if dst == end, then (dst - 1UL) >= (end - 1UL) will be true.
    return NULL;
}
>
> alx@asus5775:~/src/alx/libstp$ grepc -tfd stpecpy
> ./include/stp/stpe/stpecpy.h:21:
> inline char *stp_nullable
> stpecpy(char *stp_nullable dst, char *end, const char *restrict src)
> {
>         bool    trunc;
>         size_t  dsize, dlen, slen;
>
>         slen = strlen(src);
>
>         if (dst == end)
>                 return end;
>         if (stp_unlikely(dst == NULL))  // Allow chaining with stpeprintf().
>                 return NULL;
>         stp_impossible(dst > end);
>
>         dsize = end - dst;
>         trunc = (slen >= dsize);
>         dlen = trunc ? dsize - 1 : slen;
>         dst[dlen] = '\0';
>
>         return mempcpy(dst, src, dlen) + trunc;
> }
> alx@asus5775:~/src/alx/libstp$ grepc -tm stp_impossible
> ./include/stp/_compiler.h:14:
> #define stp_impossible(e)   do                                                \
> {                                                                             \
>         if (e)                                                                \
>                 stp_unreachable();                                            \
> } while (0)
> alx@asus5775:~/src/alx/libstp$ grep -rnC1 define.stp_unreachable
> include/stp/_compiler.h-28-#if defined(unreachable)
> include/stp/_compiler.h:29:# define stp_unreachable()  unreachable()
> include/stp/_compiler.h-30-#else
> include/stp/_compiler.h:31:# define stp_unreachable()  __builtin_unreachable()
> include/stp/_compiler.h-32-#endif
>
>
> I'd do that for glibc, but I don't see any facility.  Maybe we should add an
> __impossible() macro to document UB, and help the compiler.

Does it result in any improved codegen? If not seems like
making it fail more noisily is always a win.
>
> Cheers,
>
> Alex
>
> >>       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/>
>
> --
> <http://www.alejandro-colomar.es/>