[PATCH v2 0/3] Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros

5 views
Skip to first unread message

Maíra Canal

unread,
Aug 2, 2022, 5:26:40 PM8/2/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ or
KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp function,
such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, if the
expectation fails the error message is not very helpful, indicating only the
return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a determined size. In
case of expectation failure, those macros print the hex dump of the memory
blocks, making it easier to debug test failures for memory blocks.

For example, if I am using the KUNIT_EXPECT_MEMEQ macro and apply the
following diff (introducing a test failure) to the
drm/tests/drm_format_helper.c:

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 3106abb3bead..942aa131a768 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -131,9 +131,9 @@ static struct convert_xrgb8888_case convert_xrgb8888_cases[] = {
.rgb565_result = {
.dst_pitch = 10,
.expected = {
- 0x0A33, 0x1260, 0xA800, 0x0000, 0x0000,
- 0x6B8E, 0x0A33, 0x1260, 0x0000, 0x0000,
- 0xA800, 0x6B8E, 0x0A33, 0x0000, 0x0000,
+ 0x0A31, 0x1260, 0xA800, 0x0000, 0x0000,
+ 0x6B81, 0x0A33, 0x1260, 0x0000, 0x0000,
+ 0xA801, 0x6B8E, 0x0A33, 0x0000, 0x0000,
},
.expected_swab = {
0x330A, 0x6012, 0x00A8, 0x0000, 0x0000,}}}

I will get a test failure with the following form:

➜ ./tools/testing/kunit/kunit.py run --kunitconfig=drivers/gpu/drm/tests \
--kconfig_add CONFIG_UML_PCI_OVER_VIRTIO=y --kconfig_add CONFIG_VIRTIO_UML=y \
'drm_format_helper_test'
[...]
[18:15:35] ================= xrgb8888_to_rgb565_test ==================
[18:15:35] [PASSED] single_pixel_source_buffer
[18:15:35] [PASSED] single_pixel_clip_rectangle
[18:15:35] [PASSED] well_known_colors
[18:15:35] # xrgb8888_to_rgb565_test: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_format_helper_test.c:248
[18:15:35] Expected dst == result->expected, but
[18:15:35] dst ==
[18:15:35] <33> 0a 60 12 00 a8 00 00 00 00 <8e> 6b 33 0a 60 12
[18:15:35] 00 00 00 00 <00> a8 8e 6b 33 0a 00 00 00 00
[18:15:35] result->expected ==
[18:15:35] <31> 0a 60 12 00 a8 00 00 00 00 <81> 6b 33 0a 60 12
[18:15:35] 00 00 00 00 <01> a8 8e 6b 33 0a 00 00 00 00
[18:15:35] not ok 4 - destination_pitch
[18:15:35] [FAILED] destination_pitch
[18:15:35] # Subtest: xrgb8888_to_rgb565_test
[18:15:35] # xrgb8888_to_rgb565_test: pass:3 fail:1 skip:0 total:4
[18:15:35] not ok 2 - xrgb8888_to_rgb565_test
[...]
[18:15:35] ============= [FAILED] drm_format_helper_test ==============
[18:15:35] ============================================================
[18:15:35] Testing complete. Ran 8 tests: passed: 7, failed: 1
[18:15:35] Elapsed time: 3.148s total, 0.002s configuring, 3.031s building, 0.090s running

Noticed that, with the hex dump, it is possible to check which bytes are
making the test fail. So, it is easier to debug the cause of the failure.

Moreover, on this v2, the differed bytes are marked with a <>, to ease the
identication of the differences. The bytes are not ideally aligned, but the
marks, suggested by Daniel, are very helpful.

The first patch of the series introduces the KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ. The second patch adds an example of memory block
expectations on the kunit-example-test.c. And the last patch replaces the
KUNIT_EXPECT_EQ for KUNIT_EXPECT_MEMEQ on the existing occurrences.

Best Regards,
- Maíra Canal

v1 -> v2: https://lore.kernel.org/linux-kselftest/2a0dcd75-5461-5266...@riseup.net/T/#m402cc72eb01fb3b88d6706cf7d1705fdd51e5da2

- Change "determinated" to "specified" (Daniel Latypov).
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Mark the different bytes on the failure message with a <> (Daniel Latypov).
- Replace a constant number of array elements for ARRAY_SIZE() (André Almeida).
- Rename "array" and "expected" variables to "array1" and "array2" (Daniel Latypov).

Maíra Canal (3):
kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
kunit: Add KUnit memory block assertions to the example_all_expect_macros_test
kunit: Use KUNIT_EXPECT_MEMEQ macro

.../gpu/drm/tests/drm_format_helper_test.c | 6 +-
include/kunit/assert.h | 35 +++++++++
include/kunit/test.h | 76 +++++++++++++++++++
lib/kunit/assert.c | 54 +++++++++++++
lib/kunit/kunit-example-test.c | 7 ++
net/core/dev_addr_lists_test.c | 4 +-
6 files changed, 177 insertions(+), 5 deletions(-)

--
2.37.1

Maíra Canal

unread,
Aug 2, 2022, 5:26:48 PM8/2/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
function, such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, when
the expectation fails, the error message is not very helpful,
indicating only the return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
In case of expectation failure, those macros print the hex dump of the
memory blocks, making it easier to debug test failures for memory blocks.

That said, the expectation

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

