From: David Gow <davidgow@google.com>
To: Daniel Latypov <dlatypov@google.com>
Cc: brendanhiggins@google.com, linux-kernel@vger.kernel.org,
kunit-dev@googlegroups.com, linux-kselftest@vger.kernel.org,
skhan@linuxfoundation.org
Subject: Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros
Date: Thu, 27 Jan 2022 11:36:45 +0800 [thread overview]
Message-ID: <CABVgOSmESbi9Cm6GqGn+G1vD8qmX8wwPwtCxzUOZp-qRpBKQYA@mail.gmail.com> (raw)
In-Reply-To: <20220125210011.3817742-3-dlatypov@google.com>
On Wed, Jan 26, 2022 at 5:00 AM Daniel Latypov <dlatypov@google.com> 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@google.com>
> ---
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 prev parent reply other threads:[~2022-01-27 3:37 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-01-25 21:00 [PATCH 0/3] kunit: further reduce stack usage of asserts Daniel Latypov
2022-01-25 21:00 ` [PATCH 1/3] kunit: remove va_format from kunit_assert Daniel Latypov
2022-01-27 3:34 ` David Gow
2022-01-27 21:22 ` Brendan Higgins
2022-01-25 21:00 ` [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros Daniel Latypov
2022-01-27 3:36 ` David Gow [this message]
2022-01-27 3:37 ` David Gow
2022-01-27 21:31 ` Brendan Higgins
2022-01-25 21:00 ` [PATCH 3/3] kunit: factor out str constants from binary assertion structs Daniel Latypov
2022-01-27 3:39 ` David Gow
2022-01-27 20:21 ` Daniel Latypov
2022-01-27 20:27 ` Daniel Latypov
2022-01-27 21:47 ` Brendan Higgins
2022-01-27 21:46 ` Brendan Higgins
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=CABVgOSmESbi9Cm6GqGn+G1vD8qmX8wwPwtCxzUOZp-qRpBKQYA@mail.gmail.com \
--to=davidgow@google.com \
--cc=brendanhiggins@google.com \
--cc=dlatypov@google.com \
--cc=kunit-dev@googlegroups.com \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=skhan@linuxfoundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).