MIME-Version: 1.0
References: <20230128071007.1134942-1-davidgow@google.com>
In-Reply-To: <20230128071007.1134942-1-davidgow@google.com>
From:   Brendan Higgins <brendanhiggins@google.com>
Date:   Mon, 30 Jan 2023 19:04:44 -0500
Message-ID: <CAFd5g47r-J+0FMt+F3pmVWBBKNXEqRD03Jh_i2HjUBhZBM8rdg@mail.gmail.com>
Subject: Re: [PATCH] kunit: Add "hooks" to call into KUnit when it's built as
 a module
To:     David Gow <davidgow@google.com>
Cc:     Brendan Higgins <brendan.higgins@linux.dev>,
        Kees Cook <keescook@chromium.org>,
        Shuah Khan <skhan@linuxfoundation.org>,
        Daniel Latypov <dlatypov@google.com>,
        Rae Moar <rmoar@google.com>, kunit-dev@googlegroups.com,
        linux-kselftest@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Sat, Jan 28, 2023 at 2:10 AM 'David Gow' via KUnit Development
<kunit-dev@googlegroups.com> wrote:
>
> KUnit has several macros and functions intended for use from non-test
> code. These hooks, currently the kunit_get_current_test() and
> kunit_fail_current_test() macros, didn't work when CONFIG_KUNIT=m.
>
> In order to support this case, the required functions and static data
> need to be available unconditionally, even when KUnit itself is not
> built-in. The new 'hooks.c' file is therefore always included, and has
> both the static key required for kunit_get_current_test(), and a table
> of function pointers in struct kunit_hooks_table. This is filled in with
> the real implementations by kunit_install_hooks(), which is kept in
> hooks-impl.h and called when the kunit module is loaded.
>
> This can  be extended for future features which require similar
> "hook" behaviour, such as static stubs, by simply adding new entries to
> the struct, and the appropriate code to set them.
> Signed-off-by: David Gow <davidgow@google.com>

I agree with Rae that a new file for just setting the pointer seems a
bit much, but I also understand the point of separating it out now -
not sure of a better place for it to live. Aside from that, looks good
to me:

Reviewed-by: Brendan Higgins <brendanhiggins@google.com>