From a91e9ade402c48ee35cd1e4b671e3938db798d17 Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Thu, 13 Jan 2022 08:59:28 -0800 Subject: kunit: drop unused kunit* field in kunit_assert The `struct kunit* test` field in kunit_assert is unused. Note: string_stream needs it, but it has its own `test` field. I assume `test` in `kunit_assert` predates this and was leftover after some refactoring. This patch removes the field and cleans up the macros to avoid needlessly passing around `test`. Signed-off-by: Daniel Latypov Reviewed-by: Brendan Higgins Reviewed-by: David Gow Signed-off-by: Shuah Khan --- include/kunit/assert.h | 45 +++++++++++++-------------------------------- 1 file changed, 13 insertions(+), 32 deletions(-) (limited to 'include/kunit/assert.h') diff --git a/include/kunit/assert.h b/include/kunit/assert.h index ccbc36c0b02f..f568166ef034 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -30,7 +30,6 @@ enum kunit_assert_type { /** * struct kunit_assert - Data for printing a failed assertion or expectation. - * @test: the test case this expectation/assertion is associated with. * @type: the type (either an expectation or an assertion) of this kunit_assert. * @line: the source code line number that the expectation/assertion is at. * @file: the file path of the source file that the expectation/assertion is in. @@ -41,7 +40,6 @@ enum kunit_assert_type { * format a string to a user reporting the failure. */ struct kunit_assert { - struct kunit *test; enum kunit_assert_type type; int line; const char *file; @@ -60,14 +58,12 @@ struct kunit_assert { /** * KUNIT_INIT_ASSERT_STRUCT() - Initializer for a &struct kunit_assert. - * @kunit: The test case that this expectation/assertion is associated with. * @assert_type: The type (assertion or expectation) of this kunit_assert. * @fmt: The formatting function which builds a string out of this kunit_assert. * * The base initializer for a &struct kunit_assert. */ -#define KUNIT_INIT_ASSERT_STRUCT(kunit, assert_type, fmt) { \ - .test = kunit, \ +#define KUNIT_INIT_ASSERT_STRUCT(assert_type, fmt) { \ .type = assert_type, \ .file = __FILE__, \ .line = __LINE__, \ @@ -96,15 +92,13 @@ void kunit_fail_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_FAIL_ASSERT_STRUCT() - Initializer for &struct kunit_fail_assert. - * @test: The test case that this expectation/assertion is associated with. * @type: The type (assertion or expectation) of this kunit_assert. * * Initializes a &struct kunit_fail_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_FAIL_ASSERT_STRUCT(test, type) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(test, \ - type, \ +#define KUNIT_INIT_FAIL_ASSERT_STRUCT(type) { \ + .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ kunit_fail_assert_format) \ } @@ -129,7 +123,6 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_UNARY_ASSERT_STRUCT() - Initializes &struct kunit_unary_assert. - * @test: The test case that this expectation/assertion is associated with. * @type: The type (assertion or expectation) of this kunit_assert. * @cond: A string representation of the expression asserted true or false. * @expect_true: True if of type KUNIT_{EXPECT|ASSERT}_TRUE, false otherwise. @@ -137,9 +130,8 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_unary_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_UNARY_ASSERT_STRUCT(test, type, cond, expect_true) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(test, \ - type, \ +#define KUNIT_INIT_UNARY_ASSERT_STRUCT(type, cond, expect_true) { \ + .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ kunit_unary_assert_format), \ .condition = cond, \ .expected_true = expect_true \ @@ -167,7 +159,6 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_PTR_NOT_ERR_ASSERT_STRUCT() - Initializes a * &struct kunit_ptr_not_err_assert. - * @test: The test case that this expectation/assertion is associated with. * @type: The type (assertion or expectation) of this kunit_assert. * @txt: A string representation of the expression passed to the expectation. * @val: The actual evaluated pointer value of the expression. @@ -175,9 +166,8 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_ptr_not_err_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_PTR_NOT_ERR_STRUCT(test, type, txt, val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(test, \ - type, \ +#define KUNIT_INIT_PTR_NOT_ERR_STRUCT(type, txt, val) { \ + .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ kunit_ptr_not_err_assert_format), \ .text = txt, \ .value = val \ @@ -212,7 +202,6 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_BINARY_ASSERT_STRUCT() - Initializes a * &struct kunit_binary_assert. - * @test: The test case that this expectation/assertion is associated with. * @type: The type (assertion or expectation) of this kunit_assert. * @op_str: A string representation of the comparison operator (e.g. "=="). * @left_str: A string representation of the expression in the left slot. @@ -223,15 +212,13 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_binary_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_BINARY_ASSERT_STRUCT(test, \ - type, \ +#define KUNIT_INIT_BINARY_ASSERT_STRUCT(type, \ op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(test, \ - type, \ + .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ kunit_binary_assert_format), \ .operation = op_str, \ .left_text = left_str, \ @@ -269,7 +256,6 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT() - Initializes a * &struct kunit_binary_ptr_assert. - * @test: The test case that this expectation/assertion is associated with. * @type: The type (assertion or expectation) of this kunit_assert. * @op_str: A string representation of the comparison operator (e.g. "=="). * @left_str: A string representation of the expression in the left slot. @@ -280,15 +266,13 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_binary_ptr_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT(test, \ - type, \ +#define KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT(type, \ op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(test, \ - type, \ + .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ kunit_binary_ptr_assert_format), \ .operation = op_str, \ .left_text = left_str, \ @@ -326,7 +310,6 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_BINARY_STR_ASSERT_STRUCT() - Initializes a * &struct kunit_binary_str_assert. - * @test: The test case that this expectation/assertion is associated with. * @type: The type (assertion or expectation) of this kunit_assert. * @op_str: A string representation of the comparison operator (e.g. "=="). * @left_str: A string representation of the expression in the left slot. @@ -337,15 +320,13 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_binary_str_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(test, \ - type, \ +#define KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(type, \ op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(test, \ - type, \ + .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ kunit_binary_str_assert_format), \ .operation = op_str, \ .left_text = left_str, \ -- cgit v1.2.3-71-gd317 From 21957f90b28f6bc118c055e3e564d45f6e4df45d Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Thu, 13 Jan 2022 08:59:30 -0800 Subject: kunit: split out part of kunit_assert into a static const This is per Linus's suggestion in [1]. The issue there is that every KUNIT_EXPECT/KUNIT_ASSERT puts a kunit_assert object onto the stack. Normally we rely on compilers to elide this, but when that doesn't work out, this blows up the stack usage of kunit test functions. We can move some data off the stack by making it static. This change introduces a new `struct kunit_loc` to hold the file and line number and then just passing assert_type (EXPECT or ASSERT) as an argument. In [1], it was suggested to also move out the format string as well, but users could theoretically craft a format string at runtime, so we can't. This change leaves a copy of `assert_type` in kunit_assert for now because cleaning up all the macros to not pass it around is a bit more involved. Here's an example of the expanded code for KUNIT_FAIL(): if (__builtin_expect(!!(!(false)), 0)) { static const struct kunit_loc loc = { .file = ... }; struct kunit_fail_assert __assertion = { .assert = { .type ... }; kunit_do_failed_assertion(test, &loc, KUNIT_EXPECTATION, &__assertion.assert, ...); }; [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ Signed-off-by: Daniel Latypov Suggested-by: Linus Torvalds Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/assert.h | 25 +++++++++++++++++-------- include/kunit/test.h | 12 +++++++++++- lib/kunit/assert.c | 9 +++++---- lib/kunit/test.c | 15 +++++++++------ 4 files changed, 42 insertions(+), 19 deletions(-) (limited to 'include/kunit/assert.h') diff --git a/include/kunit/assert.h b/include/kunit/assert.h index f568166ef034..0da1bbdd1ee8 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -28,11 +28,21 @@ enum kunit_assert_type { KUNIT_EXPECTATION, }; +/** + * struct kunit_loc - Identifies the source location of a line of code. + * @line: the line number in the file. + * @file: the file name. + */ +struct kunit_loc { + int line; + const char *file; +}; + +#define KUNIT_CURRENT_LOC { .file = __FILE__, .line = __LINE__ } + /** * struct kunit_assert - Data for printing a failed assertion or expectation. * @type: the type (either an expectation or an assertion) of this kunit_assert. - * @line: the source code line number that the expectation/assertion is at. - * @file: the file path of the source file that the expectation/assertion is in. * @message: an optional message to provide additional context. * @format: a function which formats the data in this kunit_assert to a string. * @@ -40,9 +50,9 @@ enum kunit_assert_type { * format a string to a user reporting the failure. */ struct kunit_assert { + // TODO(dlatypov@google.com): delete this unused field when we've + // updated all the related KUNIT_INIT_ASSERT* macros. enum kunit_assert_type type; - int line; - const char *file; struct va_format message; void (*format)(const struct kunit_assert *assert, struct string_stream *stream); @@ -65,14 +75,13 @@ struct kunit_assert { */ #define KUNIT_INIT_ASSERT_STRUCT(assert_type, fmt) { \ .type = assert_type, \ - .file = __FILE__, \ - .line = __LINE__, \ .message = KUNIT_INIT_VA_FMT_NULL, \ .format = fmt \ } -void kunit_base_assert_format(const struct kunit_assert *assert, - struct string_stream *stream); +void kunit_assert_prologue(const struct kunit_loc *loc, + enum kunit_assert_type type, + struct string_stream *stream); void kunit_assert_print_msg(const struct kunit_assert *assert, struct string_stream *stream); diff --git a/include/kunit/test.h b/include/kunit/test.h index 25ea3bce6663..7b752175e614 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -772,13 +772,18 @@ void __printf(2, 3) kunit_log_append(char *log, const char *fmt, ...); #define KUNIT_SUCCEED(test) do {} while (0) void kunit_do_failed_assertion(struct kunit *test, + const struct kunit_loc *loc, + enum kunit_assert_type type, struct kunit_assert *assert, const char *fmt, ...); -#define KUNIT_ASSERTION(test, pass, assert_class, INITIALIZER, fmt, ...) do { \ +#define KUNIT_ASSERTION(test, assert_type, pass, assert_class, INITIALIZER, fmt, ...) do { \ if (unlikely(!(pass))) { \ + static const struct kunit_loc loc = KUNIT_CURRENT_LOC; \ struct assert_class __assertion = INITIALIZER; \ kunit_do_failed_assertion(test, \ + &loc, \ + assert_type, \ &__assertion.assert, \ fmt, \ ##__VA_ARGS__); \ @@ -788,6 +793,7 @@ void kunit_do_failed_assertion(struct kunit *test, #define KUNIT_FAIL_ASSERTION(test, assert_type, fmt, ...) \ KUNIT_ASSERTION(test, \ + assert_type, \ false, \ kunit_fail_assert, \ KUNIT_INIT_FAIL_ASSERT_STRUCT(assert_type), \ @@ -818,6 +824,7 @@ void kunit_do_failed_assertion(struct kunit *test, fmt, \ ...) \ KUNIT_ASSERTION(test, \ + assert_type, \ !!(condition) == !!expected_true, \ kunit_unary_assert, \ KUNIT_INIT_UNARY_ASSERT_STRUCT(assert_type, \ @@ -876,6 +883,7 @@ do { \ typeof(right) __right = (right); \ \ KUNIT_ASSERTION(test, \ + assert_type, \ __left op __right, \ assert_class, \ ASSERT_CLASS_INIT(assert_type, \ @@ -1230,6 +1238,7 @@ do { \ const char *__right = (right); \ \ KUNIT_ASSERTION(test, \ + assert_type, \ strcmp(__left, __right) op 0, \ kunit_binary_str_assert, \ KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(assert_type, \ @@ -1289,6 +1298,7 @@ do { \ typeof(ptr) __ptr = (ptr); \ \ KUNIT_ASSERTION(test, \ + assert_type, \ !IS_ERR_OR_NULL(__ptr), \ kunit_ptr_not_err_assert, \ KUNIT_INIT_PTR_NOT_ERR_STRUCT(assert_type, \ diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index 4d9a1295efc7..9f4492a8e24e 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -10,12 +10,13 @@ #include "string-stream.h" -void kunit_base_assert_format(const struct kunit_assert *assert, +void kunit_assert_prologue(const struct kunit_loc *loc, + enum kunit_assert_type type, struct string_stream *stream) { const char *expect_or_assert = NULL; - switch (assert->type) { + switch (type) { case KUNIT_EXPECTATION: expect_or_assert = "EXPECTATION"; break; @@ -25,9 +26,9 @@ void kunit_base_assert_format(const struct kunit_assert *assert, } string_stream_add(stream, "%s FAILED at %s:%d\n", - expect_or_assert, assert->file, assert->line); + expect_or_assert, loc->file, loc->line); } -EXPORT_SYMBOL_GPL(kunit_base_assert_format); +EXPORT_SYMBOL_GPL(kunit_assert_prologue); void kunit_assert_print_msg(const struct kunit_assert *assert, struct string_stream *stream) diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 345a9dd88c27..7dec3248562f 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -240,7 +240,8 @@ static void kunit_print_string_stream(struct kunit *test, } } -static void kunit_fail(struct kunit *test, struct kunit_assert *assert) +static void kunit_fail(struct kunit *test, const struct kunit_loc *loc, + enum kunit_assert_type type, struct kunit_assert *assert) { struct string_stream *stream; @@ -250,12 +251,12 @@ static void kunit_fail(struct kunit *test, struct kunit_assert *assert) if (!stream) { WARN(true, "Could not allocate stream to print failed assertion in %s:%d\n", - assert->file, - assert->line); + loc->file, + loc->line); return; } - kunit_base_assert_format(assert, stream); + kunit_assert_prologue(loc, type, stream); assert->format(assert, stream); kunit_print_string_stream(test, stream); @@ -277,6 +278,8 @@ static void __noreturn kunit_abort(struct kunit *test) } void kunit_do_failed_assertion(struct kunit *test, + const struct kunit_loc *loc, + enum kunit_assert_type type, struct kunit_assert *assert, const char *fmt, ...) { @@ -286,11 +289,11 @@ void kunit_do_failed_assertion(struct kunit *test, assert->message.fmt = fmt; assert->message.va = &args; - kunit_fail(test, assert); + kunit_fail(test, loc, type, assert); va_end(args); - if (assert->type == KUNIT_ASSERTION) + if (type == KUNIT_ASSERTION) kunit_abort(test); } EXPORT_SYMBOL_GPL(kunit_do_failed_assertion); -- cgit v1.2.3-71-gd317 From 05a7da89c15ddb3fc7618a16f5941eca68fc441c Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Thu, 13 Jan 2022 08:59:31 -0800 Subject: kunit: drop unused assert_type from kunit_assert and clean up macros This field has been split out from kunit_assert to make the struct less heavy along with the filename and line number. This change drops the assert_type field and cleans up all the macros that were plumbing assert_type into kunit_assert. Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/assert.h | 48 ++++++++++++++---------------------------------- include/kunit/test.h | 14 +++++--------- 2 files changed, 19 insertions(+), 43 deletions(-) (limited to 'include/kunit/assert.h') diff --git a/include/kunit/assert.h b/include/kunit/assert.h index 0da1bbdd1ee8..f2b3ae5cc2de 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -42,7 +42,6 @@ struct kunit_loc { /** * struct kunit_assert - Data for printing a failed assertion or expectation. - * @type: the type (either an expectation or an assertion) of this kunit_assert. * @message: an optional message to provide additional context. * @format: a function which formats the data in this kunit_assert to a string. * @@ -50,9 +49,6 @@ struct kunit_loc { * format a string to a user reporting the failure. */ struct kunit_assert { - // TODO(dlatypov@google.com): delete this unused field when we've - // updated all the related KUNIT_INIT_ASSERT* macros. - enum kunit_assert_type type; struct va_format message; void (*format)(const struct kunit_assert *assert, struct string_stream *stream); @@ -68,13 +64,11 @@ struct kunit_assert { /** * KUNIT_INIT_ASSERT_STRUCT() - Initializer for a &struct kunit_assert. - * @assert_type: The type (assertion or expectation) of this kunit_assert. * @fmt: The formatting function which builds a string out of this kunit_assert. * * The base initializer for a &struct kunit_assert. */ -#define KUNIT_INIT_ASSERT_STRUCT(assert_type, fmt) { \ - .type = assert_type, \ +#define KUNIT_INIT_ASSERT_STRUCT(fmt) { \ .message = KUNIT_INIT_VA_FMT_NULL, \ .format = fmt \ } @@ -100,15 +94,13 @@ void kunit_fail_assert_format(const struct kunit_assert *assert, struct string_stream *stream); /** - * KUNIT_INIT_FAIL_ASSERT_STRUCT() - Initializer for &struct kunit_fail_assert. - * @type: The type (assertion or expectation) of this kunit_assert. + * 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(type) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ - kunit_fail_assert_format) \ +#define KUNIT_INIT_FAIL_ASSERT_STRUCT { \ + .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_fail_assert_format) \ } /** @@ -132,16 +124,14 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_UNARY_ASSERT_STRUCT() - Initializes &struct kunit_unary_assert. - * @type: The type (assertion or expectation) of this kunit_assert. * @cond: A string representation of the expression asserted true or false. * @expect_true: True if of type KUNIT_{EXPECT|ASSERT}_TRUE, false otherwise. * * Initializes a &struct kunit_unary_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_UNARY_ASSERT_STRUCT(type, cond, expect_true) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ - kunit_unary_assert_format), \ +#define KUNIT_INIT_UNARY_ASSERT_STRUCT(cond, expect_true) { \ + .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_unary_assert_format), \ .condition = cond, \ .expected_true = expect_true \ } @@ -168,16 +158,14 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_PTR_NOT_ERR_ASSERT_STRUCT() - Initializes a * &struct kunit_ptr_not_err_assert. - * @type: The type (assertion or expectation) of this kunit_assert. * @txt: A string representation of the expression passed to the expectation. * @val: The actual evaluated pointer value of the expression. * * Initializes a &struct kunit_ptr_not_err_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_PTR_NOT_ERR_STRUCT(type, txt, val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ - kunit_ptr_not_err_assert_format), \ +#define KUNIT_INIT_PTR_NOT_ERR_STRUCT(txt, val) { \ + .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_ptr_not_err_assert_format), \ .text = txt, \ .value = val \ } @@ -211,7 +199,6 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_BINARY_ASSERT_STRUCT() - Initializes a * &struct kunit_binary_assert. - * @type: The type (assertion or expectation) of this kunit_assert. * @op_str: A string representation of the comparison operator (e.g. "=="). * @left_str: A string representation of the expression in the left slot. * @left_val: The actual evaluated value of the expression in the left slot. @@ -221,14 +208,12 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_binary_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_BINARY_ASSERT_STRUCT(type, \ - op_str, \ +#define KUNIT_INIT_BINARY_ASSERT_STRUCT(op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ - kunit_binary_assert_format), \ + .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_assert_format), \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ @@ -275,14 +260,12 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_binary_ptr_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT(type, \ - op_str, \ +#define KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT(op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ - kunit_binary_ptr_assert_format), \ + .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_ptr_assert_format), \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ @@ -319,7 +302,6 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, /** * KUNIT_INIT_BINARY_STR_ASSERT_STRUCT() - Initializes a * &struct kunit_binary_str_assert. - * @type: The type (assertion or expectation) of this kunit_assert. * @op_str: A string representation of the comparison operator (e.g. "=="). * @left_str: A string representation of the expression in the left slot. * @left_val: The actual evaluated value of the expression in the left slot. @@ -329,14 +311,12 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, * Initializes a &struct kunit_binary_str_assert. Intended to be used in * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ -#define KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(type, \ - op_str, \ +#define KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(type, \ - kunit_binary_str_assert_format), \ + .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_str_assert_format), \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ diff --git a/include/kunit/test.h b/include/kunit/test.h index 7b752175e614..5964af750d93 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -796,7 +796,7 @@ void kunit_do_failed_assertion(struct kunit *test, assert_type, \ false, \ kunit_fail_assert, \ - KUNIT_INIT_FAIL_ASSERT_STRUCT(assert_type), \ + KUNIT_INIT_FAIL_ASSERT_STRUCT, \ fmt, \ ##__VA_ARGS__) @@ -827,8 +827,7 @@ void kunit_do_failed_assertion(struct kunit *test, assert_type, \ !!(condition) == !!expected_true, \ kunit_unary_assert, \ - KUNIT_INIT_UNARY_ASSERT_STRUCT(assert_type, \ - #condition, \ + KUNIT_INIT_UNARY_ASSERT_STRUCT(#condition, \ expected_true), \ fmt, \ ##__VA_ARGS__) @@ -886,8 +885,7 @@ do { \ assert_type, \ __left op __right, \ assert_class, \ - ASSERT_CLASS_INIT(assert_type, \ - #op, \ + ASSERT_CLASS_INIT(#op, \ #left, \ __left, \ #right, \ @@ -1241,8 +1239,7 @@ do { \ assert_type, \ strcmp(__left, __right) op 0, \ kunit_binary_str_assert, \ - KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(assert_type, \ - #op, \ + KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(#op, \ #left, \ __left, \ #right, \ @@ -1301,8 +1298,7 @@ do { \ assert_type, \ !IS_ERR_OR_NULL(__ptr), \ kunit_ptr_not_err_assert, \ - KUNIT_INIT_PTR_NOT_ERR_STRUCT(assert_type, \ - #ptr, \ + KUNIT_INIT_PTR_NOT_ERR_STRUCT(#ptr, \ __ptr), \ fmt, \ ##__VA_ARGS__); \ -- cgit v1.2.3-71-gd317 From 6419abb80e82c603bbec6d7f5af6c2f79fa5c4ae Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Tue, 25 Jan 2022 13:00:09 -0800 Subject: kunit: remove va_format from kunit_assert The concern is that having a lot of redundant fields in kunit_assert can blow up stack usage if the compiler doesn't optimize them away [1]. The comment on this field implies that it was meant to be initialized when the expect/assert was declared, but this only happens when we run kunit_do_failed_assertion(). We don't need to access it outside of that function, so move it out of the struct and make it a local variable there. This change also takes the chance to reduce the number of macros by inlining the now simplified KUNIT_INIT_ASSERT_STRUCT() macro. [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/assert.h | 43 +++++++++++++------------------------------ lib/kunit/assert.c | 27 ++++++++++++++++----------- lib/kunit/test.c | 12 +++++++----- 3 files changed, 36 insertions(+), 46 deletions(-) (limited to 'include/kunit/assert.h') diff --git a/include/kunit/assert.h b/include/kunit/assert.h index f2b3ae5cc2de..0b3704db54b6 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -42,44 +42,21 @@ struct kunit_loc { /** * struct kunit_assert - Data for printing a failed assertion or expectation. - * @message: an optional message to provide additional context. * @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 { - struct va_format message; void (*format)(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); }; -/** - * KUNIT_INIT_VA_FMT_NULL - Default initializer for struct va_format. - * - * Used inside a struct initialization block to initialize struct va_format to - * default values where fmt and va are null. - */ -#define KUNIT_INIT_VA_FMT_NULL { .fmt = NULL, .va = NULL } - -/** - * KUNIT_INIT_ASSERT_STRUCT() - Initializer for a &struct kunit_assert. - * @fmt: The formatting function which builds a string out of this kunit_assert. - * - * The base initializer for a &struct kunit_assert. - */ -#define KUNIT_INIT_ASSERT_STRUCT(fmt) { \ - .message = KUNIT_INIT_VA_FMT_NULL, \ - .format = fmt \ -} - void kunit_assert_prologue(const struct kunit_loc *loc, enum kunit_assert_type type, struct string_stream *stream); -void kunit_assert_print_msg(const struct kunit_assert *assert, - struct string_stream *stream); - /** * struct kunit_fail_assert - Represents a plain fail expectation/assertion. * @assert: The parent of this type. @@ -91,6 +68,7 @@ struct kunit_fail_assert { }; void kunit_fail_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); /** @@ -100,7 +78,7 @@ void kunit_fail_assert_format(const struct kunit_assert *assert, * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. */ #define KUNIT_INIT_FAIL_ASSERT_STRUCT { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_fail_assert_format) \ + .assert = { .format = kunit_fail_assert_format }, \ } /** @@ -120,6 +98,7 @@ struct kunit_unary_assert { }; void kunit_unary_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); /** @@ -131,7 +110,7 @@ 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 = KUNIT_INIT_ASSERT_STRUCT(kunit_unary_assert_format), \ + .assert = { .format = kunit_unary_assert_format }, \ .condition = cond, \ .expected_true = expect_true \ } @@ -153,6 +132,7 @@ struct kunit_ptr_not_err_assert { }; void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); /** @@ -165,7 +145,7 @@ 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 = KUNIT_INIT_ASSERT_STRUCT(kunit_ptr_not_err_assert_format), \ + .assert = { .format = kunit_ptr_not_err_assert_format }, \ .text = txt, \ .value = val \ } @@ -194,6 +174,7 @@ struct kunit_binary_assert { }; void kunit_binary_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); /** @@ -213,7 +194,7 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_assert_format), \ + .assert = { .format = kunit_binary_assert_format }, \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ @@ -245,6 +226,7 @@ struct kunit_binary_ptr_assert { }; void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); /** @@ -265,7 +247,7 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_ptr_assert_format), \ + .assert = { .format = kunit_binary_ptr_assert_format }, \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ @@ -297,6 +279,7 @@ struct kunit_binary_str_assert { }; void kunit_binary_str_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream); /** @@ -316,7 +299,7 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, left_val, \ right_str, \ right_val) { \ - .assert = KUNIT_INIT_ASSERT_STRUCT(kunit_binary_str_assert_format), \ + .assert = { .format = kunit_binary_str_assert_format }, \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index 9f4492a8e24e..c9c7ee0dfafa 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -30,22 +30,23 @@ void kunit_assert_prologue(const struct kunit_loc *loc, } EXPORT_SYMBOL_GPL(kunit_assert_prologue); -void kunit_assert_print_msg(const struct kunit_assert *assert, - struct string_stream *stream) +static void kunit_assert_print_msg(const struct va_format *message, + struct string_stream *stream) { - if (assert->message.fmt) - string_stream_add(stream, "\n%pV", &assert->message); + if (message->fmt) + string_stream_add(stream, "\n%pV", message); } -EXPORT_SYMBOL_GPL(kunit_assert_print_msg); void kunit_fail_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream) { - string_stream_add(stream, "%pV", &assert->message); + string_stream_add(stream, "%pV", message); } EXPORT_SYMBOL_GPL(kunit_fail_assert_format); void kunit_unary_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream) { struct kunit_unary_assert *unary_assert; @@ -60,11 +61,12 @@ void kunit_unary_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBTEST_INDENT "Expected %s to be false, but is true\n", unary_assert->condition); - kunit_assert_print_msg(assert, stream); + kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_unary_assert_format); void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream) { struct kunit_ptr_not_err_assert *ptr_assert; @@ -82,7 +84,7 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, ptr_assert->text, PTR_ERR(ptr_assert->value)); } - kunit_assert_print_msg(assert, stream); + kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_ptr_not_err_assert_format); @@ -110,6 +112,7 @@ static bool is_literal(struct kunit *test, const char *text, long long value, } void kunit_binary_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream) { struct kunit_binary_assert *binary_assert; @@ -132,11 +135,12 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld", binary_assert->right_text, binary_assert->right_value); - kunit_assert_print_msg(assert, stream); + kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_binary_assert_format); void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream) { struct kunit_binary_ptr_assert *binary_assert; @@ -155,7 +159,7 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px", binary_assert->right_text, binary_assert->right_value); - kunit_assert_print_msg(assert, stream); + kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_binary_ptr_assert_format); @@ -176,6 +180,7 @@ static bool is_str_literal(const char *text, const char *value) } void kunit_binary_str_assert_format(const struct kunit_assert *assert, + const struct va_format *message, struct string_stream *stream) { struct kunit_binary_str_assert *binary_assert; @@ -196,6 +201,6 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"", binary_assert->right_text, binary_assert->right_value); - kunit_assert_print_msg(assert, stream); + kunit_assert_print_msg(message, stream); } EXPORT_SYMBOL_GPL(kunit_binary_str_assert_format); diff --git a/lib/kunit/test.c b/lib/kunit/test.c index 7dec3248562f..3bca3bf5c15b 100644 --- a/lib/kunit/test.c +++ b/lib/kunit/test.c @@ -241,7 +241,8 @@ 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, struct kunit_assert *assert) + enum kunit_assert_type type, struct kunit_assert *assert, + const struct va_format *message) { struct string_stream *stream; @@ -257,7 +258,7 @@ static void kunit_fail(struct kunit *test, const struct kunit_loc *loc, } kunit_assert_prologue(loc, type, stream); - assert->format(assert, stream); + assert->format(assert, message, stream); kunit_print_string_stream(test, stream); @@ -284,12 +285,13 @@ void kunit_do_failed_assertion(struct kunit *test, const char *fmt, ...) { va_list args; + struct va_format message; va_start(args, fmt); - assert->message.fmt = fmt; - assert->message.va = &args; + message.fmt = fmt; + message.va = &args; - kunit_fail(test, loc, type, assert); + kunit_fail(test, loc, type, assert, &message); va_end(args); -- cgit v1.2.3-71-gd317 From 064ff292aca500d6b911dca6abe1ece22620d475 Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Tue, 25 Jan 2022 13:00:10 -0800 Subject: kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros We currently have 2 other versions of KUNIT_INIT_BINARY_ASSERT_STRUCT. The only differences are that * the format funcition they pass is different * the types of left_val/right_val should be different (integral, pointer, string). The latter doesn't actually matter since these macros are just plumbing them along to KUNIT_ASSERTION where they will get type checked. So combine them all into a single KUNIT_INIT_BINARY_ASSERT_STRUCT that now also takes the format function as a parameter. Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/assert.h | 68 ++++++++------------------------------------------ include/kunit/test.h | 20 ++++++++------- 2 files changed, 22 insertions(+), 66 deletions(-) (limited to 'include/kunit/assert.h') diff --git a/include/kunit/assert.h b/include/kunit/assert.h index 0b3704db54b6..649bfac9f406 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -178,23 +178,28 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, struct string_stream *stream); /** - * KUNIT_INIT_BINARY_ASSERT_STRUCT() - Initializes a - * &struct kunit_binary_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. * @op_str: A string representation of the comparison operator (e.g. "=="). * @left_str: A string representation of the expression in the left slot. * @left_val: The actual evaluated value of the expression in the left slot. * @right_str: A string representation of the expression in the right slot. * @right_val: The actual evaluated value of the expression in the right slot. * - * Initializes a &struct kunit_binary_assert. Intended to be used in - * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. + * Initializes a binary assert like kunit_binary_assert, + * kunit_binary_ptr_assert, etc. This relies on these structs having the same + * 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(op_str, \ +#define KUNIT_INIT_BINARY_ASSERT_STRUCT(format_func, \ + op_str, \ left_str, \ left_val, \ right_str, \ right_val) { \ - .assert = { .format = kunit_binary_assert_format }, \ + .assert = { .format = format_func }, \ .operation = op_str, \ .left_text = left_str, \ .left_value = left_val, \ @@ -229,32 +234,6 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, const struct va_format *message, struct string_stream *stream); -/** - * KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT() - Initializes a - * &struct kunit_binary_ptr_assert. - * @type: The type (assertion or expectation) of this kunit_assert. - * @op_str: A string representation of the comparison operator (e.g. "=="). - * @left_str: A string representation of the expression in the left slot. - * @left_val: The actual evaluated value of the expression in the left slot. - * @right_str: A string representation of the expression in the right slot. - * @right_val: The actual evaluated value of the expression in the right slot. - * - * Initializes a &struct kunit_binary_ptr_assert. Intended to be used in - * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. - */ -#define KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT(op_str, \ - left_str, \ - left_val, \ - right_str, \ - right_val) { \ - .assert = { .format = kunit_binary_ptr_assert_format }, \ - .operation = op_str, \ - .left_text = left_str, \ - .left_value = left_val, \ - .right_text = right_str, \ - .right_value = right_val \ -} - /** * struct kunit_binary_str_assert - An expectation/assertion that compares two * string values (for example, KUNIT_EXPECT_STREQ(test, foo, "bar")). @@ -282,29 +261,4 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, const struct va_format *message, struct string_stream *stream); -/** - * KUNIT_INIT_BINARY_STR_ASSERT_STRUCT() - Initializes a - * &struct kunit_binary_str_assert. - * @op_str: A string representation of the comparison operator (e.g. "=="). - * @left_str: A string representation of the expression in the left slot. - * @left_val: The actual evaluated value of the expression in the left slot. - * @right_str: A string representation of the expression in the right slot. - * @right_val: The actual evaluated value of the expression in the right slot. - * - * Initializes a &struct kunit_binary_str_assert. Intended to be used in - * KUNIT_EXPECT_* and KUNIT_ASSERT_* macros. - */ -#define KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(op_str, \ - left_str, \ - left_val, \ - right_str, \ - right_val) { \ - .assert = { .format = kunit_binary_str_assert_format }, \ - .operation = op_str, \ - .left_text = left_str, \ - .left_value = left_val, \ - .right_text = right_str, \ - .right_value = right_val \ -} - #endif /* _KUNIT_ASSERT_H */ diff --git a/include/kunit/test.h b/include/kunit/test.h index bf82c313223b..a93dfb8ff393 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -864,7 +864,7 @@ void kunit_do_failed_assertion(struct kunit *test, */ #define KUNIT_BASE_BINARY_ASSERTION(test, \ assert_class, \ - ASSERT_CLASS_INIT, \ + format_func, \ assert_type, \ left, \ op, \ @@ -879,11 +879,12 @@ do { \ assert_type, \ __left op __right, \ assert_class, \ - ASSERT_CLASS_INIT(#op, \ - #left, \ - __left, \ - #right, \ - __right), \ + KUNIT_INIT_BINARY_ASSERT_STRUCT(format_func, \ + #op, \ + #left, \ + __left, \ + #right, \ + __right), \ fmt, \ ##__VA_ARGS__); \ } while (0) @@ -897,7 +898,7 @@ do { \ ...) \ KUNIT_BASE_BINARY_ASSERTION(test, \ kunit_binary_assert, \ - KUNIT_INIT_BINARY_ASSERT_STRUCT, \ + kunit_binary_assert_format, \ assert_type, \ left, op, right, \ fmt, \ @@ -912,7 +913,7 @@ do { \ ...) \ KUNIT_BASE_BINARY_ASSERTION(test, \ kunit_binary_ptr_assert, \ - KUNIT_INIT_BINARY_PTR_ASSERT_STRUCT, \ + kunit_binary_ptr_assert_format, \ assert_type, \ left, op, right, \ fmt, \ @@ -933,7 +934,8 @@ do { \ assert_type, \ strcmp(__left, __right) op 0, \ kunit_binary_str_assert, \ - KUNIT_INIT_BINARY_STR_ASSERT_STRUCT(#op, \ + KUNIT_INIT_BINARY_ASSERT_STRUCT(kunit_binary_str_assert_format,\ + #op, \ #left, \ __left, \ #right, \ -- cgit v1.2.3-71-gd317 From 2b6861e2372bac68861c54372f68f6016a7484fc Mon Sep 17 00:00:00 2001 From: Daniel Latypov Date: Tue, 25 Jan 2022 13:00:11 -0800 Subject: kunit: factor out str constants from binary assertion structs If the compiler doesn't optimize them away, each kunit assertion (use of KUNIT_EXPECT_EQ, etc.) can use 88 bytes of stack space in the worst and most common case. This has led to compiler warnings and a suggestion from Linus to move data from the structs into static const's where possible [1]. This builds upon [2] which did so for the base struct kunit_assert type. That only reduced sizeof(struct kunit_binary_assert) from 88 to 64. Given these are by far the most commonly used asserts, this patch factors out the textual representations of the operands and comparator into another static const, saving 16 more bytes. In detail, KUNIT_EXPECT_EQ(test, 2 + 2, 5) yields the following struct (struct kunit_binary_assert) { .assert = , .operation = "==", .left_text = "2 + 2", .left_value = 4, .right_text = "5", .right_value = 5, } After this change static const struct kunit_binary_assert_text __text = { .operation = "==", .left_text = "2 + 2", .right_text = "5", }; (struct kunit_binary_assert) { .assert = , .text = &__text, .left_value = 4, .right_value = 5, } This also DRYs the code a bit more since these str fields were repeated for the string and pointer versions of kunit_binary_assert. Note: we could name the kunit_binary_assert_text fields left/right instead of left_text/right_text. But that would require changing the macros a bit since they have args called "left" and "right" which would be substituted in `.left = #left` as `.2 + 2 = \"2 + 2\"`. [1] https://groups.google.com/g/kunit-dev/c/i3fZXgvBrfA/m/VULQg1z6BAAJ [2] https://lore.kernel.org/linux-kselftest/20220113165931.451305-6-dlatypov@google.com/ Signed-off-by: Daniel Latypov Reviewed-by: David Gow Reviewed-by: Brendan Higgins Signed-off-by: Shuah Khan --- include/kunit/assert.h | 49 ++++++++++++++++++++++--------------------------- include/kunit/test.h | 20 +++++++++++++------- lib/kunit/assert.c | 38 +++++++++++++++++++------------------- 3 files changed, 54 insertions(+), 53 deletions(-) (limited to 'include/kunit/assert.h') diff --git a/include/kunit/assert.h b/include/kunit/assert.h index 649bfac9f406..4b52e12c2ae8 100644 --- a/include/kunit/assert.h +++ b/include/kunit/assert.h @@ -150,14 +150,25 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, .value = val \ } +/** + * struct kunit_binary_assert_text - holds strings for &struct + * kunit_binary_assert and friends to try and make the structs smaller. + * @operation: A string representation of the comparison operator (e.g. "=="). + * @left_text: A string representation of the left expression (e.g. "2+2"). + * @right_text: A string representation of the right expression (e.g. "2+2"). + */ +struct kunit_binary_assert_text { + const char *operation; + const char *left_text; + const char *right_text; +}; + /** * struct kunit_binary_assert - An expectation/assertion that compares two * non-pointer values (for example, KUNIT_EXPECT_EQ(test, 1 + 1, 2)). * @assert: The parent of this type. - * @operation: A string representation of the comparison operator (e.g. "=="). - * @left_text: A string representation of the expression in the left slot. + * @text: Holds the textual representations of the operands and op (e.g. "=="). * @left_value: The actual evaluated value of the expression in the left slot. - * @right_text: A string representation of the expression in the right slot. * @right_value: The actual evaluated value of the expression in the right slot. * * Represents an expectation/assertion that compares two non-pointer values. For @@ -166,10 +177,8 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert, */ struct kunit_binary_assert { struct kunit_assert assert; - const char *operation; - const char *left_text; + const struct kunit_binary_assert_text *text; long long left_value; - const char *right_text; long long right_value; }; @@ -182,10 +191,8 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, * kunit_binary_assert, kunit_binary_ptr_assert, etc. * * @format_func: a function which formats the assert to a string. - * @op_str: A string representation of the comparison operator (e.g. "=="). - * @left_str: A string representation of the expression in the left slot. + * @text_: Pointer to a kunit_binary_assert_text. * @left_val: The actual evaluated value of the expression in the left slot. - * @right_str: A string representation of the expression in the right slot. * @right_val: The actual evaluated value of the expression in the right slot. * * Initializes a binary assert like kunit_binary_assert, @@ -194,16 +201,12 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, * This is ultimately used by binary assertion macros like KUNIT_EXPECT_EQ, etc. */ #define KUNIT_INIT_BINARY_ASSERT_STRUCT(format_func, \ - op_str, \ - left_str, \ + text_, \ left_val, \ - right_str, \ right_val) { \ .assert = { .format = format_func }, \ - .operation = op_str, \ - .left_text = left_str, \ + .text = text_, \ .left_value = left_val, \ - .right_text = right_str, \ .right_value = right_val \ } @@ -211,10 +214,8 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, * struct kunit_binary_ptr_assert - An expectation/assertion that compares two * pointer values (for example, KUNIT_EXPECT_PTR_EQ(test, foo, bar)). * @assert: The parent of this type. - * @operation: A string representation of the comparison operator (e.g. "=="). - * @left_text: A string representation of the expression in the left slot. + * @text: Holds the textual representations of the operands and op (e.g. "=="). * @left_value: The actual evaluated value of the expression in the left slot. - * @right_text: A string representation of the expression in the right slot. * @right_value: The actual evaluated value of the expression in the right slot. * * Represents an expectation/assertion that compares two pointer values. For @@ -223,10 +224,8 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, */ struct kunit_binary_ptr_assert { struct kunit_assert assert; - const char *operation; - const char *left_text; + const struct kunit_binary_assert_text *text; const void *left_value; - const char *right_text; const void *right_value; }; @@ -238,10 +237,8 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, * struct kunit_binary_str_assert - An expectation/assertion that compares two * string values (for example, KUNIT_EXPECT_STREQ(test, foo, "bar")). * @assert: The parent of this type. - * @operation: A string representation of the comparison operator (e.g. "=="). - * @left_text: A string representation of the expression in the left slot. + * @text: Holds the textual representations of the operands and comparator. * @left_value: The actual evaluated value of the expression in the left slot. - * @right_text: A string representation of the expression in the right slot. * @right_value: The actual evaluated value of the expression in the right slot. * * Represents an expectation/assertion that compares two string values. For @@ -250,10 +247,8 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, */ struct kunit_binary_str_assert { struct kunit_assert assert; - const char *operation; - const char *left_text; + const struct kunit_binary_assert_text *text; const char *left_value; - const char *right_text; const char *right_value; }; diff --git a/include/kunit/test.h b/include/kunit/test.h index a93dfb8ff393..088ff394ae94 100644 --- a/include/kunit/test.h +++ b/include/kunit/test.h @@ -874,16 +874,19 @@ void kunit_do_failed_assertion(struct kunit *test, do { \ typeof(left) __left = (left); \ typeof(right) __right = (right); \ + static const struct kunit_binary_assert_text __text = { \ + .operation = #op, \ + .left_text = #left, \ + .right_text = #right, \ + }; \ \ KUNIT_ASSERTION(test, \ assert_type, \ __left op __right, \ assert_class, \ KUNIT_INIT_BINARY_ASSERT_STRUCT(format_func, \ - #op, \ - #left, \ + &__text, \ __left, \ - #right, \ __right), \ fmt, \ ##__VA_ARGS__); \ @@ -928,17 +931,20 @@ do { \ ...) \ do { \ const char *__left = (left); \ - const char *__right = (right); \ + const char *__right = (right); \ + static const struct kunit_binary_assert_text __text = { \ + .operation = #op, \ + .left_text = #left, \ + .right_text = #right, \ + }; \ \ KUNIT_ASSERTION(test, \ assert_type, \ strcmp(__left, __right) op 0, \ kunit_binary_str_assert, \ KUNIT_INIT_BINARY_ASSERT_STRUCT(kunit_binary_str_assert_format,\ - #op, \ - #left, \ + &__text, \ __left, \ - #right, \ __right), \ fmt, \ ##__VA_ARGS__); \ diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c index c9c7ee0dfafa..d00d6d181ee8 100644 --- a/lib/kunit/assert.c +++ b/lib/kunit/assert.c @@ -122,18 +122,18 @@ void kunit_binary_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", - binary_assert->left_text, - binary_assert->operation, - binary_assert->right_text); - if (!is_literal(stream->test, binary_assert->left_text, + binary_assert->text->left_text, + binary_assert->text->operation, + binary_assert->text->right_text); + if (!is_literal(stream->test, binary_assert->text->left_text, binary_assert->left_value, stream->gfp)) string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld\n", - binary_assert->left_text, + binary_assert->text->left_text, binary_assert->left_value); - if (!is_literal(stream->test, binary_assert->right_text, + if (!is_literal(stream->test, binary_assert->text->right_text, binary_assert->right_value, stream->gfp)) string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld", - binary_assert->right_text, + binary_assert->text->right_text, binary_assert->right_value); kunit_assert_print_msg(message, stream); } @@ -150,14 +150,14 @@ void kunit_binary_ptr_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", - binary_assert->left_text, - binary_assert->operation, - binary_assert->right_text); + binary_assert->text->left_text, + binary_assert->text->operation, + binary_assert->text->right_text); string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px\n", - binary_assert->left_text, + binary_assert->text->left_text, binary_assert->left_value); string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %px", - binary_assert->right_text, + binary_assert->text->right_text, binary_assert->right_value); kunit_assert_print_msg(message, stream); } @@ -190,16 +190,16 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert, string_stream_add(stream, KUNIT_SUBTEST_INDENT "Expected %s %s %s, but\n", - binary_assert->left_text, - binary_assert->operation, - binary_assert->right_text); - if (!is_str_literal(binary_assert->left_text, binary_assert->left_value)) + binary_assert->text->left_text, + binary_assert->text->operation, + binary_assert->text->right_text); + if (!is_str_literal(binary_assert->text->left_text, binary_assert->left_value)) string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"\n", - binary_assert->left_text, + binary_assert->text->left_text, binary_assert->left_value); - if (!is_str_literal(binary_assert->right_text, binary_assert->right_value)) + if (!is_str_literal(binary_assert->text->right_text, binary_assert->right_value)) string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"", - binary_assert->right_text, + binary_assert->text->right_text, binary_assert->right_value); kunit_assert_print_msg(message, stream); } -- cgit v1.2.3-71-gd317