[PATCH 0/4] improvements for strncpy.3 etc

[PATCH 0/4] improvements for strncpy.3 etc

[Paul Eggert]

Improve man pages for strncpy and related functions.
Do this partly by using clearer terminology (e.g.,
"byte" rather than "character" which is ambiguous),
partly by being clearer about whether arguments need
to be strings, and partly by using better sample
implementations and examples.

This patch does not affect man pages for the corresponding
wide-character functions.  These functions are rarely used
and so are lower priority.

It also does not affect string_copying(7) which needs similar
work; I ran out of time for now.

Paul Eggert (4):
  * Remove man3/stpecpyx.3; no longer present.
  string.3 fixes
  strncat.3 fixes
  stpncpy(3) fixes

 man3/stpecpyx.3 |  1 -
 man3/stpncpy.3  | 82 ++++++++++++++++++++++++-------------------------
 man3/string.3   |  4 +--
 man3/strncat.3  | 54 ++++++++++++++------------------
 4 files changed, 65 insertions(+), 76 deletions(-)
 delete mode 100644 man3/stpecpyx.3

--
2.41.0

[PATCH 1/4] * Remove man3/stpecpyx.3; no longer present.

[Paul Eggert]

---
 man3/stpecpyx.3 | 1 -
 1 file changed, 1 deletion(-)
 delete mode 100644 man3/stpecpyx.3

diff --git a/man3/stpecpyx.3 b/man3/stpecpyx.3
deleted file mode 100644
index beb850746..000000000
--- a/man3/stpecpyx.3
+++ /dev/null
@@ -1 +0,0 @@
-.so man7/string_copying.7
-- 
2.41.0

Re: [PATCH 1/4] * Remove man3/stpecpyx.3; no longer present.

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 623 bytes --]

Hi Paul,

On Sun, Nov 12, 2023 at 03:52:05PM -0800, Paul Eggert wrote:

Patch applied.
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=c67f69f50c7e20800113e73b946ff7b2b51f1733>

Thanks,
Alex

> ---
>  man3/stpecpyx.3 | 1 -
>  1 file changed, 1 deletion(-)
>  delete mode 100644 man3/stpecpyx.3
> 
> diff --git a/man3/stpecpyx.3 b/man3/stpecpyx.3
> deleted file mode 100644
> index beb850746..000000000
> --- a/man3/stpecpyx.3
> +++ /dev/null
> @@ -1 +0,0 @@
> -.so man7/string_copying.7
> -- 
> 2.41.0
> 
> 

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

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

[PATCH 2/4] string.3 fixes

[Paul Eggert]

strncpy’s src need not be a string; it can be an array that
is not null-terminated.  Reword so as to not imply that
the result always has some null padding.  Use “byte” rather
than “character” to avoid issues with “character”, as
not every byte is a valid character in (for example) UTF-8 locales.
Replace “‐” (U+2020 HYPHEN) with “-” (U+002D HYPHEN-MINUS).
---
 man3/string.3 | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/man3/string.3 b/man3/string.3
index bd8b342a6..4037fce47 100644
--- a/man3/string.3
+++ b/man3/string.3
@@ -185,8 +185,8 @@ bytes to
 const char " src "[restrict ." n ],
 .BI "       size_t " n );
 .fi
-Fill a fixed‐width buffer with characters from a string
-and pad with null bytes.
+Fill a fixed-size buffer with non-null bytes from a source array,
+padding with null bytes as needed.
 .SH DESCRIPTION
 The string functions perform operations on null-terminated
 strings.
-- 
2.41.0

Re: [PATCH 2/4] string.3 fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2163 bytes --]

Hi Paul,

On Sun, Nov 12, 2023 at 03:52:06PM -0800, Paul Eggert wrote:
> strncpy’s src need not be a string; it can be an array that
> is not null-terminated.  Reword so as to not imply that
> the result always has some null padding.  Use “byte” rather
> than “character” to avoid issues with “character”, as
> not every byte is a valid character in (for example) UTF-8 locales.
> Replace “‐” (U+2020 HYPHEN) with “-” (U+002D HYPHEN-MINUS).

I fixed the hyphen typo in a separate commit:
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=6be4cc77bb91dd25c48952b647ff49d52c16ab7a>

> ---
>  man3/string.3 | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/man3/string.3 b/man3/string.3
> index bd8b342a6..4037fce47 100644
> --- a/man3/string.3
> +++ b/man3/string.3
> @@ -185,8 +185,8 @@ bytes to
>  const char " src "[restrict ." n ],
>  .BI "       size_t " n );
>  .fi
> -Fill a fixed‐width buffer with characters from a string
> -and pad with null bytes.
> +Fill a fixed-size buffer with non-null bytes from a source array,

I'm not convinced by the array wording, as it could be understood as
doing this:

	char d[3], s[3] = {'a', '\0', 'b'};

	strncpy(d, s, 3);  // "a\0b"?  Or maybe "ab\0"?

Did it copy the non-null byte 'b'?  It is certainly part of the array,
and is non-null.  While saying that it only reads strings is incomplete,
I'm not sure this alternative is complete either.

> +padding with null bytes as needed.

I've split the patch into several pieces:
-  Clarify that dst isn't padded if there's no room.
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=1873da38de4be3d36bb67d011b907f6f774eef82>