would translate to the expectation

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <maira...@riseup.net>
---
v1 -> v2:
- Change "determinated" to "specified" (Daniel Latypov).
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Mark the different bytes on the failure message with a <> (Daniel Latypov).
---
include/kunit/assert.h | 35 +++++++++++++++++++
include/kunit/test.h | 76 ++++++++++++++++++++++++++++++++++++++++++
lib/kunit/assert.c | 54 ++++++++++++++++++++++++++++++
3 files changed, 165 insertions(+)

diff --git a/include/kunit/assert.h b/include/kunit/assert.h
index 4b52e12c2ae8..a54f5253b997 100644
--- a/include/kunit/assert.h
+++ b/include/kunit/assert.h
@@ -256,4 +256,39 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
const struct va_format *message,
struct string_stream *stream);

+
+#define KUNIT_INIT_MEM_ASSERT_STRUCT(text_, left_val, right_val, size_) \
+ { \
+ .assert = { .format = kunit_mem_assert_format }, \
+ .text = text_, \
+ .left_value = left_val, \
+ .right_value = right_val, .size = size_, \
+ }
+
+/**
+ * struct kunit_mem_assert - An expectation/assertion that compares two
+ * memory blocks.
+ * @assert: The parent of this type.
+ * @text: Holds the textual representations of the operands and comparator.
+ * @left_value: The actual evaluated value of the expression in the left slot.
+ * @right_value: The actual evaluated value of the expression in the right slot.
+ * @size: Size of the memory block analysed in bytes.
+ *
+ * Represents an expectation/assertion that compares two memory blocks. For
+ * example, to expect that the first three bytes of foo is equal to the
+ * first three bytes of bar, you can use the expectation
+ * KUNIT_EXPECT_MEMEQ(test, foo, bar, 3);
+ */
+struct kunit_mem_assert {
+ struct kunit_assert assert;
+ const struct kunit_binary_assert_text *text;
+ const void *left_value;
+ const void *right_value;
+ const size_t size;
+};
+
+void kunit_mem_assert_format(const struct kunit_assert *assert,
+ const struct va_format *message,
+ struct string_stream *stream);
+
#endif /* _KUNIT_ASSERT_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8ffcd7de9607..1925d648eec8 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -684,6 +684,36 @@ do { \
##__VA_ARGS__); \
} while (0)

+#define KUNIT_MEM_ASSERTION(test, \
+ assert_type, \
+ left, \
+ op, \
+ right, \
+ size, \
+ fmt, \
+ ...) \
+do { \
+ const void *__left = (left); \
+ const void *__right = (right); \
+ const size_t __size = (size); \
+ static const struct kunit_binary_assert_text __text = { \
+ .operation = #op, \
+ .left_text = #left, \
+ .right_text = #right, \
+ }; \
+ \
+ KUNIT_ASSERTION(test, \
+ assert_type, \
+ memcmp(__left, __right, __size) op 0, \
+ kunit_mem_assert, \
+ KUNIT_INIT_MEM_ASSERT_STRUCT(&__text, \
+ __left, \
+ __right, \
+ __size), \
+ fmt, \
+ ##__VA_ARGS__); \
+} while (0)
+
#define KUNIT_PTR_NOT_ERR_OR_NULL_MSG_ASSERTION(test, \
assert_type, \
ptr, \
@@ -952,6 +982,52 @@ do { \
fmt, \
##__VA_ARGS__)

+/**
+ * KUNIT_EXPECT_MEMEQ() - Expects that the first @size bytes of @left and @right are equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !memcmp((@left), (@right), (@size))). See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_EXPECT_MEMEQ(test, left, right, size) \
+ KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ left, ==, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_MEMNEQ() - Expects that the first @size bytes of @left and @right are not equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, memcmp((@left), (@right), (@size))). See
+ * KUNIT_EXPECT_TRUE() for more information.
+ */
+#define KUNIT_EXPECT_MEMNEQ(test, left, right, size) \
+ KUNIT_EXPECT_MEMNEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_EXPECT_MEMNEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ left, !=, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
/**
* KUNIT_EXPECT_NULL() - Expects that @ptr is null.
* @test: The test context object.
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index d00d6d181ee8..abd434bc7ec6 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -204,3 +204,57 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
kunit_assert_print_msg(message, stream);
}
EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format);
+
+/* Adds a hexdump of a buffer to a string_stream comparing it with
+ * a second buffer. The different bytes are marked with <>.
+ */
+static void kunit_assert_hexdump(struct string_stream *stream,
+ const void *buf, const void *compared_buf, const size_t len)
+{
+ size_t i;
+ const u8 *buf1 = buf;
+ const u8 *buf2 = compared_buf;
+
+ for (i = 0; i < len; ++i) {
+ if (i % 16)
+ string_stream_add(stream, " ");
+ else if (i)
+ string_stream_add(stream, "\n ");
+
+ if (buf1[i] != buf2[i])
+ string_stream_add(stream, "<%02x>", buf1[i]);
+ else
+ string_stream_add(stream, "%02x", buf1[i]);
+ }
+}
+
+void kunit_mem_assert_format(const struct kunit_assert *assert,
+ const struct va_format *message,
+ struct string_stream *stream)
+{
+ struct kunit_mem_assert *mem_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);
+
+ 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, 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);
+}
+EXPORT_SYMBOL_GPL(kunit_mem_assert_format);
--
2.37.1

Maíra Canal

unread,
Aug 2, 2022, 5:26:56 PM8/2/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Increament the example_all_expect_macros_test with the
KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros by creating a test
with memory block assertions.

