Re: [PATCH 1/4] kunit: remove format func from struct kunit_assert, get it to 0 bytes

On Sat, Oct 1, 2022 at 8:26 AM Daniel Latypov dlatypov@google.com wrote:

Each calll to a KUNIT_EXPECT_*() macro creates a local variable which contains a struct kunit_assert.

Normally, we'd hope the compiler would be able to optimize this away, but we've seen cases where it hasn't, see https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/GbrMNej2BAAJ.

In changes like commit 21957f90b28f ("kunit: split out part of kunit_assert into a static const"), we've moved more and more parts out of struct kunit_assert and its children types (kunit_binary_assert).

This patch removes the final field and gets us to: sizeof(struct kunit_assert) == 0 sizeof(struct kunit_binary_assert) == 24 (on UML x86_64).

This also reduces the amount of macro plumbing going on at the cost of passing in one more arg to the base KUNIT_ASSERTION macro and kunit_do_failed_assertion().

Signed-off-by: Daniel Latypov dlatypov@google.com

Very glad to see this finally happening: we've definitely still been seeing these stack size warnings, so reducing the stack use is good

And as someone who has always been a little uneasy with the number of nested macros, the simplification is also a big win.

Reviewed-by: David Gow davidgow@google.com

Cheers, -- David

include/kunit/assert.h | 28 ++++++---------------------- include/kunit/test.h | 17 +++++++++++------ lib/kunit/test.c | 7 ++++--- 3 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/include/kunit/assert.h b/include/kunit/assert.h index 4b52e12c2ae8..ace3de8d1ee7 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -42,16 +42,15 @@ struct kunit_loc {

/**

• struct kunit_assert - Data for printing a failed assertion or expectation.

• • @format: a function which formats the data in this kunit_assert to a string.
  • Represents a failed expectation/assertion. Contains all the data necessary to
  • format a string to a user reporting the failure.
  */

-struct kunit_assert {

•   void (*format)(const struct kunit_assert *assert,
  

•                  const struct va_format *message,
  

•                  struct string_stream *stream);
  

-}; +struct kunit_assert {};

+typedef void (*assert_format_t)(const struct kunit_assert *assert,

•                           const struct va_format *message,
  

•                           struct string_stream *stream);
  
  

void kunit_assert_prologue(const struct kunit_loc *loc, enum kunit_assert_type type, @@ -71,16 +70,6 @@ void kunit_fail_assert_format(const struct kunit_assert *assert, const struct va_format *message, struct string_stream *stream);

-/**

• • KUNIT_INIT_FAIL_ASSERT_STRUCT - Initializer for &struct kunit_fail_assert.
• • Initializes a &struct kunit_fail_assert. Intended to be used in
• • KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.
• */

-#define KUNIT_INIT_FAIL_ASSERT_STRUCT { \

•   .assert = { .format = kunit_fail_assert_format },               \
  

-}

/**

• struct kunit_unary_assert - Represents a KUNIT_{EXPECT|ASSERT}_{TRUE|FALSE}
• @assert: The parent of this type.

@@ -110,7 +99,6 @@ void kunit_unary_assert_format(const struct kunit_assert *assert,

• KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.

*/ #define KUNIT_INIT_UNARY_ASSERT_STRUCT(cond, expect_true) { \

•   .assert = { .format = kunit_unary_assert_format },                     \
    .condition = cond,                                                     \
    .expected_true = expect_true                                           \
  

} @@ -145,7 +133,6 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert,

• KUNIT_EXPECT_* and KUNIT_ASSERT_* macros.

*/ #define KUNIT_INIT_PTR_NOT_ERR_STRUCT(txt, val) { \