-  Talk about bytes, not characters:
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=f51cbcca1c0e12e94590bd9723e3e446ac50c33a>

Thanks!
Alex

>  .SH DESCRIPTION
>  The string functions perform operations on null-terminated
>  strings.
> -- 
> 2.41.0
> 
> 

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

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

Re: [PATCH 2/4] string.3 fixes

[Paul Eggert]

[-- Attachment #1: Type: text/plain, Size: 391 bytes --]

On 11/12/23 16:53, Alejandro Colomar wrote:
> I'm not convinced by the array wording, as it could be understood as
> doing this:
> 
> 	char d[3], s[3] = {'a', '\0', 'b'};
> 
> 	strncpy(d, s, 3);  // "a\0b"?  Or maybe "ab\0"?
> 
> Did it copy the non-null byte 'b'?

OK, let's fix that confusion by saying it copies "leading non-null 
bytes", not merely "non-null bytes". Please see attached.

[-- Attachment #2: 0001-string.3-fixes.patch --]
[-- Type: text/x-patch, Size: 1021 bytes --]

From ac104738b8b13159271c40a86341f871fa260e33 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Mon, 13 Nov 2023 14:26:42 -0800
Subject: [PATCH] string.3 fixes
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Say "size" not "width" for byte counts; "width" is for the number
of bits in a word.  Say "leading" to make it clear we don’t care
about what the source contains, after its first null byte.
---
 man3/string.3 | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/man3/string.3 b/man3/string.3
index 4c374a7df..08b050b17 100644
--- a/man3/string.3
+++ b/man3/string.3
@@ -185,7 +185,7 @@ bytes to
 const char " src "[restrict ." n ],
 .BI "       size_t " n );
 .fi
-Fill a fixed-width buffer with non-null bytes from a source string,
+Fill a fixed-size buffer with leading non-null bytes from a source array,
 padding with null bytes as needed.
 .SH DESCRIPTION
 The string functions perform operations on null-terminated
-- 
2.41.0

Re: [PATCH 2/4] string.3 fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1841 bytes --]

Hi Paul,

On Mon, Nov 13, 2023 at 02:27:51PM -0800, Paul Eggert wrote:
> On 11/12/23 16:53, Alejandro Colomar wrote:
> > I'm not convinced by the array wording, as it could be understood as
> > doing this:
> > 
> > 	char d[3], s[3] = {'a', '\0', 'b'};
> > 
> > 	strncpy(d, s, 3);  // "a\0b"?  Or maybe "ab\0"?
> > 
> > Did it copy the non-null byte 'b'?
> 
> OK, let's fix that confusion by saying it copies "leading non-null bytes",
> not merely "non-null bytes". Please see attached.

LGTM.  Patch applied.
<https://www.alejandro-colomar.es/src/alx/linux/man-pages/man-pages.git/commit/?h=contrib&id=0726f57d8362553b697fa19c405f7fc77f2e42dd>

Cheers,
Alex

> From ac104738b8b13159271c40a86341f871fa260e33 Mon Sep 17 00:00:00 2001
> From: Paul Eggert <eggert@cs.ucla.edu>
> Date: Mon, 13 Nov 2023 14:26:42 -0800
> Subject: [PATCH] string.3 fixes
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
> 
> Say "size" not "width" for byte counts; "width" is for the number
> of bits in a word.  Say "leading" to make it clear we don’t care
> about what the source contains, after its first null byte.
> ---
>  man3/string.3 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/man3/string.3 b/man3/string.3
> index 4c374a7df..08b050b17 100644
> --- a/man3/string.3
> +++ b/man3/string.3
> @@ -185,7 +185,7 @@ bytes to
>  const char " src "[restrict ." n ],
>  .BI "       size_t " n );
>  .fi
> -Fill a fixed-width buffer with non-null bytes from a source string,
> +Fill a fixed-size buffer with leading non-null bytes from a source array,
>  padding with null bytes as needed.
>  .SH DESCRIPTION
>  The string functions perform operations on null-terminated
> -- 
> 2.41.0
> 


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

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

[PATCH 3/4] strncat.3 fixes

[Paul Eggert]

Don't say "concatenate". Use "byte" instead of "character",
and use standalone terminology rather than relying on the
reader already having read string_copying(7).
Don't say "width" when "size" was intended.
Fix indenting of prototype.
Simplify possible implementation, fixing a bug when the
source string length and sz exceed INT_MAX.
Say that strncat is rarely useful.
Say that behavior is undefined if the destination is not a string.
Simplify example by using plain sizeof rather than an nitems macro,
by removing a confusingly-named 'maxsize' local,
and by removing an unnecessary call to 'exit'.
---
 man3/strncat.3 | 54 +++++++++++++++++++++-----------------------------
 1 file changed, 23 insertions(+), 31 deletions(-)

diff --git a/man3/strncat.3 b/man3/strncat.3
index d0f777d36..9a7df474a 100644
--- a/man3/strncat.3
+++ b/man3/strncat.3
@@ -5,7 +5,8 @@
 .\"
 .TH strncat 3 (date) "Linux man-pages (unreleased)"
 .SH NAME
-strncat \- concatenate a null-padded character sequence into a string
+strncat \- append non-null bytes from a source array to a string,
+and null-terminate the result
 .SH LIBRARY
 Standard C library
 .RI ( libc ", " \-lc )
@@ -14,15 +15,18 @@ Standard C library
 .B #include <string.h>
 .P
 .BI "char *strncat(char *restrict " dst ", const char " src "[restrict ." sz ],
