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:
signature.asc
Description: PGP signature
