linux-kernel - Re: [PATCH 2/2] kunit: Add example of kunit_activate_static

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] [day] [month] [year] [list]

Message-ID: <CA+GJov7jyn461d-wyTMr=kN5TyFP=muox75rtj4M3kYeu4=JfQ@mail.gmail.com>
Date: Tue, 19 Dec 2023 18:15:48 -0500
From: Rae Moar <rmoar@...gle.com>
To: Richard Fitzgerald <rf@...nsource.cirrus.com>
Cc: brendan.higgins@...ux.dev, davidgow@...gle.com, 
	linux-kselftest@...r.kernel.org, kunit-dev@...glegroups.com, 
	linux-kernel@...r.kernel.org, patches@...nsource.cirrus.com
Subject: Re: [PATCH 2/2] kunit: Add example of kunit_activate_static_stub()
 with pointer-to-function

On Mon, Dec 18, 2023 at 11:10 AM Richard Fitzgerald
<rf@...nsource.cirrus.com> wrote:
>
> Adds a variant of example_static_stub_test() that shows use of a
> pointer-to-function with kunit_activate_static_stub().
>
> A const pointer to the add_one() function is declared. This
> pointer-to-function is passed to kunit_activate_static_stub() and
> kunit_deactivate_static_stub() instead of passing add_one directly.
>
> Signed-off-by: Richard Fitzgerald <rf@...nsource.cirrus.com>

Hello!

This test looks good to me. However, I had issues applying this patch
so I think it needs rebasing due to the newest additions to
kselftest/kunit. But otherwise this patch looks good other than my
very small comment below.

Thanks!
-Rae

> ---
>  lib/kunit/kunit-example-test.c | 27 +++++++++++++++++++++++++++
>  1 file changed, 27 insertions(+)
>
> diff --git a/lib/kunit/kunit-example-test.c b/lib/kunit/kunit-example-test.c
> index d2f7a3c62c18..9e57f341dc37 100644
> --- a/lib/kunit/kunit-example-test.c
> +++ b/lib/kunit/kunit-example-test.c
> @@ -168,6 +168,8 @@ static int subtract_one(int i)
>         return i - 1;
>  }
>
> +static int (* const add_one_fn_ptr)(int i) = add_one;

This is a bit of a nit but could you add a brief comment above this
pointer definition? This would then match the commenting on the other
functions in kunit-example-test and provide more context for those
looking at the example tests.

> +
>  /*
>   * This test shows the use of static stubs.
>   */
> @@ -187,6 +189,30 @@ static void example_static_stub_test(struct kunit *test)
>         KUNIT_EXPECT_EQ(test, add_one(1), 2);
>  }
>
> +/*
> + * This test shows the use of static stubs when the function being
> + * replaced is provided as a pointer-to-function instead of the
> + * actual function. This is useful for providing access to static
> + * functions in a module by exporting a pointer to that function
> + * instead of having to change the static function to a non-static
> + * exported function.
> + */
> +static void example_static_stub_using_fn_ptr_test(struct kunit *test)
> +{
> +       /* By default, function is not stubbed. */
> +       KUNIT_EXPECT_EQ(test, add_one(1), 2);
> +
> +       /* Replace add_one() with subtract_one(). */
> +       kunit_activate_static_stub(test, add_one_fn_ptr, subtract_one);
> +
> +       /* add_one() is now replaced. */
> +       KUNIT_EXPECT_EQ(test, add_one(1), 0);
> +
> +       /* Return add_one() to normal. */
> +       kunit_deactivate_static_stub(test, add_one_fn_ptr);
> +       KUNIT_EXPECT_EQ(test, add_one(1), 2);
> +}
> +
>  static const struct example_param {
>         int value;
>  } example_params_array[] = {
> @@ -245,6 +271,7 @@ static struct kunit_case example_test_cases[] = {
>         KUNIT_CASE(example_mark_skipped_test),
>         KUNIT_CASE(example_all_expect_macros_test),
>         KUNIT_CASE(example_static_stub_test),
> +       KUNIT_CASE(example_static_stub_using_fn_ptr_test),
>         KUNIT_CASE_PARAM(example_params_test, example_gen_params),
>         KUNIT_CASE_SLOW(example_slow_test),
>         {}
> --
> 2.30.2
>