-.BI "               size_t " sz );
+.BI "              size_t " sz );
 .fi
 .SH DESCRIPTION
-This function catenates the input character sequence
-contained in a null-padded fixed-width buffer,
-into a string at the buffer pointed to by
+This function appends at most
+.I sz
+non-null bytes from the array pointed to by
+.I src
+to the end of the string pointed to by
 .IR dst .
-The programmer is responsible for allocating a destination buffer large enough,
-that is,
+.I dst
+must point to a string contained in a buffer that is large enough,
+that is, the buffer size must be at least
 .IR "strlen(dst) + strnlen(src, sz) + 1" .
 .P
 An implementation of this function might be:
@@ -32,12 +36,7 @@ An implementation of this function might be:
 char *
 strncat(char *restrict dst, const char *restrict src, size_t sz)
 {
-    int   len;
-    char  *p;
-\&
-    len = strnlen(src, sz);
-    p = dst + strlen(dst);
-    p = mempcpy(p, src, len);
+    char *p = mempcpy(dst + strlen(dst), src, strnlen(src, sz));
     *p = \[aq]\e0\[aq];
 \&
     return dst;
@@ -67,11 +66,12 @@ C11, POSIX.1-2008.
 .SH HISTORY
 POSIX.1-2001, C89, SVr4, 4.3BSD.
 .SH CAVEATS
-The name of this function is confusing.
-This function has no relation to
+The name of this function is confusing, as it has no relation to
 .BR strncpy (3).
+This function is rarely useful in practice.
 .P
-If the destination buffer is not large enough,
+If the destination buffer does not already contain a string,
+or is not large enough,
 the behavior is undefined.
 See
 .B _FORTIFY_SOURCE
@@ -91,40 +91,32 @@ Shlemiel the painter
 #include <stdlib.h>
 #include <string.h>
 \&
-#define nitems(arr)  (sizeof((arr)) / sizeof((arr)[0]))
-\&
 int
 main(void)
 {
-    size_t  maxsize;
-\&
-    // Null-padded fixed-width character sequences
+    // Null-padded fixed-size character sequences
     char    pre[4] = "pre.";
     char    new_post[50] = ".foo.bar";
 \&
     // Strings
     char    post[] = ".post";
     char    src[] = "some_long_body.post";
-    char    *dest;
-\&
-    maxsize = nitems(pre) + strlen(src) \- strlen(post) +
-              nitems(new_post) + 1;
-    dest = malloc(sizeof(*dest) * maxsize);
+    char *dest = malloc(sizeof pre + strlen(src) \- strlen(post)
+                        + sizeof new_post + 1);
     if (dest == NULL)
         err(EXIT_FAILURE, "malloc()");
 \&
-    dest[0] = \[aq]\e0\[aq];  // There's no 'cpy' function to this 'cat'.
-    strncat(dest, pre, nitems(pre));
+    dest[0] = \[aq]\e0\[aq];  // There's no `cpy' function to this `cat'.
+    strncat(dest, pre, sizeof pre);
     strncat(dest, src, strlen(src) \- strlen(post));
-    strncat(dest, new_post, nitems(new_post));
+    strncat(dest, new_post, sizeof new_post);
 \&
     puts(dest);  // "pre.some_long_body.foo.bar"
     free(dest);
-    exit(EXIT_SUCCESS);
 }
 .EE
 .\" SRC END
 .in
 .SH SEE ALSO
 .BR string (3),
-.BR string_copying (3)
+.BR string_copying (7)
-- 
2.41.0

Re: [PATCH 3/4] strncat.3 fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 6133 bytes --]

Hi Paul,

On Sun, Nov 12, 2023 at 03:52:07PM -0800, Paul Eggert wrote:

> Don't say "concatenate".

Ok

> Use "byte" instead of "character",

Ok

> and use standalone terminology rather than relying on the
> reader already having read string_copying(7).

I need to check again in a standalone commit.

> Don't say "width" when "size" was intended.

Ok

> Fix indenting of prototype.

Ok

> Simplify possible implementation, fixing a bug when the
> source string length and sz exceed INT_MAX.

Heh!  Good.

> Say that strncat is rarely useful.

Do we need to say that, or is it already implied by
"append non-null bytes from a source array to a string,
 and null-terminate the result"?
Not many programs need to do that operation.  I'm fine with saying it's
rarely useful; I'm just wondering if it's worth it.

> Say that behavior is undefined if the destination is not a string.

Ok

> Simplify example by using plain sizeof rather than an nitems macro,

If you want sizeof(), please use sizeof(), not sizeof.

I use nitems() with these functions because if you switch to wide
strings, you can keep the nitems() part, while you'd have to change it
if you had sizeof().  Also, nitems() makes it safe against sizeof(ptr).
What do you think of this?

> by removing a confusingly-named 'maxsize' local,

Ok

> and by removing an unnecessary call to 'exit'.