Signed-off-by: Maíra Canal <maira...@riseup.net>
---
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Replace a constant number of array elements for ARRAY_SIZE() (André Almeida).
- Rename "array" and "expected" variables to "array1" and "array2" (Daniel Latypov).
---
lib/kunit/kunit-example-test.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index f8fe582c9e36..8a9b0eeb1934 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -86,6 +86,9 @@ static void example_mark_skipped_test(struct kunit *test)
*/
static void example_all_expect_macros_test(struct kunit *test)
{
+ const u32 array1[] = { 0x0F, 0xFF };
+ const u32 array2[] = { 0x1F, 0xFF };
+
/* Boolean assertions */
KUNIT_EXPECT_TRUE(test, true);
KUNIT_EXPECT_FALSE(test, false);
@@ -109,6 +112,10 @@ static void example_all_expect_macros_test(struct kunit *test)
KUNIT_EXPECT_STREQ(test, "hi", "hi");
KUNIT_EXPECT_STRNEQ(test, "hi", "bye");

+ /* Memory block assertions */
+ KUNIT_EXPECT_MEMEQ(test, array1, array1, sizeof(u32) * ARRAY_SIZE(array1));
+ KUNIT_EXPECT_MEMNEQ(test, array1, array2, sizeof(u32) * ARRAY_SIZE(array1));
+
/*
* There are also ASSERT variants of all of the above that abort test
* execution if they fail. Useful for memory allocations, etc.
--
2.37.1

Maíra Canal

unread,
Aug 2, 2022, 5:27:03 PM8/2/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Use KUNIT_EXPECT_MEMEQ to compare memory blocks in replacement of the
KUNIT_EXPECT_EQ macro. Therefor, the statement

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

is replaced by:

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <maira...@riseup.net>
---
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
---
drivers/gpu/drm/tests/drm_format_helper_test.c | 6 +++---
net/core/dev_addr_lists_test.c | 4 ++--
2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/tests/drm_format_helper_test.c b/drivers/gpu/drm/tests/drm_format_helper_test.c
index 26ecf3b4b137..482136282273 100644
--- a/drivers/gpu/drm/tests/drm_format_helper_test.c
+++ b/drivers/gpu/drm/tests/drm_format_helper_test.c
@@ -217,7 +217,7 @@ static void xrgb8888_to_rgb332_test(struct kunit *test)

drm_fb_xrgb8888_to_rgb332(dst, result->dst_pitch, src, &fb,
&params->clip);
- KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, dst, result->expected, dst_size);
}

static void xrgb8888_to_rgb565_test(struct kunit *test)
@@ -245,11 +245,11 @@ static void xrgb8888_to_rgb565_test(struct kunit *test)

drm_fb_xrgb8888_to_rgb565(dst, result->dst_pitch, src, &fb,
&params->clip, false);
- KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, dst, result->expected, dst_size);

drm_fb_xrgb8888_to_rgb565(dst, result->dst_pitch, src, &fb,
&params->clip, true);
- KUNIT_EXPECT_EQ(test, memcmp(dst, result->expected_swab, dst_size), 0);
+ KUNIT_EXPECT_MEMEQ(test, dst, result->expected_swab, dst_size);
}

static struct kunit_case drm_format_helper_test_cases[] = {
diff --git a/net/core/dev_addr_lists_test.c b/net/core/dev_addr_lists_test.c
index 049cfbc58aa9..90e7e3811ae7 100644
--- a/net/core/dev_addr_lists_test.c
+++ b/net/core/dev_addr_lists_test.c
@@ -71,11 +71,11 @@ static void dev_addr_test_basic(struct kunit *test)

memset(addr, 2, sizeof(addr));
eth_hw_addr_set(netdev, addr);
- KUNIT_EXPECT_EQ(test, 0, memcmp(netdev->dev_addr, addr, sizeof(addr)));
+ KUNIT_EXPECT_MEMEQ(test, netdev->dev_addr, addr, sizeof(addr));

memset(addr, 3, sizeof(addr));
dev_addr_set(netdev, addr);
- KUNIT_EXPECT_EQ(test, 0, memcmp(netdev->dev_addr, addr, sizeof(addr)));
+ KUNIT_EXPECT_MEMEQ(test, netdev->dev_addr, addr, sizeof(addr));
}

static void dev_addr_test_sync_one(struct kunit *test)
--
2.37.1

Daniel Latypov

unread,
Aug 2, 2022, 5:57:52 PM8/2/22
to Maíra Canal, Brendan Higgins, davi...@google.com, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org
On Tue, Aug 2, 2022 at 2:26 PM Maíra Canal <maira...@riseup.net> wrote:
>
> Increament the example_all_expect_macros_test with the

nit: typo ("Increment")
But "Augment" would be a bit more idiomatic here

Sorry I didn't catch this one in v1.

> KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros by creating a test
> with memory block assertions.
>
> Signed-off-by: Maíra Canal <maira...@riseup.net>

Reviewed-by: Daniel Latypov <dlat...@google.com>

Thanks!
Just a couple very small nits (one above, one below).
Note: the following would be equivalent
KUNIT_EXPECT_MEMEQ(test, array1, array1, sizeof(array1));
KUNIT_EXPECT_MEMNEQ(test, array1, array2, sizeof(array1));
I think now we've dropped the use of "array equal", sizeof() is also
generally more appropriate.

We could also optionally prefix these with
KUNIT_ASSERT_EQ(test, sizeof(array1), sizeof(array2));
if we want to be extra paranoid here, but I don't think that's really necessary.

Daniel Latypov

unread,
Aug 2, 2022, 6:25:06 PM8/2/22
to Maíra Canal, Brendan Higgins, davi...@google.com, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org
On Tue, Aug 2, 2022 at 2:26 PM Maíra Canal <maira...@riseup.net> wrote:
>
> Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
> or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
> function, such as:
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> Although this usage produces correct results for the test cases, when
> the expectation fails, the error message is not very helpful,
> indicating only the return of the memcmp function.
>
> Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
> KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
> In case of expectation failure, those macros print the hex dump of the
> memory blocks, making it easier to debug test failures for memory blocks.
>
> That said, the expectation
>
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> would translate to the expectation
>
> KUNIT_EXPECT_MEMEQ(test, foo, bar, size);
>
> Signed-off-by: Maíra Canal <maira...@riseup.net>

Reviewed-by: Daniel Latypov <dlat...@google.com>
Thanks, this is nice to have and I think the clarity issues have been resolved.

Some various optional nits about whitespace below.
I'd see if anyone has any complaints about the output format before
wasting any time on those nits.

Looking at example output now, I see
Expected array2 == array1, but
array2 ==
<1f> 00 00 00 ff 00 00 00
array1 ==
<0f> 00 00 00 ff 00 00 00
not ok 4 - example_all_expect_macros_test

Looks good to me.
very nit: the trailing \s aren't quite lined up.
In this particular case, I'm planning on deleting this block in the
future, so it doesn't matter too much.

> diff --git a/include/kunit/test.h b/include/kunit/test.h
> index 8ffcd7de9607..1925d648eec8 100644
> --- a/include/kunit/test.h
> +++ b/include/kunit/test.h
> @@ -684,6 +684,36 @@ do { \
> ##__VA_ARGS__); \
> } while (0)
>
> +#define KUNIT_MEM_ASSERTION(test, \

very nit: the trailing \s are also a bit out of line here.
We can fix this particular line by just adding another \t after "test,"

In general, lining these up is just a matter of adding a \t after the
text and then maybe add or delete some of the " "s before the \s.
E.g. with vim's `:set list`, after lining up the \s, I get

#define KUNIT_MEM_ASSERTION(test,^I^I^I^I^I \$
^I^I^I^I assert_type,^I^I^I^I \$
^I^I^I^I left,^I^I^I^I \$
^I^I^I^I op,^I^I^I^I^I \$
^I^I^I^I right,^I^I^I^I \$
^I^I^I^I size,^I^I \$
^I^I^I^I fmt,^I^I^I^I^I \$
^I^I^I^I ...)^I^I^I^I^I \$
do {^I^I^I^I^I^I^I^I^I \$
^Iconst void *__left = (left);^I^I^I^I^I \$
...
very nit: the func above doesn't line up the params, but I don't think
it matters too much.
It uses \t\t whereas this one is \t\t\t + some spaces.

I think it'd be fine if they were consistent with each other, or if
you're willing to mess around with spaces, we could get the parameters
to line up.
(e.g. see how kunit_binary_ptr_assert_format and others do their line breaks)

Daniel Latypov

unread,
Aug 2, 2022, 6:29:46 PM8/2/22
to Maíra Canal, Brendan Higgins, davi...@google.com, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org
On Tue, Aug 2, 2022 at 2:27 PM Maíra Canal <maira...@riseup.net> wrote:
>
> Use KUNIT_EXPECT_MEMEQ to compare memory blocks in replacement of the
> KUNIT_EXPECT_EQ macro. Therefor, the statement
>
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> is replaced by:
>
> KUNIT_EXPECT_MEMEQ(test, foo, bar, size);
>
> Signed-off-by: Maíra Canal <maira...@riseup.net>

Acked-by: Daniel Latypov <dlat...@google.com>

I didn't go and find the appropriate commit from the drm tree to base
this on, so I couldn't apply it locally.
But looking at the diff itself, looks good!

Maíra Canal

unread,
Aug 3, 2022, 5:59:09 PM8/3/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ or
KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp function,
such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, if the
expectation fails the error message is not very helpful, indicating only the
return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a determined size. In
case of expectation failure, those macros print the hex dump of the memory
blocks, making it easier to debug test failures for memory blocks.

Other than the style changes, this v3 brings alignment to the bytes, making
it easier to identify the faulty bytes. So, on the previous version, the
output from a failure would be:
[14:27:42] # xrgb8888_to_rgb565_test: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_format_helper_test.c:248
[14:27:42] Expected dst == result->expected, but
[14:27:42] dst ==
[14:27:42] 33 0a <60> 12 00 a8 00 00 <00> 00 8e 6b <33> 0a 60 12
[14:27:42] 00 00 <00> 00 00 a8 <8e> 6b 33 0a 00 00 <00> 00
[14:27:42] result->expected ==
[14:27:42] 33 0a <61> 12 00 a8 00 00 <01> 00 8e 6b <31> 0a 60 12
[14:27:42] 00 00 <01> 00 00 a8 <81> 6b 33 0a 00 00 <01> 00

Now, with the alignment, the output is:
[14:27:42] # xrgb8888_to_rgb565_test: EXPECTATION FAILED at drivers/gpu/drm/tests/drm_format_helper_test.c:248
[14:27:42] Expected dst == result->expected, but
[14:27:42] dst ==
[14:27:42] 33 0a <60> 12 00 a8 00 00 <00> 00 8e 6b <33> 0a 60 12
[14:27:42] 00 00 <00> 00 00 a8 <8e> 6b 33 0a 00 00 <00> 00
[14:27:42] result->expected ==
[14:27:42] 33 0a <61> 12 00 a8 00 00 <01> 00 8e 6b <31> 0a 60 12
[14:27:42] 00 00 <01> 00 00 a8 <81> 6b 33 0a 00 00 <01> 00

Moreover, on the raw output, there were some indentation problems. Those
problems were solved with the use of KUNIT_SUBSUBTEST_INDENT.

The first patch of the series introduces the KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ. The second patch adds an example of memory block
expectations on the kunit-example-test.c. And the last patch replaces the
KUNIT_EXPECT_EQ for KUNIT_EXPECT_MEMEQ on the existing occurrences.

Best Regards,
- Maíra Canal

v1 -> v2: https://lore.kernel.org/linux-kselftest/2a0dcd75-5461-5266...@riseup.net/T/#m402cc72eb01fb3b88d6706cf7d1705fdd51e5da2

- Change "determinated" to "specified" (Daniel Latypov).
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Mark the different bytes on the failure message with a <> (Daniel Latypov).
- Replace a constant number of array elements for ARRAY_SIZE() (André Almeida).
- Rename "array" and "expected" variables to "array1" and "array2" (Daniel Latypov).

v2 -> v3: https://lore.kernel.org/linux-kselftest/20220802212621.42...@riseup.net/T/#t

- Make the bytes aligned at output.
- Add KUNIT_SUBSUBTEST_INDENT to the output for the indentation (Daniel Latypov).
- Line up the trailing \ at macros using tabs (Daniel Latypov).
- Line up the params to the functions (Daniel Latypov).
- Change "Increament" to "Augment" (Daniel Latypov).
- Use sizeof() for array sizes (Daniel Latypov).

Maíra Canal (3):
kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
kunit: Add KUnit memory block assertions to the example_all_expect_macros_test
kunit: Use KUNIT_EXPECT_MEMEQ macro

.../gpu/drm/tests/drm_format_helper_test.c | 6 +-
include/kunit/assert.h | 34 +++++++++
include/kunit/test.h | 76 +++++++++++++++++++
lib/kunit/assert.c | 56 ++++++++++++++
lib/kunit/kunit-example-test.c | 7 ++
net/core/dev_addr_lists_test.c | 4 +-
6 files changed, 178 insertions(+), 5 deletions(-)

--
2.37.1

Maíra Canal

unread,
Aug 3, 2022, 5:59:15 PM8/3/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
function, such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, when
the expectation fails, the error message is not very helpful,
indicating only the return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
In case of expectation failure, those macros print the hex dump of the
memory blocks, making it easier to debug test failures for memory blocks.

That said, the expectation

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

would translate to the expectation

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <maira...@riseup.net>
Reviewed-by: Daniel Latypov <dlat...@google.com>
---
v1 -> v2:
- Change "determinated" to "specified" (Daniel Latypov).
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Mark the different bytes on the failure message with a <> (Daniel Latypov).

v2 -> v3:
- Make the bytes aligned at output.
- Add KUNIT_SUBSUBTEST_INDENT to the output for the indentation (Daniel Latypov).
- Line up the trailing \ at macros using tabs (Daniel Latypov).
- Line up the params to the functions (Daniel Latypov).
---
include/kunit/assert.h | 34 +++++++++++++++++++
include/kunit/test.h | 76 ++++++++++++++++++++++++++++++++++++++++++
lib/kunit/assert.c | 56 +++++++++++++++++++++++++++++++
3 files changed, 166 insertions(+)

diff --git a/include/kunit/assert.h b/include/kunit/assert.h
index 4b52e12c2ae8..4b817a8eb619 100644
--- a/include/kunit/assert.h
+++ b/include/kunit/assert.h
@@ -256,4 +256,38 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
const struct va_format *message,
struct string_stream *stream);

+#define KUNIT_INIT_MEM_ASSERT_STRUCT(text_, left_val, right_val, size_) \
+ { \
+ .assert = { .format = kunit_mem_assert_format }, \
+ .text = text_, \
+ .left_value = left_val, \
+ .right_value = right_val, .size = size_, \
+ }
+
+/**
+ * struct kunit_mem_assert - An expectation/assertion that compares two
+ * memory blocks.
+ * @assert: The parent of this type.
+ * @text: Holds the textual representations of the operands and comparator.
+ * @left_value: The actual evaluated value of the expression in the left slot.
+ * @right_value: The actual evaluated value of the expression in the right slot.
+ * @size: Size of the memory block analysed in bytes.
+ *
+ * Represents an expectation/assertion that compares two memory blocks. For
+ * example, to expect that the first three bytes of foo is equal to the
+ * first three bytes of bar, you can use the expectation
+ * KUNIT_EXPECT_MEMEQ(test, foo, bar, 3);
+ */
+struct kunit_mem_assert {
+ struct kunit_assert assert;
+ const struct kunit_binary_assert_text *text;
+ const void *left_value;
+ const void *right_value;
+ const size_t size;
+};
+
+void kunit_mem_assert_format(const struct kunit_assert *assert,
+ const struct va_format *message,
+ struct string_stream *stream);
+
#endif /* _KUNIT_ASSERT_H */
diff --git a/include/kunit/test.h b/include/kunit/test.h
index 8ffcd7de9607..02eeaff1c58e 100644
--- a/include/kunit/test.h
+++ b/include/kunit/test.h
@@ -684,6 +684,36 @@ do { \
##__VA_ARGS__); \
} while (0)

+#define KUNIT_MEM_ASSERTION(test, \
diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
index d00d6d181ee8..c346a8d7fa6e 100644
--- a/lib/kunit/assert.c
+++ b/lib/kunit/assert.c
@@ -204,3 +204,59 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
kunit_assert_print_msg(message, stream);
}
EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format);
+
+/* Adds a hexdump of a buffer to a string_stream comparing it with
+ * a second buffer. The different bytes are marked with <>.
+ */
+static void kunit_assert_hexdump(struct string_stream *stream,
+ const void *buf,
+ const void *compared_buf,
+ const size_t len)
+{
+ size_t i;
+ const u8 *buf1 = buf;
+ const u8 *buf2 = compared_buf;
+
+ string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT);
+
+ for (i = 0; i < len; ++i) {
+ if (!(i % 16) && i)
+ string_stream_add(stream, "\n" KUNIT_SUBSUBTEST_INDENT);
+
+ if (buf1[i] != buf2[i])
+ string_stream_add(stream, "<%02x>", buf1[i]);
+ else
+ string_stream_add(stream, " %02x ", buf1[i]);
+ }
+}
+
+void kunit_mem_assert_format(const struct kunit_assert *assert,
+ const struct va_format *message,
+ struct string_stream *stream)
+{
+ struct kunit_mem_assert *mem_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);
+
+ 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");
+

