bug#55719: [PATCH] bindat strz fixes

[Stefan Monnier]

>>> +  (ert-deftest bindat-test--strz-fixedlen-pack-overflow ()
>>> +    :expected-result :failed
>>> +    (should (equal (bindat-pack spec "abc") "\141\0")))
>> I think this changes the intended semantics.  Until now `strz N` has
>> meant that N bytes are used to encode the string and that it can
>> hold upto a string of length N (in which case there's no terminating NUL
>> byte).  I agree that it's not the only valid semantics, but I'm not sure
>> we want to change it at this point.
>> Do you have a particular reason to make this change.
>
> A few:
>   * The documentation says that the packed output is null terminated
>     so that's what users expect.
>   * It is safer (packed output is less likely to cause some other
>     program to run past the end of the field).
>   * Without this change, there is no difference between `strz N` and
>     `str N`. So what would be the point of `strz N`?
>   * If the user selected strz, the application probably requires null
>     termination in all cases, not just when the string is under a
>     certain length.

You're listing advantages of this choice, which we know about already.
The other choice also has advantages.  These don't count as "particular
reasons" (e.g. a real-life concrete *need* for it, rather than a mere
preference).

The particular reason to prefer the current behavior is
backward compatibility (which we could call "inertia").

Note also that `strz` without a length (or with a nil length) behaves
the way you want.

Of course, we could add an additional (optional) arg to `strz` to
specify what should happen when unpacking a string with missing NUL byte
as well as whether to truncate to N-1 chars rather than to N chars to
make sure there is a terminating NUL byte.

>>> +  (ert-deftest bindat-test--strz-fixedlen-unpack ()
>>> +    (should (equal (bindat-unpack spec "\0\0") ""))
>>> +    (should (equal (bindat-unpack spec "a\0") "a"))))
>> How 'bout
>>       (bindat-unpack spec "ab")
>> ?
>
> I added some comments explaining why cases like that aren't tested.

The byte-string to unpack is not necessarily built from our own code
(usually bindat is used to communicate with some other application), so
whether our code can generate "ab" is not really relevant: the question
still comes up about what we should do with "ab" (where valid answers
could be to return "ab" or to return "a" or to signal an error, ...).
Of course we can also decide it's "undefined".


        Stefan