This was practice from Michael Kerrisk, which I like: always terminate
the program with exit(1); don't rely on just ending the scope of main().
That way, it's more visual.

Please split all these things into separate patches, if you don't mind,
and sign the patch.

> ---
>  man3/strncat.3 | 54 +++++++++++++++++++++-----------------------------
>  1 file changed, 23 insertions(+), 31 deletions(-)
> 
> diff --git a/man3/strncat.3 b/man3/strncat.3
> index d0f777d36..9a7df474a 100644
> --- a/man3/strncat.3
> +++ b/man3/strncat.3
> @@ -5,7 +5,8 @@
>  .\"
>  .TH strncat 3 (date) "Linux man-pages (unreleased)"
>  .SH NAME
> -strncat \- concatenate a null-padded character sequence into a string
> +strncat \- append non-null bytes from a source array to a string,
> +and null-terminate the result
>  .SH LIBRARY
>  Standard C library
>  .RI ( libc ", " \-lc )
> @@ -14,15 +15,18 @@ Standard C library
>  .B #include <string.h>
>  .P
>  .BI "char *strncat(char *restrict " dst ", const char " src "[restrict ." sz ],
> -.BI "               size_t " sz );
> +.BI "              size_t " sz );
>  .fi
>  .SH DESCRIPTION
> -This function catenates the input character sequence
> -contained in a null-padded fixed-width buffer,
> -into a string at the buffer pointed to by
> +This function appends at most
> +.I sz
> +non-null bytes from the array pointed to by
> +.I src
> +to the end of the string pointed to by
>  .IR dst .
> -The programmer is responsible for allocating a destination buffer large enough,
> -that is,
> +.I dst
> +must point to a string contained in a buffer that is large enough,
> +that is, the buffer size must be at least
>  .IR "strlen(dst) + strnlen(src, sz) + 1" .
>  .P
>  An implementation of this function might be:
> @@ -32,12 +36,7 @@ An implementation of this function might be:
>  char *
>  strncat(char *restrict dst, const char *restrict src, size_t sz)
>  {
> -    int   len;

Oops!  :)

> -    char  *p;
> -\&
> -    len = strnlen(src, sz);
> -    p = dst + strlen(dst);
> -    p = mempcpy(p, src, len);
> +    char *p = mempcpy(dst + strlen(dst), src, strnlen(src, sz));

Please use a C89 declaration for p (top of the function).

>      *p = \[aq]\e0\[aq];
>  \&
>      return dst;
> @@ -67,11 +66,12 @@ C11, POSIX.1-2008.
>  .SH HISTORY
>  POSIX.1-2001, C89, SVr4, 4.3BSD.
>  .SH CAVEATS
> -The name of this function is confusing.
> -This function has no relation to
> +The name of this function is confusing, as it has no relation to
>  .BR strncpy (3).

I didn't connect both sentences because I think it is confusing at
several levels.  Not only it has no relation to strncpy(), but it is
neither to writing 'n' bytes.  But yeah, having no relation to strncpy()
is the main one, so we could simplify.  What do you think?

Thanks!
Alex

> +This function is rarely useful in practice.
>  .P
> -If the destination buffer is not large enough,
> +If the destination buffer does not already contain a string,
> +or is not large enough,
>  the behavior is undefined.
>  See
>  .B _FORTIFY_SOURCE
> @@ -91,40 +91,32 @@ Shlemiel the painter
>  #include <stdlib.h>
>  #include <string.h>
>  \&
> -#define nitems(arr)  (sizeof((arr)) / sizeof((arr)[0]))
> -\&
>  int
>  main(void)
>  {
> -    size_t  maxsize;
> -\&
> -    // Null-padded fixed-width character sequences
> +    // Null-padded fixed-size character sequences
>      char    pre[4] = "pre.";
>      char    new_post[50] = ".foo.bar";
>  \&
>      // Strings
>      char    post[] = ".post";
>      char    src[] = "some_long_body.post";
> -    char    *dest;
> -\&
> -    maxsize = nitems(pre) + strlen(src) \- strlen(post) +
> -              nitems(new_post) + 1;
> -    dest = malloc(sizeof(*dest) * maxsize);
> +    char *dest = malloc(sizeof pre + strlen(src) \- strlen(post)
> +                        + sizeof new_post + 1);
>      if (dest == NULL)
>          err(EXIT_FAILURE, "malloc()");
>  \&
> -    dest[0] = \[aq]\e0\[aq];  // There's no 'cpy' function to this 'cat'.
> -    strncat(dest, pre, nitems(pre));
> +    dest[0] = \[aq]\e0\[aq];  // There's no `cpy' function to this `cat'.
> +    strncat(dest, pre, sizeof pre);
>      strncat(dest, src, strlen(src) \- strlen(post));
> -    strncat(dest, new_post, nitems(new_post));
> +    strncat(dest, new_post, sizeof new_post);
>  \&
>      puts(dest);  // "pre.some_long_body.foo.bar"
>      free(dest);
> -    exit(EXIT_SUCCESS);
>  }
>  .EE
>  .\" SRC END
>  .in
>  .SH SEE ALSO
>  .BR string (3),
> -.BR string_copying (3)
> +.BR string_copying (7)
> -- 
> 2.41.0
> 
> 

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

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

