Messages in this thread | | | From | David Gow <> | Date | Wed, 25 Jan 2023 14:38:08 +0800 | Subject | Re: [PATCH v1] kunit: fix bug in KUNIT_EXPECT_MEMEQ |
| |
On Wed, 25 Jan 2023 at 14:19, Rae Moar <rmoar@google.com> 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@intel.com/ > > Reported-by: kernel test robot <lkp@intel.com> > > Signed-off-by: Rae Moar <rmoar@google.com> > ---
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@google.com>
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 > [unhandled content-type:application/pkcs7-signature] | |