| Message ID | 20220125210011.3817742-3-dlatypov@google.com |
|---|---|
| State | Accepted |
| Commit | 064ff292aca500d6b911dca6abe1ece22620d475 |
| Headers | show |
| Series | kunit: further reduce stack usage of asserts | expand |
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 >
On Thu, Jan 27, 2022 at 11:36 AM David Gow <davidgow@google.com> wrote: > > 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... > Oops, forgot to add the Reviewed-by! Reviewed-by: David Gow <davidgow@google.com> > -- David >
On Tue, Jan 25, 2022 at 4:00 PM 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 > * 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> Reviewed-by: Brendan Higgins <brendanhiggins@google.com>
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, \
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 <dlatypov@google.com> --- include/kunit/assert.h | 68 +++++++----------------------------------- include/kunit/test.h | 20 +++++++------ 2 files changed, 22 insertions(+), 66 deletions(-)