[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CA+GJov7LzOSfFVRA4rSSeR_AeryWC7hPpXki8RDQDywa4ngEig@mail.gmail.com>
Date: Fri, 21 Jun 2024 17:19:34 -0400
From: Rae Moar <rmoar@...gle.com>
To: Ivan Orlov <ivan.orlov0322@...il.com>
Cc: brendan.higgins@...ux.dev, davidgow@...gle.com,
linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com,
linux-kernel@...r.kernel.org, skhan@...uxfoundation.org
Subject: Re: [PATCH v2 4/5] kunit: assert_test: Prepare to be merged into kunit-test.c
On Tue, Jun 18, 2024 at 1:03 PM Ivan Orlov <ivan.orlov0322@...il.com> wrote:
>
> Add 'kunit_assert_' prefix for 'is_literal' and 'is_str_literal'
> functions. This way we will be sure that we are not exporting ambiguous
> symbols into the KUnit namespace.
>
> Export these (and other) functions from assert into the KUnit namespace,
> so we could use them in the tests (and cover them as well).
>
> Signed-off-by: Ivan Orlov <ivan.orlov0322@...il.com>
Hi!
This looks good to me. I am happy with the changes since v1.
Reviewed-by: Rae Moar <rmoar@...gle.com>
Thanks for your work on this!
-Rae
> ---
> V1 -> V2:
> - Besides exporting the non-static functions from assert.c into the
> KUnit namespace, rename some of them as well (add kunit_assert_ prefix
> to make their names less ambiguous).
>
> include/kunit/assert.h | 4 ++--
> lib/kunit/assert.c | 19 +++++++++++++------
> lib/kunit/assert_test.c | 40 ++++++++++++++++++++--------------------
> 3 files changed, 35 insertions(+), 28 deletions(-)
>
> diff --git a/include/kunit/assert.h b/include/kunit/assert.h
> index 7e7490a74b13..3994acc520ae 100644
> --- a/include/kunit/assert.h
> +++ b/include/kunit/assert.h
> @@ -221,8 +221,8 @@ void kunit_mem_assert_format(const struct kunit_assert *assert,
> #if IS_ENABLED(CONFIG_KUNIT)
> void kunit_assert_print_msg(const struct va_format *message,
> struct string_stream *stream);
> -bool is_literal(const char *text, long long value);
> -bool is_str_literal(const char *text, const char *value);
> +bool kunit_assert_is_literal(const char *text, long long value);
> +bool kunit_assert_is_str_literal(const char *text, const char *value);
> void kunit_assert_hexdump(struct string_stream *stream,
> const void *buf,
> const void *compared_buf,
> diff --git a/lib/kunit/assert.c b/lib/kunit/assert.c
> index 867aa5c4bccf..62b86bf5603e 100644
> --- a/lib/kunit/assert.c
> +++ b/lib/kunit/assert.c
> @@ -38,6 +38,7 @@ void kunit_assert_print_msg(const struct va_format *message,
> if (message->fmt)
> string_stream_add(stream, "\n%pV", message);
> }
> +EXPORT_SYMBOL_IF_KUNIT(kunit_assert_print_msg);
>
> void kunit_fail_assert_format(const struct kunit_assert *assert,
> const struct va_format *message,
> @@ -91,7 +92,8 @@ void kunit_ptr_not_err_assert_format(const struct kunit_assert *assert,
> EXPORT_SYMBOL_GPL(kunit_ptr_not_err_assert_format);
>
> /* Checks if `text` is a literal representing `value`, e.g. "5" and 5 */
> -VISIBLE_IF_KUNIT bool is_literal(const char *text, long long value)
> +VISIBLE_IF_KUNIT
> +bool kunit_assert_is_literal(const char *text, long long value)
> {
> char *buffer;
> int len;
> @@ -112,6 +114,7 @@ VISIBLE_IF_KUNIT bool is_literal(const char *text, long long value)
>
> return ret;
> }
> +EXPORT_SYMBOL_IF_KUNIT(kunit_assert_is_literal);
>
> void kunit_binary_assert_format(const struct kunit_assert *assert,
> const struct va_format *message,
> @@ -127,12 +130,12 @@ void kunit_binary_assert_format(const struct kunit_assert *assert,
> binary_assert->text->left_text,
> binary_assert->text->operation,
> binary_assert->text->right_text);
> - if (!is_literal(binary_assert->text->left_text, binary_assert->left_value))
> + if (!kunit_assert_is_literal(binary_assert->text->left_text, binary_assert->left_value))
> string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)\n",
> binary_assert->text->left_text,
> binary_assert->left_value,
> binary_assert->left_value);
> - if (!is_literal(binary_assert->text->right_text, binary_assert->right_value))
> + if (!kunit_assert_is_literal(binary_assert->text->right_text, binary_assert->right_value))
> string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == %lld (0x%llx)",
> binary_assert->text->right_text,
> binary_assert->right_value,
> @@ -168,7 +171,8 @@ EXPORT_SYMBOL_GPL(kunit_binary_ptr_assert_format);
> /* Checks if KUNIT_EXPECT_STREQ() args were string literals.
> * Note: `text` will have ""s where as `value` will not.
> */
> -VISIBLE_IF_KUNIT bool is_str_literal(const char *text, const char *value)
> +VISIBLE_IF_KUNIT
> +bool kunit_assert_is_str_literal(const char *text, const char *value)
> {
> int len;
>
> @@ -180,6 +184,7 @@ VISIBLE_IF_KUNIT bool is_str_literal(const char *text, const char *value)
>
> return strncmp(text + 1, value, len - 2) == 0;
> }
> +EXPORT_SYMBOL_IF_KUNIT(kunit_assert_is_str_literal);
>
> void kunit_binary_str_assert_format(const struct kunit_assert *assert,
> const struct va_format *message,
> @@ -195,11 +200,12 @@ void kunit_binary_str_assert_format(const struct kunit_assert *assert,
> 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))
> + if (!kunit_assert_is_str_literal(binary_assert->text->left_text, binary_assert->left_value))
> string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"\n",
> binary_assert->text->left_text,
> binary_assert->left_value);
> - if (!is_str_literal(binary_assert->text->right_text, binary_assert->right_value))
> + if (!kunit_assert_is_str_literal(binary_assert->text->right_text,
> + binary_assert->right_value))
> string_stream_add(stream, KUNIT_SUBSUBTEST_INDENT "%s == \"%s\"",
> binary_assert->text->right_text,
> binary_assert->right_value);
> @@ -232,6 +238,7 @@ void kunit_assert_hexdump(struct string_stream *stream,
> string_stream_add(stream, " %02x ", buf1[i]);
> }
> }
> +EXPORT_SYMBOL_IF_KUNIT(kunit_assert_hexdump);
>
> void kunit_mem_assert_format(const struct kunit_assert *assert,
> const struct va_format *message,
> diff --git a/lib/kunit/assert_test.c b/lib/kunit/assert_test.c
> index 4a5967712186..4999233180d6 100644
> --- a/lib/kunit/assert_test.c
> +++ b/lib/kunit/assert_test.c
> @@ -11,28 +11,28 @@
> #define ASSERT_TEST_EXPECT_CONTAIN(test, str, substr) KUNIT_EXPECT_TRUE(test, strstr(str, substr))
> #define ASSERT_TEST_EXPECT_NCONTAIN(test, str, substr) KUNIT_EXPECT_FALSE(test, strstr(str, substr))
>
> -static void kunit_test_is_literal(struct kunit *test)
> +static void kunit_test_assert_is_literal(struct kunit *test)
> {
> - KUNIT_EXPECT_TRUE(test, is_literal("5", 5));
> - KUNIT_EXPECT_TRUE(test, is_literal("0", 0));
> - KUNIT_EXPECT_TRUE(test, is_literal("1234567890", 1234567890));
> - KUNIT_EXPECT_TRUE(test, is_literal("-1234567890", -1234567890));
> - KUNIT_EXPECT_FALSE(test, is_literal("05", 5));
> - KUNIT_EXPECT_FALSE(test, is_literal("", 0));
> - KUNIT_EXPECT_FALSE(test, is_literal("-0", 0));
> - KUNIT_EXPECT_FALSE(test, is_literal("12#45", 1245));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_literal("5", 5));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_literal("0", 0));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_literal("1234567890", 1234567890));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_literal("-1234567890", -1234567890));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_literal("05", 5));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_literal("", 0));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_literal("-0", 0));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_literal("12#45", 1245));
> }
>
> -static void kunit_test_is_str_literal(struct kunit *test)
> +static void kunit_test_assert_is_str_literal(struct kunit *test)
> {
> - KUNIT_EXPECT_TRUE(test, is_str_literal("\"Hello, World!\"", "Hello, World!"));
> - KUNIT_EXPECT_TRUE(test, is_str_literal("\"\"", ""));
> - KUNIT_EXPECT_TRUE(test, is_str_literal("\"\"\"", "\""));
> - KUNIT_EXPECT_FALSE(test, is_str_literal("", ""));
> - KUNIT_EXPECT_FALSE(test, is_str_literal("\"", "\""));
> - KUNIT_EXPECT_FALSE(test, is_str_literal("\"Abacaba", "Abacaba"));
> - KUNIT_EXPECT_FALSE(test, is_str_literal("Abacaba\"", "Abacaba"));
> - KUNIT_EXPECT_FALSE(test, is_str_literal("\"Abacaba\"", "\"Abacaba\""));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_str_literal("\"Hello, World!\"", "Hello, World!"));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_str_literal("\"\"", ""));
> + KUNIT_EXPECT_TRUE(test, kunit_assert_is_str_literal("\"\"\"", "\""));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_str_literal("", ""));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_str_literal("\"", "\""));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_str_literal("\"Abacaba", "Abacaba"));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_str_literal("Abacaba\"", "Abacaba"));
> + KUNIT_EXPECT_FALSE(test, kunit_assert_is_str_literal("\"Abacaba\"", "\"Abacaba\""));
> }
>
> KUNIT_DEFINE_ACTION_WRAPPER(kfree_wrapper, kfree, const void *);
> @@ -366,8 +366,8 @@ static void kunit_test_mem_assert_format(struct kunit *test)
> }
>
> static struct kunit_case assert_test_cases[] = {
> - KUNIT_CASE(kunit_test_is_literal),
> - KUNIT_CASE(kunit_test_is_str_literal),
> + KUNIT_CASE(kunit_test_assert_is_literal),
> + KUNIT_CASE(kunit_test_assert_is_str_literal),
> KUNIT_CASE(kunit_test_assert_prologue),
> KUNIT_CASE(kunit_test_assert_print_msg),
> KUNIT_CASE(kunit_test_unary_assert_format),
> --
> 2.34.1
>
Powered by blists - more mailing lists