Maíra Canal

unread,
Aug 3, 2022, 5:59:22 PM8/3/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Augment the example_all_expect_macros_test with the KUNIT_EXPECT_MEMEQ
and KUNIT_EXPECT_MEMNEQ macros by creating a test with memory block
assertions.

Signed-off-by: Maíra Canal <maira...@riseup.net>
Reviewed-by: Daniel Latypov <dlat...@google.com>
---
v1 -> v2:
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Replace a constant number of array elements for ARRAY_SIZE() (André Almeida).
- Rename "array" and "expected" variables to "array1" and "array2" (Daniel Latypov).

v2 -> v3:
- Change "Increament" to "Augment" (Daniel Latypov).
- Use sizeof() for array sizes (Daniel Latypov).
---
lib/kunit/kunit-example-test.c | 7 +++++++
1 file changed, 7 insertions(+)

diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
index f8fe582c9e36..66cc4e2365ec 100644
--- a/lib/kunit/kunit-example-test.c
+++ b/lib/kunit/kunit-example-test.c
@@ -86,6 +86,9 @@ static void example_mark_skipped_test(struct kunit *test)
*/
static void example_all_expect_macros_test(struct kunit *test)
{
+ const u32 array1[] = { 0x0F, 0xFF };
+ const u32 array2[] = { 0x1F, 0xFF };
+
/* Boolean assertions */
KUNIT_EXPECT_TRUE(test, true);
KUNIT_EXPECT_FALSE(test, false);
@@ -109,6 +112,10 @@ static void example_all_expect_macros_test(struct kunit *test)
KUNIT_EXPECT_STREQ(test, "hi", "hi");
KUNIT_EXPECT_STRNEQ(test, "hi", "bye");

+ /* Memory block assertions */
+ KUNIT_EXPECT_MEMEQ(test, array1, array1, sizeof(array1));
+ KUNIT_EXPECT_MEMNEQ(test, array1, array2, sizeof(array1));

Maíra Canal

unread,
Aug 3, 2022, 5:59:27 PM8/3/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Use KUNIT_EXPECT_MEMEQ to compare memory blocks in replacement of the
KUNIT_EXPECT_EQ macro. Therefor, the statement

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

is replaced by:

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <maira...@riseup.net>
Acked-by: Daniel Latypov <dlat...@google.com>
---
v1 -> v2:
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).