Re: [PATCH 3/4] strncat.3 fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1022 bytes --]

On Mon, Nov 13, 2023 at 02:15:20AM +0100, Alejandro Colomar wrote:
> > and by removing an unnecessary call to 'exit'.
> 
> This was practice from Michael Kerrisk, which I like: always terminate
> the program with exit(1); don't rely on just ending the scope of main().

D'oh!  I meant exit(3).  Since I need to write this email, I'll take the
opportunity to quote man-pages(7):

STYLE GUIDE
     ...

   Example programs and shell sessions
     Manual  pages  may  include example programs demonstrating how to
     use a system call or library function.  However, note the follow‐
     ing:

     ...

     •  For consistency, all example programs should  terminate  using
        either of:

            exit(EXIT_SUCCESS);
            exit(EXIT_FAILURE);

        Avoid using the following forms to terminate a program:

            exit(0);
            exit(1);
            return n;


Cheers,
Alex

> That way, it's more visual.
> 

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

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

Re: [PATCH 3/4] strncat.3 fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 2689 bytes --]

Hi Paul,

On Mon, Nov 13, 2023 at 02:15:20AM +0100, Alejandro Colomar wrote:
> Hi Paul,
> 
> On Sun, Nov 12, 2023 at 03:52:07PM -0800, Paul Eggert wrote:
> 
> > Don't say "concatenate".
> 
> Ok
> 
> > Use "byte" instead of "character",
> 
> Ok
> 
> > and use standalone terminology rather than relying on the
> > reader already having read string_copying(7).
> 
> I need to check again in a standalone commit.
> 
> > Don't say "width" when "size" was intended.
> 
> Ok
> 
> > Fix indenting of prototype.
> 
> Ok
> 
> > Simplify possible implementation, fixing a bug when the
> > source string length and sz exceed INT_MAX.
> 
> Heh!  Good.
> 
> > Say that strncat is rarely useful.
> 
> Do we need to say that, or is it already implied by
> "append non-null bytes from a source array to a string,
>  and null-terminate the result"?
> Not many programs need to do that operation.  I'm fine with saying it's
> rarely useful; I'm just wondering if it's worth it.
> 
> > Say that behavior is undefined if the destination is not a string.
> 
> Ok
> 
> > Simplify example by using plain sizeof rather than an nitems macro,
> 
> If you want sizeof(), please use sizeof(), not sizeof.
> 
> I use nitems() with these functions because if you switch to wide
> strings, you can keep the nitems() part, while you'd have to change it
> if you had sizeof().  Also, nitems() makes it safe against sizeof(ptr).
> What do you think of this?
> 
> > by removing a confusingly-named 'maxsize' local,
> 
> Ok
> 
> > and by removing an unnecessary call to 'exit'.
> 
> This was practice from Michael Kerrisk, which I like: always terminate
> the program with exit(1); don't rely on just ending the scope of main().
> That way, it's more visual.
> 
> Please split all these things into separate patches, if you don't mind,
> and sign the patch.

I've applied a set of patches myself:

d7cdccd84 (HEAD -> contrib, alx/contrib) strncat.3: CAVEATS: Say that strncat() is rarely useful
dc2c99ffe strncat.3: CAVEATS: wfix
39209460f strncat.3: Reword description
556a2d03e strncat.3: Say that if dst isn't a string, the behavior is undefined
ec37b6ac5 strncat.3: SEE ALSO: tfix
f8de0ec01 strncat.3: EXAMPLES: Fix name of variable
1c1c9f93c strncat.3: Simplify possible implementation of strncat()
09b80aed1 strncat.3: Rename third parameter to ssize
f5429cefe strncat.3: Fix bug in possible implementation of strncat(3)
2fe3076fe strncat.3: SYNOPSIS: ffix
696152d30 strncat.3: Don't say 'width' when 'size' is intended
6fe104a7d strncat.3: Say append instead of (con)catenate

Cheers,
Alex

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

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

[PATCH 4/4] stpncpy(3) fixes

[Paul Eggert]

Don't say "width" when "size" was meant.
Prefer "byte" to the confusing word "character".
Don't say that the source is a string; it need not be a string.
Don't imply the result always has some null padding.
Prefer standalone terminology.
Fix indenting of prototype.
Improve sample implementation by using memset rather than
the less-standard bzero, and by not overwriting part of
the destination more than once which is confusing.
Simplify example without losing its lessons.
Use fwrite instead of printf to avoid assuming buffer length fits in int;
although obviously this buffer length does fit, it's better if the sample
code is general.
---
 man3/stpncpy.3 | 82 ++++++++++++++++++++++++--------------------------
 1 file changed, 40 insertions(+), 42 deletions(-)

diff --git a/man3/stpncpy.3 b/man3/stpncpy.3
index 3cf4eb371..afe230307 100644
--- a/man3/stpncpy.3
+++ b/man3/stpncpy.3
@@ -7,8 +7,8 @@
 .SH NAME
 stpncpy, strncpy
 \-
-fill a fixed-width buffer with characters from a string
-and pad with null bytes
+fill a fixed-size buffer with non-null bytes from a source array,
+padding with null bytes as needed
 .SH LIBRARY
 Standard C library
 .RI ( libc ", " \-lc )
