On 2026-09-17 09:45, Andy Shevchenko wrote:
On Wed, Sep 16, 2026 at 07:38:09PM +0200, Jonas Rebmann wrote:
Although all of the available sequences expand to exactly one byte, the
current implementation decrements the remaining bytes in the destination
buffer twice, effectively shortening it by one byte per each unescaped
character.

The extra decrement is only needed in the one case where a single loop
iteration produces two output bytes: when the sequence turns out not to
be a valid escape sequence, the previously skipped backslash has to be
emitted before the character is copied verbatim.

Add a kunit regression-test that unescapes into a barely long enough 3
buffer.

Can you add a simple example here with the hexdump to show the point?
Yes, reading the code helps, but the above commit message is a bit
unclear (at least to me).


My idea was that the regression tests below would serve as examples
here.

Maybe I could mention that "short buffer" without my patch yields "A\"
instead of "AAB"...

I'll add that to the commit messages

And I'll add the kunit outputs to the cover letter:

     # test_unescape: EXPECTATION FAILED at lib/tests/string_helpers_kunit.c:24
     Expected q_real == q_test, but
         q_real == 2 (0x2)
         q_test == 3 (0x3)
 name:short buffer
     # test_unescape: EXPECTATION FAILED at lib/tests/string_helpers_kunit.c:25
     Expected out_test == out_real, but
         out_test ==
          41 <41><42>
         out_real ==
          41 <5c><00>
 name:short buffer
     # test_unescape: EXPECTATION FAILED at lib/tests/string_helpers_kunit.c:25
     Expected out_test == out_real, but
         out_test ==
          42 <41>
         out_real ==
          42 <5c>
 name:end of buffer

Fixes: 16c7fa05829e ("lib/string_helpers: introduce generic string_unescape")
Signed-off-by: Jonas Rebmann <[email protected]>

...

+       test_string_unescape_one(test, "short buffer", UNESCAPE_HEX, "\\x41\\x41B", 4, 
"AAB", 3);
+       test_string_unescape_one(test, "unrecognized escape at end", UNESCAPE_HEX, 
"B\\qX", 4, "B\\q", 3);

Still wondering if we can incorporate this into the "combined" loop?

Both these tests really only fail without my fixes because the
destination buffer is short, barely fitting the result.

With a buffer length of 4 it yields "A\", for 5 "AA" and for 6 "AAB",
a bug because "AAB" would fit the 4 byte buffer too.

--
Pengutronix e.K.                           | Jonas Rebmann               |
Steuerwalder Str. 21                       | http://www.pengutronix.de/  |
31137 Hildesheim, Germany                  | Phone: +49-5121-206917-0    |
Amtsgericht Hildesheim, HRA 2686           | Fax:   +49-5121-206917-9    |

Reply via email to