Subject: Re: bug#55815: [PATCH] bindat: Improve str, strz documentation

From: Richard Hansen <[email-masked]>
Date: Mon, 6 Jun 2022 19:31:35 -0400

Thanks for the review.  A new revision is attached.

On 6/6/22 06:59, Eli Zaretskii wrote:
> I think it is better to say
> 
>    Unibyte string that is @var{len} bytes long.

Done.  I may have gone overboard though -- I did so because there are three representations that matter:

   1. The input string to be packed.
   2. The packed output.
   3. The result of unpacking.

Right now all three of those are unibyte, but in a future patch I plan on changing the first to accept unibyte-convertible multibyte input strings.

> Our conventions are to leave two spaces between sentences.

Done.

> Also, for consistency, I suggest to use "null byte" everywhere, to 
> avoid potential confusion of non-native English speakers.

Done.

I also fixed a flaw in the previous revision: packing to a fixed-length field doesn't actually write a null byte if the input is shorter than the field. This only matters if the caller provided a pre-allocated string that doesn't have null bytes.

Thanks,
Richard
From acc7717dee85dbaac66c3ddc65ea9dfe62bbdc23 Mon Sep 17 00:00:00 2001
From: Richard Hansen <[email-masked]>
Date: Thu, 2 Jun 2022 21:05:40 -0400
Subject: [PATCH v2] bindat: Improve str, strz documentation

* doc/lispref/processes.texi (Bindat Types): Expand the documentation
for the `str' and `strz' types to clarify expectations and explain
edge case behavior.
---
 doc/lispref/processes.texi | 37 ++++++++++++++++++++++++++++++++++---
 1 file changed, 34 insertions(+), 3 deletions(-)

diff --git a/doc/lispref/processes.texi b/doc/lispref/processes.texi
index 668a577870..1f6a9e3a7d 100644
--- a/doc/lispref/processes.texi
+++ b/doc/lispref/processes.texi
@@ -3479,11 +3479,42 @@ Bindat Types
 @var{bitlen} has to be a multiple of 8.
 
 @item str @var{len}
-String of bytes of length @var{len}.
+Unibyte string of length @var{len}.  When packing, the first @var{len}
+bytes of the input string are copied to the packed output.  If the
+input string is shorter than @var{len}, the remaining bytes will be
+null (zero) unless a pre-allocated string was provided to
+@code{bindat-pack}, in which case the remaining bytes are left
+unmodified.  The input string must be unibyte (@pxref{Text
+Representations}).  When unpacking, any null bytes in the packed input
+string will appear in the unpacked unibyte output.
 
 @item strz &optional @var{len}
-Zero-terminated string of bytes, can be of arbitrary length or in a fixed-size
-field with length @var{len}.
+If @var{len} is not provided: Variable-length null-terminated unibyte
+string.  When packing, the entire input string is copied to the packed
+output followed by a null byte.  The length of the packed output is
+the length of the input string plus one (for the added null byte).
+The input string must be unibyte (@pxref{Text Representations}) and
+must not contain any null bytes.  When unpacking, the resulting
+unibyte string contains all bytes up to (but excluding) the null byte.
+
+If @var{len} is provided: @code{strz} behaves the same as @code{str}
+with one difference: When unpacking, the first null byte encountered
+in the packed string and all subsequent bytes are excluded from the
+unpacked result.
+
+@quotation Caution
+The packed output will not be null-terminated unless one of the
+following is true:
+@itemize
+@item
+The input string is shorter than @var{len} and either no pre-allocated
+string was provided to @code{bindat-pack} or the appropriate byte in
+the pre-allocated string was already null.
+@item
+The input string contains a null byte within the first @var{len}
+bytes.
+@end itemize
+@end quotation
 
 @item vec @var{len} [@var{type}]
 Vector of @var{len} elements.  The type of the elements is given by
-- 
2.36.1