lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CABVgOSmESbi9Cm6GqGn+G1vD8qmX8wwPwtCxzUOZp-qRpBKQYA@mail.gmail.com>
Date:   Thu, 27 Jan 2022 11:36:45 +0800
From:   David Gow <davidgow@...gle.com>
To:     Daniel Latypov <dlatypov@...gle.com>
Cc:     brendanhiggins@...gle.com, linux-kernel@...r.kernel.org,
        kunit-dev@...glegroups.com, linux-kselftest@...r.kernel.org,
        skhan@...uxfoundation.org
Subject: Re: [PATCH 2/3] kunit: consolidate KUNIT_INIT_BINARY_ASSERT_STRUCT macros

On Wed, Jan 26, 2022 at 5:00 AM Daniel Latypov <dlatypov@...gle.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@...gle.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
>

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