v2 -> v3: No changes.

David Gow

unread,
Aug 5, 2022, 12:45:12 AM8/5/22
to Maíra Canal, Brendan Higgins, Daniel Latypov, David Airlie, Daniel Vetter, David S. Miller, Jakub Kicinski, José Expósito, Javier Martinez Canillas, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220803215855.258704-1-mairacanal%40riseup.net.

These patches look pretty good to me overall, but I was unable to
apply v3 to test -- it looks like the mail client has wrapped some
lines or something...

davidgow@slicestar:~/linux-kselftest$ git am
./v3_20220803_mairacanal_introduce_kunit_expect_memeq_and_kunit_expect_memneq_macros.mbx
Applying: kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
error: corrupt patch at line 24
Patch failed at 0001 kunit: Introduce KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ macros

Checkpatch also picks up an issue:
ERROR: patch seems to be corrupt (line wrapped?)
#62: FILE: include/kunit/assert.h:255:
const struct va_format *message,

v2 applied clearnly, so it seems to be specific to v3.

In general, I like the patches, though. While I think there are a few
places it'd be slightly suboptimale if it's being used to compare more
structured data, such as the prospect of comparing padding between
elements, as well as the output formatting not being ideal. It's
perfect for the cases where memcmp() otherwise would be used, though.

