On Wed, 25 Jan 2023 at 14:19, Rae Moar <rmoar@xxxxxxxxxx> wrote: > > In KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ, add check if one of the > inputs is NULL and fail if this is the case. > > Currently, the kernel crashes if one of the inputs is NULL. Instead, > fail the test and add an appropriate error message. > > This was found by the kernel test robot: > https://lore.kernel.org/all/202212191448.D6EDPdOh-lkp@xxxxxxxxx/ > > Reported-by: kernel test robot <lkp@xxxxxxxxx> > > Signed-off-by: Rae Moar <rmoar@xxxxxxxxxx> > --- This looks good to me, modulo a small formatting issue with the continuation backslashes (see below). It might be worth considering this as a fix to the patch which first introduced the MEMEQ macros with a Fixes tag, e.g.: Fixes: b8a926bea8b1 ("kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros") Otherwise, this is: Reviewed-by: David Gow <davidgow@xxxxxxxxxx> Cheers, -- David > include/kunit/test.h | 7 ++++--- > lib/kunit/assert.c | 40 +++++++++++++++++++++++++--------------- > 2 files changed, 29 insertions(+), 18 deletions(-) > > diff --git a/include/kunit/test.h b/include/kunit/test.h > index 87ea90576b50..3c7045e22512 100644 > --- a/include/kunit/test.h > +++ b/include/kunit/test.h > @@ -683,9 +683,10 @@ do { \ > .right_text = #right, \ > }; \ > \ > - if (likely(memcmp(__left, __right, __size) op 0)) \ > - break; \ > - \ > + if (likely(__left && __right)) \ > + if (likely(memcmp(__left, __right, __size) op 0)) \ > + break; \ > + \ The backslashes no longer line up here. > _KUNIT_FAILED(test, \ > assert_type, \ > kunit_mem_assert, \ > diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c > index f5b50babe38d..05a09652f5a1 100644 > --- a/lib/kunit/assert.c > +++ b/lib/kunit/assert.c > @@ -241,24 +241,34 @@ void kunit_mem_assert_format(const struct kunit_assert *assert, > mem_assert = container_of(assert, struct kunit_mem_assert, > assert); > > - string_stream_add(stream, > - KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", > - mem_assert->text->left_text, > - mem_assert->text->operation, > - mem_assert->text->right_text); > + if (!mem_assert->left_value) { > + string_stream_add(stream, > + KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n", > + mem_assert->text->left_text); > + } else if (!mem_assert->right_value) { > + string_stream_add(stream, > + KUNIT_SUBTEST_INDENT "Expected %s is not null, but is\n", > + mem_assert->text->right_text); > + } else { > + string_stream_add(stream, > + KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", > + mem_assert->text->left_text, > + mem_assert->text->operation, > + mem_assert->text->right_text); > > - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", > - mem_assert->text->left_text); > - kunit_assert_hexdump(stream, mem_assert->left_value, > - mem_assert->right_value, mem_assert->size); > + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", > + mem_assert->text->left_text); > + kunit_assert_hexdump(stream, mem_assert->left_value, > + mem_assert->right_value, mem_assert->size); > > - string_stream_add(stream, "\n"); > + string_stream_add(stream, "\n"); > > - string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", > - mem_assert->text->right_text); > - kunit_assert_hexdump(stream, mem_assert->right_value, > - mem_assert->left_value, mem_assert->size); > + string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s ==\n", > + mem_assert->text->right_text); > + kunit_assert_hexdump(stream, mem_assert->right_value, > + mem_assert->left_value, mem_assert->size); > > - kunit_assert_print_msg(message, stream); > + kunit_assert_print_msg(message, stream); > + } > } > EXPORT_SYMBOL_GPL(kunit_mem_assert_format); > > base-commit: 5cb26c298ffde271d9bd1dd1b87ad028218f77fe > -- > 2.39.1.405.gd4c25cc71f-goog >
Attachment:
smime.p7s
Description: S/MIME Cryptographic Signature