Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros

From: David Gow
Date: Wed Jan 26 2022 - 22:37:01 EST

Next message: David Gow: "Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros"
Previous message: Stephen Rothwell: "Re: linux-next: build warnings after merge of the jc_docs tree"
In reply to: Daniel Latypov: "[PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros"
Next in thread: David Gow: "Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

On Wed, Jan 26, 2022 at 5:00 AM Daniel Latypov <dlatypov@xxxxxxxxxx> wrote:
>
> We currently have 2 other versions of KUNIT_INIT_BINARY_ASSERT_STRUCT.
> The only differences are that
> * the format funcition they pass is different
Minor nit: s/funcition/function/
> * 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 <dlatypov@xxxxxxxxxx>
> ---

Makes sense to me.

One minor spelling nit: probably not worth a whole new version over,
but if v2 ever happens, worth fixing at the same time...

-- David

> include/kunit/assert.h | 68 +++++++-----------------------------------
> include/kunit/test.h | 20 +++++++------
> 2 files changed, 22 insertions(+), 66 deletions(-)
>
> 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, \
> --
> 2.35.0.rc2.247.g8bbb082509-goog
>

Next message: David Gow: "Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros"
Previous message: Stephen Rothwell: "Re: linux-next: build warnings after merge of the jc_docs tree"
In reply to: Daniel Latypov: "[PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros"
Next in thread: David Gow: "Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]