Cheers,
-- David

Maíra Canal

unread,
Aug 5, 2022, 8:18:24 AM8/5/22
to David Gow, Brendan Higgins, Daniel Latypov, David Airlie, Daniel Vetter, David S. Miller, Jakub Kicinski, José Expósito, Javier Martinez Canillas, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List
I'll check this issue and submit a v4. Thank you!

>
> In general, I like the patches, though. While I think there are a few
> places it'd be slightly suboptimale if it's being used to compare more
> structured data, such as the prospect of comparing padding between
> elements, as well as the output formatting not being ideal. It's
> perfect for the cases where memcmp() otherwise would be used, though.

Do you any take on how to make the output formatting more ideal?

Best Regards,
- Maíra Canal

>
> Cheers,
> -- David

David Gow

unread,
Aug 5, 2022, 6:46:59 PM8/5/22
to Maíra Canal, Brendan Higgins, Daniel Latypov, David Airlie, Daniel Vetter, David S. Miller, Jakub Kicinski, José Expósito, Javier Martinez Canillas, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List
Thanks!

> >
> > In general, I like the patches, though. While I think there are a few
> > places it'd be slightly suboptimale if it's being used to compare more
> > structured data, such as the prospect of comparing padding between
> > elements, as well as the output formatting not being ideal. It's
> > perfect for the cases where memcmp() otherwise would be used, though.
>
> Do you any take on how to make the output formatting more ideal?
>

