This series fixes two bugs in string_unescape() regarding the
destination buffer length. Both fixes are accompanied with kunit tests
which would fail without the fixes.

To make this possible, preparatory patches 1 and 2 improve and clean up
testing helpers and 3 introduces test_unescape_one() which allows for
targeted testing of the string_unescape() function.

Signed-off-by: Jonas Rebmann <jre@pengutronix.de>
---
Jonas Rebmann (5):
      lib/tests: string_helpers: check null terminator too
      lib/tests: string_helpers: drop unused parameters
      lib/tests: string_helpers: introduce test_string_unescape_one
      lib/string_helpers: use full destination buffer in string_unescape()
      lib/string_helpers: fix counting of remaining bytes in string_unescape()

 lib/string_helpers.c             |  5 +++--
 lib/tests/string_helpers_kunit.c | 46 +++++++++++++++++++++++++++++-----------
 2 files changed, 37 insertions(+), 14 deletions(-)
---
base-commit: 587858367581b9c55c3690f4e63382ad622719d4
change-id: 20260915-string_unescape-c056aacc90cb

Best regards,
--  
Jonas Rebmann <jre@pengutronix.de>

On Wed, 16 Sep 2026 19:38:05 +0200 Jonas Rebmann <jre@pengutronix.de> wrote:

> This series fixes two bugs in string_unescape() regarding the
> destination buffer length. Both fixes are accompanied with kunit tests
> which would fail without the fixes.
> 
> To make this possible, preparatory patches 1 and 2 improve and clean up
> testing helpers and 3 introduces test_unescape_one() which allows for
> targeted testing of the string_unescape() function.

Sashiko complains about uncheched kunit_kzalloc() return val.

	https://sashiko.dev/#/patchset/20260916-string_unescape-v1-0-7f8bd986fa33@pengutronix.de

I wonder whether we really need the usual allocation-failure semantics
in kunit.  Can we just make the kunit memory allocation functions abort
on error?

On Wed, Sep 16, 2026 at 02:36:23PM -0700, Andrew Morton wrote:
> On Wed, 16 Sep 2026 19:38:05 +0200 Jonas Rebmann <jre@pengutronix.de> wrote:
> 
> > This series fixes two bugs in string_unescape() regarding the
> > destination buffer length. Both fixes are accompanied with kunit tests
> > which would fail without the fixes.
> > 
> > To make this possible, preparatory patches 1 and 2 improve and clean up
> > testing helpers and 3 introduces test_unescape_one() which allows for
> > targeted testing of the string_unescape() function.
> 
> Sashiko complains about uncheched kunit_kzalloc() return val.
> 
> 	https://sashiko.dev/#/patchset/20260916-string_unescape-v1-0-7f8bd986fa33@pengutronix.de
> 
> I wonder whether we really need the usual allocation-failure semantics
> in kunit.  Can we just make the kunit memory allocation functions abort
> on error?

FWIW, having KUNIT_ASSERT_NOT_NULL built-in to kunit_kmalloc() et al
sounds good to me.  It's not clear to me why it wasn't done that way.
Maybe so that assertion failures show the correct file and line number?
But that can be solved by making them macros.

As-is, every test has to check for NULL, which is unnecessary
boilerplate.  It's also resulted in test-specific wrapper functions that
work around this, like alloc_buf() in lib/crypto/tests/test-utils.h.

If a fallible version is needed in rare cases, it could be underscored:
__kunit_kmalloc().  But I'm not sure any test needs that.

- Eric

On Wed, 16 Sep 2026 23:13:36 +0000 Eric Biggers <ebiggers@kernel.org> wrote:

> On Wed, Sep 16, 2026 at 02:36:23PM -0700, Andrew Morton wrote:
> > On Wed, 16 Sep 2026 19:38:05 +0200 Jonas Rebmann <jre@pengutronix.de> wrote:
> > 
> > > This series fixes two bugs in string_unescape() regarding the
> > > destination buffer length. Both fixes are accompanied with kunit tests
> > > which would fail without the fixes.
> > > 
> > > To make this possible, preparatory patches 1 and 2 improve and clean up
> > > testing helpers and 3 introduces test_unescape_one() which allows for
> > > targeted testing of the string_unescape() function.
> > 
> > Sashiko complains about uncheched kunit_kzalloc() return val.
> > 
> > 	https://sashiko.dev/#/patchset/20260916-string_unescape-v1-0-7f8bd986fa33@pengutronix.de
> > 
> > I wonder whether we really need the usual allocation-failure semantics
> > in kunit.  Can we just make the kunit memory allocation functions abort
> > on error?
> 
> FWIW, having KUNIT_ASSERT_NOT_NULL built-in to kunit_kmalloc() et al
> sounds good to me.  It's not clear to me why it wasn't done that way.
> Maybe so that assertion failures show the correct file and line number?
> But that can be solved by making them macros.
> 
> As-is, every test has to check for NULL, which is unnecessary
> boilerplate.  It's also resulted in test-specific wrapper functions that
> work around this, like alloc_buf() in lib/crypto/tests/test-utils.h.
> 
> If a fallible version is needed in rare cases, it could be underscored:
> __kunit_kmalloc().  But I'm not sure any test needs that.

Sure.  

Simply doing kthread_exit() when we're in the middle of something is
rather rude - it'll leak things.  I doubt if anyone cares about that much
if they're engaged in poking around with kunit tests.  otoh, small GFP_KERNEL
allocation failures are close to can't-happen.

One thought is to create a fake struct device in kunit, use
devm_kzalloc everywhere then get kunit_try_catch_throw() to release the
device.  Reuse the devm_ code's tracking and garbage-collecting
infrastructure.  But it just isn't worthwhile.  Make the kunit layer
say "ha ha, you lost some memory" and leave it at that...