•   .assert = { .format = kunit_ptr_not_err_assert_format },               \
    .text = txt,                                                           \
    .value = val                                                           \
  

} @@ -190,7 +177,6 @@ void kunit_binary_assert_format(const struct kunit_assert *assert,

• KUNIT_INIT_BINARY_ASSERT_STRUCT() - Initializes a binary assert like

• kunit_binary_assert, kunit_binary_ptr_assert, etc.
  

• • @format_func: a function which formats the assert to a string.
  • @text_: Pointer to a kunit_binary_assert_text.
  • @left_val: The actual evaluated value of the expression in the left slot.
  • @right_val: The actual evaluated value of the expression in the right slot.

@@ -200,11 +186,9 @@ void kunit_binary_assert_format(const struct kunit_assert *assert,

• fields but with different types for left_val/right_val.
• This is ultimately used by binary assertion macros like KUNIT_EXPECT_EQ, etc.

*/ -#define KUNIT_INIT_BINARY_ASSERT_STRUCT(format_func, \

•                                   text_,                                 \
  

+#define KUNIT_INIT_BINARY_ASSERT_STRUCT(text_, \ left_val, \ right_val) { \

•   .assert = { .format = format_func },                                   \
    .text = text_,                                                         \
    .left_value = left_val,                                                \
    .right_value = right_val                                               \
  

diff --git a/include/kunit/test.h b/include/kunit/test.h index 840a2c375065..3476549106f7 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -472,9 +472,10 @@ void kunit_do_failed_assertion(struct kunit *test, const struct kunit_loc *loc, enum kunit_assert_type type, const struct kunit_assert *assert,

•                          assert_format_t assert_format,
                           const char *fmt, ...);
  
  

-#define KUNIT_ASSERTION(test, assert_type, pass, assert_class, INITIALIZER, fmt, ...) do { \ +#define KUNIT_ASSERTION(test, assert_type, pass, assert_class, assert_format, INITIALIZER, fmt, ...) do { \ if (unlikely(!(pass))) { \ static const struct kunit_loc __loc = KUNIT_CURRENT_LOC; \ struct assert_class __assertion = INITIALIZER; \ @@ -482,6 +483,7 @@ void kunit_do_failed_assertion(struct kunit *test, &__loc, \ assert_type, \ &__assertion.assert, \

•                                     assert_format,                       \
                                      fmt,                                 \
                                      ##__VA_ARGS__);                      \
    }                                                                      \
  

@@ -493,7 +495,8 @@ void kunit_do_failed_assertion(struct kunit *test, assert_type, \ false, \ kunit_fail_assert, \

•                   KUNIT_INIT_FAIL_ASSERT_STRUCT,                         \
  

•                   kunit_fail_assert_format,                              \
  

•                   {},                                                    \
                    fmt,                                                   \
                    ##__VA_ARGS__)
  
  

@@ -524,6 +527,7 @@ void kunit_do_failed_assertion(struct kunit *test, assert_type, \ !!(condition) == !!expected_true, \ kunit_unary_assert, \

•                   kunit_unary_assert_format,                             \
                    KUNIT_INIT_UNARY_ASSERT_STRUCT(#condition,             \
                                                   expected_true),         \
                    fmt,                                                   \
  

@@ -581,8 +585,8 @@ do { \ assert_type, \ __left op __right, \ assert_class, \

•                   KUNIT_INIT_BINARY_ASSERT_STRUCT(format_func,           \
  

•                                                   &__text,               \
  

•                   format_func,                                           \
  

•                   KUNIT_INIT_BINARY_ASSERT_STRUCT(&__text,               \
                                                    __left,                \
                                                    __right),              \
                    fmt,                                                   \
  

@@ -639,8 +643,8 @@ do { \ assert_type, \ strcmp(__left, __right) op 0, \ kunit_binary_str_assert, \

•                   KUNIT_INIT_BINARY_ASSERT_STRUCT(kunit_binary_str_assert_format,\
  

•                                                   &__text,               \
  

•                   kunit_binary_str_assert_format,                        \
  

•                   KUNIT_INIT_BINARY_ASSERT_STRUCT(&__text,               \
                                                    __left,                \
                                                    __right),              \
                    fmt,                                                   \
  

@@ -659,6 +663,7 @@ do { \ assert_type, \ !IS_ERR_OR_NULL(__ptr), \ kunit_ptr_not_err_assert, \

•                   kunit_ptr_not_err_assert_format,                       \
                    KUNIT_INIT_PTR_NOT_ERR_STRUCT(#ptr,                    \
                                                  __ptr),                  \
                    fmt,                                                   \
  

diff --git a/lib/kunit/test.c b/lib/kunit/test.c index b73d5bb5c473..53bf1793f915 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -247,7 +247,7 @@ static void kunit_print_string_stream(struct kunit *test,

static void kunit_fail(struct kunit *test, const struct kunit_loc *loc, enum kunit_assert_type type, const struct kunit_assert *assert,

•                  const struct va_format *message)
  

•                  assert_format_t assert_format, const struct va_format *message)
  

{ struct string_stream *stream;

@@ -263,7 +263,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc, }

    kunit_assert_prologue(loc, type, stream);

•   assert->format(assert, message, stream);
  

•   assert_format(assert, message, stream);
  
    kunit_print_string_stream(test, stream);
  
  

@@ -287,6 +287,7 @@ void kunit_do_failed_assertion(struct kunit *test, const struct kunit_loc *loc, enum kunit_assert_type type, const struct kunit_assert *assert,

•                          assert_format_t assert_format,
                           const char *fmt, ...)
  

{ va_list args; @@ -296,7 +297,7 @@ void kunit_do_failed_assertion(struct kunit *test, message.fmt = fmt; message.va = &args;

•   kunit_fail(test, loc, type, assert, &message);
  

•   kunit_fail(test, loc, type, assert, assert_format, &message);
  
    va_end(args);
  
  

-- 2.38.0.rc1.362.ged0d419d3c-goog