I don't actually think we need to change any of the formatting in this
patch, I'm just noting that usinng MEMEQ()/MEMNEQ() might not be the
best choice for comparing, e.g., structs (and that comparing their
members individually might be better there).
_Maybe_ that's something that could be mentioned in the documentation,
but I wouldn't change the code at all.

Cheers,
-- David

Muhammad Usama Anjum

unread,
Aug 6, 2022, 10:21:08 AM8/6/22
to Maíra Canal, Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, usama...@collabora.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org
On 8/4/22 2:58 AM, Maíra Canal wrote:
> Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
> or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
> function, such as:
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> Although this usage produces correct results for the test cases, when
> the expectation fails, the error message is not very helpful,
> indicating only the return of the memcmp function.
>
> Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
> KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
> In case of expectation failure, those macros print the hex dump of the
> memory blocks, making it easier to debug test failures for memory blocks.
>
> That said, the expectation
>
> KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);
>
> would translate to the expectation
>
> KUNIT_EXPECT_MEMEQ(test, foo, bar, size);
>
> Signed-off-by: Maíra Canal <maira...@riseup.net>
> Reviewed-by: Daniel Latypov <dlat...@google.com>
Reviewed-by: Muhammad Usama Anjum <usama...@collabora.com>
--
Muhammad Usama Anjum

Maíra Canal

unread,
Aug 8, 2022, 8:53:15 AM8/8/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ or
KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp function,
such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, if the
expectation fails the error message is not very helpful, indicating only the
return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a determined size. In
case of expectation failure, those macros print the hex dump of the memory
blocks, making it easier to debug test failures for memory blocks.

The v4 doesn't bring many changes. The output is aligned just like the previous
version but it fixes some mail client problems (sorry about that) and mentions
that this macros are not recommended for structured data.

The first patch of the series introduces the KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ. The second patch adds an example of memory block
expectations on the kunit-example-test.c. And the last patch replaces the
KUNIT_EXPECT_EQ for KUNIT_EXPECT_MEMEQ on the existing occurrences.

Best Regards,
- Maíra Canal

v1 -> v2: https://lore.kernel.org/linux-kselftest/2a0dcd75-5461-5266...@riseup.net/T/#m402cc72eb01fb3b88d6706cf7d1705fdd51e5da2

- Change "determinated" to "specified" (Daniel Latypov).
- Change the macro KUNIT_EXPECT_ARREQ to KUNIT_EXPECT_MEMEQ, in order to make
it easier for users to infer the right size unit (Daniel Latypov).
- Mark the different bytes on the failure message with a <> (Daniel Latypov).
- Replace a constant number of array elements for ARRAY_SIZE() (André Almeida).
- Rename "array" and "expected" variables to "array1" and "array2" (Daniel Latypov).

v2 -> v3: https://lore.kernel.org/linux-kselftest/20220802212621.42...@riseup.net/T/#t

- Make the bytes aligned at output.
- Add KUNIT_SUBSUBTEST_INDENT to the output for the indentation (Daniel Latypov).
- Line up the trailing \ at macros using tabs (Daniel Latypov).
- Line up the params to the functions (Daniel Latypov).
- Change "Increament" to "Augment" (Daniel Latypov).
- Use sizeof() for array sizes (Daniel Latypov).
- Add Daniel Latypov's tags.

v3 -> v4: https://lore.kernel.org/linux-kselftest/CABVgOSm_59Yr82deQm2C=18jjSv_akmn66z...@mail.gmail.com/T/#t

- Fix wrapped lines by the mail client (David Gow).
- Mention on documentation that KUNIT_EXPECT_MEMEQ is not recommended for
structured data (David Gow).
- Add Muhammad Usama Anjum's tag.

Maíra Canal (3):
kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
kunit: Add KUnit memory block assertions to the example_all_expect_macros_test
kunit: Use KUNIT_EXPECT_MEMEQ macro

.../gpu/drm/tests/drm_format_helper_test.c | 6 +-
include/kunit/assert.h | 34 ++++++++
include/kunit/test.h | 84 +++++++++++++++++++
lib/kunit/assert.c | 56 +++++++++++++
lib/kunit/kunit-example-test.c | 7 ++
net/core/dev_addr_lists_test.c | 4 +-
6 files changed, 186 insertions(+), 5 deletions(-)

--
2.37.1

Maíra Canal

unread,
Aug 8, 2022, 8:53:27 AM8/8/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal, Muhammad Usama Anjum
Currently, in order to compare memory blocks in KUnit, the KUNIT_EXPECT_EQ
or KUNIT_EXPECT_FALSE macros are used in conjunction with the memcmp
function, such as:
KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

Although this usage produces correct results for the test cases, when
the expectation fails, the error message is not very helpful,
indicating only the return of the memcmp function.

