Re: alx-0096r1 - string (and nonstring) copying

Alejandro Colomar wrote:
> >  Also some systems
> > (e.g. with segmented or banked memory models) have a ptrdiff_t that is
> > larger than size_t.
>
> I'm interested in those systems.  Do they support POSIX.1-2024 and/or
> ISO C23?  We were discussing the possibility of standardizing ssize_t in
> ISO C, and also adding a requirement that it's as large as size_t, and
> also potentially adding a requirement that it's the same type as
> ptrdiff_t.
>
> It would be good to know if those systems that have a larger ptrdiff_t
> are still alive or can be ignored/forced to adapt.

SDCC (https://sdcc.sourceforge.net/), when building for mcs51
(-mmcs51) or ds390 (-mds390), uses a 16-bit size_t and 32-bit
ptrdiff_t.  It appears that Open Watcom C/16 v2
(https://github.com/open-watcom/open-watcom-v2) with the huge memory
model also uses a 16-bit size_t and a 32-bit ptrdiff_t, although I
haven't verified that one.  Both compilers have active development and
support some of the new features in C23, but do not have full C23 or
POSIX support.  I don't know about any others with ptrdiff_t larger
than size_t.

>
> > > [...]
> > > > >     System V; memccpy(3)
> > > > >         memccpy(3) was invented in System V (like the other mem*()
> > > > >         functions).  The System V sources are not public (AFAIK), so
> > > > >         it's not known what this function was designed for.  It was
> > > > >         later added to the BSDs and glibc for compatibility with SysV,
> > > > >         but nobody really knew what this function is good for.  It
> > > > >         doesn't seem ergonomic for any basic functionality.
> > > >
> > > > It is for implementing functions like fgets(), which needs to copy the
> > > > next '\n'-delimited line from a buffer filled by read() (so not a
> > > > null-terminated string).
> > >
> > > Hmmmm, do you have any such code?  I'd be interested in reviewing it.
> > > I still suspect it might have been better if it didn't copy the
> > > terminator.
> >
> > Sure.
> > https://github.com/illumos/illumos-gate/blob/4b44494cae63d4bf0c7d3f34828503e68d1c0e69/usr/src/lib/libc/port/stdio/fgets.c#L44
>
> Thanks!
>
> Let's now consider an alternative world where there was a function with
> semantics similar to memccpy(3) with one difference: it wouldn't copy
> the delimiter.  Let's call such a function memcpyc().  Here's a naive
> implementation of memccpy(3):
>
>         void *
>         memccpy(void *dst, const void *src, int c, size_t n)
>         {
>                 unsigned char  *d = dst;
>                 unsigned char  *s = src;
>
>                 for (size_t i = 0; i < n; i++) {
>                         *d++ = *s++;
>                         if (*s == c)
>                                 return d;
>                 }
>                 return NULL;
>         }
>
> Here's a naive implementation of the hypothetical memcpyc():
>
>         void *
>         memcpyc(void *dst, const void *src, int c, size_t n)
>         {
>                 unsigned char  *d = dst;
>                 unsigned char  *s = src;
>
>                 for (size_t i = 0; i < n; i++) {
>                         if (*s == c)
>                                 return d;
>                         *d++ = *s++;
>                 }
>                 return NULL;
>         }
>
> (I've written both of the above quickly, without compiling nor testing;
>  I might have made a mistake.)
>
> With such a function, the fgets(3) implementation from Illumos gate
> would change in this manner:
>
>         diff --git i/usr/src/lib/libc/port/stdio/fgets.c w/usr/src/lib/libc/port/stdio/fgets.c
>         index 9401217410ca..ac1f330ea46e 100644
>         --- i/usr/src/lib/libc/port/stdio/fgets.c
>         +++ w/usr/src/lib/libc/port/stdio/fgets.c
>         @@ -82,9 +82,12 @@ fgets(char *buf, int size, FILE *iop)
>                                         break;          /* nothing left to read */
>                         }
>                         n = (int)(size < iop->_cnt ? size : iop->_cnt);
>         -               if ((p = memccpy(ptr, (char *)iop->_ptr, '\n',
>         +               if ((p = memcpyc(ptr, (char *)iop->_ptr, '\n',
>                             (size_t)n)) != NULL)
>         +               {
>         +                       p = stpcpy(p, "\n");
>                                 n = (int)(p - ptr);
>         +               }
>                         ptr += n;
>                         iop->_cnt -= n;
>                         iop->_ptr += n;
>
> IMO, this wouldn't make it more complex.  However, such a hypothetical
> memcpyc() would be much more useful elsewhere.  For example, it would
> simplify the two unique uses of memccpy(3) in FreeBSD:
>
>         diff --git i/bin/sh/parser.c w/bin/sh/parser.c
>         index 0c1b7a91c257..713a35ad8cd8 100644
>         --- i/bin/sh/parser.c
>         +++ w/bin/sh/parser.c
>         @@ -2116,7 +2116,7 @@ getprompt(void *unused __unused)
>                                 if (fmt[0] != '}') {
>                                         char *end;
>
>         -                               end = memccpy(tfmt, fmt, '}', sizeof(tfmt));
>         +                               end = memcpyc(tfmt, fmt, '}', sizeof(tfmt));
>                                         if (end == NULL) {
>                                                 /*
>                                                  * Format too long or no '}', so
>         @@ -2129,7 +2129,7 @@ getprompt(void *unused __unused)
>                                                 fmt--;
>                                                 break;
>                                         }
>         -                               *--end = '\0'; /* Ignore the copy of '}'. */
>         +                               *end = '\0'; /* Ignore the copy of '}'. */
>                                         fmt += end - tfmt;
>                                 }
>                                 now = localtime(&(time_t){time(NULL)});
>         diff --git i/lib/libc/amd64/string/strncat.c w/lib/libc/amd64/string/strncat.c
>         index 2c63ab50b3c3..4a1e8c0119ac 100644
>         --- i/lib/libc/amd64/string/strncat.c
>         +++ w/lib/libc/amd64/string/strncat.c
>         @@ -19,13 +19,13 @@ strncat(char *dest, const char *src, size_t n)
>                 char *endptr;
>
>                 len = strlen(dest);
>         -       endptr = __memccpy(dest + len, src, '\0', n);
>         +       endptr = memcpyc(dest + len, src, '\0', n);
>
>                 /* avoid an extra branch */
>                 if (endptr == NULL)
>         -               endptr = dest + len + n + 1;
>         +               endptr = dest + len + n;
>
>         -       endptr[-1] = '\0';
>         +       *endptr = '\0';
>
>                 return (dest);
>          }
>
> memccpy(3) is a bad design, which forces complex code, prone to
> off-by-one bugs.

It seems like you've already made up your mind that memccpy() is bad
and are now just looking to justify that decision.

Your modification of fgets() is obviously more complex; it adds a
whole additional function call, and instead of copying all of the
bytes at once from a single source it does two copies from two
different sources, specifying the delimiter twice making it more
error-prone and difficult to change.  Both fgets() and strncat() need
the delimiter in the destination buffer, and the one place that
doesn't is only "simplified" by removing a single "--"; certainly not
worth a whole new function.  And if simplicity or eliminating the
+/-1s was important to the author of that strncat() implementation,
they could trivially achieve those goals with the existing function:

    size_t len = strlen(dest);
    if (__memccpy(dest + len, src, '\0', n) == NULL)
        dest[len+n] = '\0';
    return dest;

Changing that to use memcpyc() would have made it more complex.

I think that copying the delimiter makes sense.  For example, you
should be able to use this function directly in a loop, e.g. to
process '\n'-delimited lines.  If it stopped before the delimiter it
would need extra code to skip over the delimiter before the next
iteration to avoid getting stuck there.  But if the final line didn't
have a '\n' at the end it would have to be sure not to skip over it to
avoid advancing past the end of the buffer.

Even if this hypothetical memcpyc() function was slightly better I
don't see how that matters.  memccpy() has been around for decades, is
already in ISO C, is already in POSIX, has already been implemented in
numerous C libraries even before ISO C existed, and is in use in
non-buggy ways, so I don't see it going away in the foreseeable
future.  The purpose of the standard is to allow for confident
reliance on a defined base set of features, and removing functions
from that set that are in use and have no fundamental flaws erodes
that confidence.  memcpyc() has no existing uses, and also little
expected use, so it would be difficult to imagine there being enough
support to add it to ISO C.


 - Mark