@@ -18,10 +18,10 @@ Standard C library
 .P
 .BI "char *strncpy(char " dst "[restrict ." sz "], \
 const char *restrict " src ,
-.BI "               size_t " sz );
+.BI "              size_t " sz );
 .BI "char *stpncpy(char " dst "[restrict ." sz "], \
 const char *restrict " src ,
-.BI "               size_t " sz );
+.BI "              size_t " sz );
 .fi
 .P
 .RS -4
@@ -37,15 +37,18 @@ Feature Test Macro Requirements for glibc (see
         _GNU_SOURCE
 .fi
 .SH DESCRIPTION
-These functions copy characters from the string pointed to by
+These functions copy non-null bytes from the array pointed to by
 .I src
-into a character sequence at the fixed-width buffer pointed to by
-.IR dst ,
-and pad with null bytes.
-If the destination buffer,
-limited by its size,
-isn't large enough to hold the copy,
-the resulting character sequence is truncated.
+into the array that is pointed to by
+.I dst
+and that contains
+.I sz
+bytes.
+If the source has too few non-null bytes to fill the destination,
+the functions pad the destination with trailing null bytes;
+if it has too many non-null bytes, the functions copy only the first
+.I sz
+bytes and do not append any null by5tes.
 For the difference between the two functions, see RETURN VALUE.
 .P
 An implementation of these functions might be:
@@ -62,8 +65,8 @@ strncpy(char *restrict dst, const char *restrict src, size_t sz)
 char *
 stpncpy(char *restrict dst, const char *restrict src, size_t sz)
 {
-    bzero(dst, sz);
-    return mempcpy(dst, src, strnlen(src, sz));
+    size_t n = strnlen(src, sz);
+    return memset(mempcpy(dst, src, n), 0, sz - n);
 }
 .EE
 .in
@@ -75,7 +78,7 @@ returns
 .TP
 .BR stpncpy ()
 returns a pointer to
-one after the last character in the destination character sequence.
+one after the last byte in the destination byte sequence.
 .SH ATTRIBUTES
 For an explanation of the terms used in this section, see
 .BR attributes (7).
@@ -107,9 +110,10 @@ C89, POSIX.1-2001, SVr4, 4.3BSD.
 glibc 1.07.
 POSIX.1-2008.
 .SH CAVEATS
-The name of these functions is confusing.
-These functions produce a null-padded character sequence,
-not a string (see
+The names of these functions are confusing.
+Because these functions append null bytes only if the source is a
+string with length less than the destination size,
+they might not create a string in their destination (see
 .BR string_copying (7)).
 For example:
 .P
@@ -122,14 +126,12 @@ strncpy(buf, "123456", 5);  // { \[aq]1\[aq], \[aq]2\[aq], \[aq]3\[aq], \[aq]4\[
 .EE
 .in
 .P
-It's impossible to distinguish truncation by the result of the call,
-from a character sequence that just fits the destination buffer;
-truncation should be detected by
-comparing the length of the input string
-with the size of the destination buffer.
+Although these functions can be used with strings,
+it is the caller's responsibility to detect whether they produce a string,
+e.g., by checking whether the result buffer ends in a null byte.
 .P
-If you're going to use this function in chained calls,
-it would be useful to develop a similar function that accepts
+To use these functions in chained calls,
+it might be useful to develop wrapper functions that accept
 a pointer to the end (one after the last element) of the destination buffer
 instead of its size.
 .SH EXAMPLES
@@ -141,30 +143,26 @@ instead of its size.
 #include <string.h>
 \&
 int
-main(void)
+main(int argc, char **argv)
 {
-    char    *p;
-    char    buf1[20];
-    char    buf2[20];
-    size_t  len;
+    char buf[20];
 \&
-    if (sizeof(buf2) < strlen("Hello world!"))
-        warnx("strncpy: truncating character sequence");
-    strncpy(buf2, "Hello world!", sizeof(buf2));
-    len = strnlen(buf2, sizeof(buf2));
+    if (strncpy(buf, argv[0], sizeof buf)[sizeof buf - 1])
+        warnx("strncpy: destination buffer is not a string");
+    size_t len = strnlen(buf, sizeof buf);
 \&
     printf("[len = %zu]: ", len);
-    printf("%.*s\en", (int) len, buf2);  // "Hello world!"
+    fwrite(buf, 1, len, stdout);
+    putchar(\[aq]\en\[aq]);
 \&
-    if (sizeof(buf1) < strlen("Hello world!"))
-        warnx("stpncpy: truncating character sequence");
-    p = stpncpy(buf1, "Hello world!", sizeof(buf1));
-    len = p \- buf1;
+    char *p = stpncpy(buf, argv[0], sizeof buf);
+    if (p == buf + sizeof buf)
+        warnx("stpncpy: destination buffer is not a string");
+    len = p \- buf;
 \&
     printf("[len = %zu]: ", len);
-    printf("%.*s\en", (int) len, buf1);  // "Hello world!"
-\&
-    exit(EXIT_SUCCESS);
+    fwrite(buf, 1, len, stdout);
+    putchar(\[aq]\en\[aq]);
 }
 .EE
 .\" SRC END
-- 
2.41.0

Re: [PATCH 4/4] stpncpy(3) fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 7680 bytes --]

Hi Paul,

On Sun, Nov 12, 2023 at 03:52:08PM -0800, Paul Eggert wrote:
> Don't say "width" when "size" was meant.
Ok

> Prefer "byte" to the confusing word "character".
Ok

> Don't say that the source is a string; it need not be a string.
As said in string.3, I'm not convinced by the new wording either.

> Don't imply the result always has some null padding.
Ok

> Prefer standalone terminology.
Ok

> Fix indenting of prototype.
Ok

> Improve sample implementation by using memset rather than
> the less-standard bzero,

While bzero(3) is non-standard, it is simpler, so I prefer it.

> and by not overwriting part of
> the destination more than once which is confusing.
Ok

> Simplify example without losing its lessons.
See some comments inline.

> Use fwrite instead of printf to avoid assuming buffer length fits in int;
Thanks!  And even without the int issue, it's more consistent with
handling non-terminated bytes.

> although obviously this buffer length does fit, it's better if the sample
> code is general.
Yep.

> ---
>  man3/stpncpy.3 | 82 ++++++++++++++++++++++++--------------------------
>  1 file changed, 40 insertions(+), 42 deletions(-)
> 
> diff --git a/man3/stpncpy.3 b/man3/stpncpy.3
> index 3cf4eb371..afe230307 100644
> --- a/man3/stpncpy.3
> +++ b/man3/stpncpy.3
> @@ -7,8 +7,8 @@
>  .SH NAME
>  stpncpy, strncpy
>  \-
> -fill a fixed-width buffer with characters from a string
> -and pad with null bytes
> +fill a fixed-size buffer with non-null bytes from a source array,
> +padding with null bytes as needed
>  .SH LIBRARY
>  Standard C library
>  .RI ( libc ", " \-lc )
> @@ -18,10 +18,10 @@ Standard C library
>  .P
>  .BI "char *strncpy(char " dst "[restrict ." sz "], \
>  const char *restrict " src ,
> -.BI "               size_t " sz );
> +.BI "              size_t " sz );
>  .BI "char *stpncpy(char " dst "[restrict ." sz "], \
>  const char *restrict " src ,
> -.BI "               size_t " sz );
> +.BI "              size_t " sz );
>  .fi
>  .P
>  .RS -4
> @@ -37,15 +37,18 @@ Feature Test Macro Requirements for glibc (see
>          _GNU_SOURCE
>  .fi
>  .SH DESCRIPTION
> -These functions copy characters from the string pointed to by
> +These functions copy non-null bytes from the array pointed to by
>  .I src
> -into a character sequence at the fixed-width buffer pointed to by
> -.IR dst ,
> -and pad with null bytes.
> -If the destination buffer,
> -limited by its size,
> -isn't large enough to hold the copy,
> -the resulting character sequence is truncated.
> +into the array that is pointed to by
> +.I dst
> +and that contains
> +.I sz
> +bytes.
> +If the source has too few non-null bytes to fill the destination,
> +the functions pad the destination with trailing null bytes;
> +if it has too many non-null bytes, the functions copy only the first
> +.I sz
> +bytes and do not append any null by5tes.
>  For the difference between the two functions, see RETURN VALUE.
>  .P
>  An implementation of these functions might be:
> @@ -62,8 +65,8 @@ strncpy(char *restrict dst, const char *restrict src, size_t sz)
>  char *
>  stpncpy(char *restrict dst, const char *restrict src, size_t sz)
>  {
> -    bzero(dst, sz);
> -    return mempcpy(dst, src, strnlen(src, sz));
> +    size_t n = strnlen(src, sz);
> +    return memset(mempcpy(dst, src, n), 0, sz - n);

Hmm, I hadn't thought of this chaining.  Nice!

>  }
>  .EE
>  .in
> @@ -75,7 +78,7 @@ returns
>  .TP
>  .BR stpncpy ()
>  returns a pointer to
> -one after the last character in the destination character sequence.
> +one after the last byte in the destination byte sequence.
>  .SH ATTRIBUTES
>  For an explanation of the terms used in this section, see
>  .BR attributes (7).
> @@ -107,9 +110,10 @@ C89, POSIX.1-2001, SVr4, 4.3BSD.
>  glibc 1.07.
>  POSIX.1-2008.
>  .SH CAVEATS
> -The name of these functions is confusing.
> -These functions produce a null-padded character sequence,
> -not a string (see
> +The names of these functions are confusing.
> +Because these functions append null bytes only if the source is a
> +string with length less than the destination size,
> +they might not create a string in their destination (see
>  .BR string_copying (7)).
>  For example:
>  .P
> @@ -122,14 +126,12 @@ strncpy(buf, "123456", 5);  // { \[aq]1\[aq], \[aq]2\[aq], \[aq]3\[aq], \[aq]4\[
>  .EE
>  .in
>  .P
> -It's impossible to distinguish truncation by the result of the call,
> -from a character sequence that just fits the destination buffer;
> -truncation should be detected by
> -comparing the length of the input string
> -with the size of the destination buffer.
> +Although these functions can be used with strings,
> +it is the caller's responsibility to detect whether they produce a string,
> +e.g., by checking whether the result buffer ends in a null byte.
>  .P
> -If you're going to use this function in chained calls,
> -it would be useful to develop a similar function that accepts
> +To use these functions in chained calls,
> +it might be useful to develop wrapper functions that accept
>  a pointer to the end (one after the last element) of the destination buffer
>  instead of its size.
>  .SH EXAMPLES
> @@ -141,30 +143,26 @@ instead of its size.
>  #include <string.h>
>  \&
>  int
> -main(void)
> +main(int argc, char **argv)
>  {
> -    char    *p;
> -    char    buf1[20];
> -    char    buf2[20];
> -    size_t  len;
> +    char buf[20];
>  \&
> -    if (sizeof(buf2) < strlen("Hello world!"))
> -        warnx("strncpy: truncating character sequence");
> -    strncpy(buf2, "Hello world!", sizeof(buf2));
> -    len = strnlen(buf2, sizeof(buf2));
> +    if (strncpy(buf, argv[0], sizeof buf)[sizeof buf - 1])
> +        warnx("strncpy: destination buffer is not a string");

The previous warning was slightly different.  It was warning because
data was lost.  Now you warn when it's not a string, even if all the
data is there.  Since we're later using the data with fwrite(3), I think
it makes more sense to warn iff data has been truncated, ignoring the
fact that it's not a string (the whole page is saying this doesn't
produce a string).

So, I think this change is a bug.

I understand you may want to show how these functions can be used to
produce a string.  Maybe having another example (not necessarily a whole
program) would be useful.  If so, please put that one below this one, to
give the intention that it's only a secondary use of this function, and
not intended for everyone.

> +    size_t len = strnlen(buf, sizeof buf);

C89 declarations, please.

>  \&
>      printf("[len = %zu]: ", len);
> -    printf("%.*s\en", (int) len, buf2);  // "Hello world!"
> +    fwrite(buf, 1, len, stdout);

I like fwrite() better.  Good!  I wasn't happy with this weird printf().

Cheers,
Alex

> +    putchar(\[aq]\en\[aq]);
>  \&
> -    if (sizeof(buf1) < strlen("Hello world!"))
> -        warnx("stpncpy: truncating character sequence");
> -    p = stpncpy(buf1, "Hello world!", sizeof(buf1));
> -    len = p \- buf1;
> +    char *p = stpncpy(buf, argv[0], sizeof buf);
> +    if (p == buf + sizeof buf)
> +        warnx("stpncpy: destination buffer is not a string");
> +    len = p \- buf;
>  \&
>      printf("[len = %zu]: ", len);
> -    printf("%.*s\en", (int) len, buf1);  // "Hello world!"
> -\&
> -    exit(EXIT_SUCCESS);
> +    fwrite(buf, 1, len, stdout);
> +    putchar(\[aq]\en\[aq]);
>  }
>  .EE
>  .\" SRC END
> -- 
> 2.41.0
> 
> 

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

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

Re: [PATCH 4/4] stpncpy(3) fixes

[Alejandro Colomar]

[-- Attachment #1: Type: text/plain, Size: 1831 bytes --]

On Mon, Nov 13, 2023 at 02:29:38AM +0100, Alejandro Colomar wrote:
> Hi Paul,
> 
> On Sun, Nov 12, 2023 at 03:52:08PM -0800, Paul Eggert wrote:
> > Don't say "width" when "size" was meant.
> Ok
> 
> > Prefer "byte" to the confusing word "character".
> Ok
> 
> > Don't say that the source is a string; it need not be a string.
> As said in string.3, I'm not convinced by the new wording either.
> 
> > Don't imply the result always has some null padding.
> Ok
> 
> > Prefer standalone terminology.
> Ok
> 
> > Fix indenting of prototype.
> Ok
> 
> > Improve sample implementation by using memset rather than
> > the less-standard bzero,
> 
> While bzero(3) is non-standard, it is simpler, so I prefer it.
> 
> > and by not overwriting part of
> > the destination more than once which is confusing.
> Ok
> 
> > Simplify example without losing its lessons.
> See some comments inline.
> 
> > Use fwrite instead of printf to avoid assuming buffer length fits in int;
> Thanks!  And even without the int issue, it's more consistent with
> handling non-terminated bytes.
> 
> > although obviously this buffer length does fit, it's better if the sample
> > code is general.
> Yep.
> 

I've applied some of the changes in separate commits:

6b050ad54 (HEAD -> contrib, alx/contrib) stpncpy.3: Reword, saying 'byte' instead of 'character'
be301345f stpncpy.3: EXAMPLES: errx(3) instead of warnx(3) if truncation occurs
d6499e777 stpncpy.3: EXAMPLES: Use fwrite(3) instead of printf(3)
2560770b2 stpncpy.3: NAME: Clarify that these functions only padd if necessary
7f19af378 stpncpy.3: Optimize possible implementation of stpncpy()
2f5a64243 stpncpy.3: SYNOPSIS: ffix
a845da18f stpncpy.3: Don't say 'width' when 'size' is meant

Cheers,
Alex

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

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

Re: [PATCH 4/4] stpncpy(3) fixes

[Paul Eggert]

On 11/21/23 10:33, Alejandro Colomar wrote:
> I've applied some of the changes in separate commits:

Thanks for doing all that. Although I haven't had time to check the 
results, I'm sure they're an improvement.