Therefore, create a new set of macros KUNIT_EXPECT_MEMEQ and
KUNIT_EXPECT_MEMNEQ that compare memory blocks until a specified size.
In case of expectation failure, those macros print the hex dump of the
memory blocks, making it easier to debug test failures for memory blocks.

That said, the expectation

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

would translate to the expectation

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <maira...@riseup.net>
Reviewed-by: Daniel Latypov <dlat...@google.com>
Reviewed-by: Muhammad Usama Anjum <usama...@collabora.com>
---
include/kunit/assert.h | 34 +++++++++++++++++
include/kunit/test.h | 84 ++++++++++++++++++++++++++++++++++++++++++
lib/kunit/assert.c | 56 ++++++++++++++++++++++++++++
3 files changed, 174 insertions(+)
index 8ffcd7de9607..9e8d45aa09b7 100644
@@ -952,6 +982,60 @@ do { \
fmt, \
##__VA_ARGS__)

+/**
+ * KUNIT_EXPECT_MEMEQ() - Expects that the first @size bytes of @left and @right are equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, !memcmp((@left), (@right), (@size))). See
+ * KUNIT_EXPECT_TRUE() for more information.
+ *
+ * Although this expectation works for any memory block, it is not recommended
+ * for comparing more structured data, such as structs. This expectation is
+ * recommended for comparing, for example, data arrays.
+ */
+#define KUNIT_EXPECT_MEMEQ(test, left, right, size) \
+ KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, NULL)
+
+#define KUNIT_EXPECT_MEMEQ_MSG(test, left, right, size, fmt, ...) \
+ KUNIT_MEM_ASSERTION(test, \
+ KUNIT_EXPECTATION, \
+ left, ==, right, \
+ size, \
+ fmt, \
+ ##__VA_ARGS__)
+
+/**
+ * KUNIT_EXPECT_MEMNEQ() - Expects that the first @size bytes of @left and @right are not equal.
+ * @test: The test context object.
+ * @left: An arbitrary expression that evaluates to the specified size.
+ * @right: An arbitrary expression that evaluates to the specified size.
+ * @size: Number of bytes compared.
+ *
+ * Sets an expectation that the values that @left and @right evaluate to are
+ * not equal. This is semantically equivalent to
+ * KUNIT_EXPECT_TRUE(@test, memcmp((@left), (@right), (@size))). See
+ * KUNIT_EXPECT_TRUE() for more information.
+ *
+ * Although this expectation works for any memory block, it is not recommended
+ * for comparing more structured data, such as structs. This expectation is
+ * recommended for comparing, for example, data arrays.

Maíra Canal

unread,
Aug 8, 2022, 8:53:37 AM8/8/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Augment the example_all_expect_macros_test with the KUNIT_EXPECT_MEMEQ
and KUNIT_EXPECT_MEMNEQ macros by creating a test with memory block
assertions.

Signed-off-by: Maíra Canal <maira...@riseup.net>
Reviewed-by: Daniel Latypov <dlat...@google.com>

Maíra Canal

unread,
Aug 8, 2022, 8:53:46 AM8/8/22
to Brendan Higgins, davi...@google.com, Daniel Latypov, air...@linux.ie, dan...@ffwll.ch, da...@davemloft.net, ku...@kernel.org, jose.ex...@gmail.com, jav...@redhat.com, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, linux-k...@vger.kernel.org, kuni...@googlegroups.com, linux-...@vger.kernel.org, Maíra Canal
Use KUNIT_EXPECT_MEMEQ to compare memory blocks in replacement of the
KUNIT_EXPECT_EQ macro. Therefor, the statement

KUNIT_EXPECT_EQ(test, memcmp(foo, bar, size), 0);

is replaced by:

KUNIT_EXPECT_MEMEQ(test, foo, bar, size);

Signed-off-by: Maíra Canal <maira...@riseup.net>
Acked-by: Daniel Latypov <dlat...@google.com>
---

David Gow

unread,
Aug 9, 2022, 11:45:55 PM8/9/22
to Maíra Canal, Brendan Higgins, Daniel Latypov, David Airlie, Daniel Vetter, David S. Miller, Jakub Kicinski, José Expósito, Javier Martinez Canillas, andre...@riseup.net, melis...@gmail.com, siqueir...@riseup.net, Isabella Basso, magali...@gmail.com, tales.a...@gmail.com, open list:KERNEL SELFTEST FRAMEWORK, KUnit Development, Linux Kernel Mailing List
Thanks very much! I've looked through and tested this, and it looks great to me.

This entire series is
Reviewed-by: David Gow <davi...@google.com>

Cheers,
-- David


> Maíra Canal (3):
> kunit: Introduce KUNIT_EXPECT_MEMEQ and KUNIT_EXPECT_MEMNEQ macros
> kunit: Add KUnit memory block assertions to the example_all_expect_macros_test
> kunit: Use KUNIT_EXPECT_MEMEQ macro
>
> .../gpu/drm/tests/drm_format_helper_test.c | 6 +-
> include/kunit/assert.h | 34 ++++++++
> include/kunit/test.h | 84 +++++++++++++++++++
> lib/kunit/assert.c | 56 +++++++++++++
> lib/kunit/kunit-example-test.c | 7 ++
> net/core/dev_addr_lists_test.c | 4 +-
> 6 files changed, 186 insertions(+), 5 deletions(-)
>
> --
> 2.37.1
>
> --
> You received this message because you are subscribed to the Google Groups "KUnit Development" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20220808125237.277126-1-mairacanal%40riseup.net.
Reply all
Reply to author
Forward
0 new messages