Re: [PATCH 0/5] lib/string_helpers: fixes and test cases for string_unescape()

[Andrew Morton]

On Wed, 16 Sep 2026 23:13:36 +0000 Eric Biggers <ebiggers@xxxxxxxxxx> 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@xxxxxxxxxxxxxx> 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@xxxxxxxxxxxxxx
> > 
> > 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...