Re: [RFC PATCH v2] kunit: Add "hooks" to call into KUnit when it's built as a module

From: Daniel Latypov
Date: Fri Jan 27 2023 - 00:39:01 EST


On Tue, Jan 24, 2023 at 12:04 AM David Gow <davidgow@xxxxxxxxxx> 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
> function pointer to the real implementation of
> __kunit_fail_current_test(), which is populated when the KUnit module is
> loaded.
>
> A new header, kunit/hooks-table.h, contains a table of all hooks, and is
> repeatedly included with different definitions of the KUNIT_HOOK() in
> order to automatically generate the needed function pointer tables. When

Perhaps I'm overlooking something and this is a dumb question.

Is there a reason we can't go with a less-clever approach?
Like have a global struct?
We could memset it to 0 to clear it instead of defining a macro to set
individual variables to NULL?

i.e.

// hooks.h
extern struct kunit_hook_table {
__printf(3, 4) void (*fail_current_test)(const char*, int,
const char*, ...);
} kunit_hooks;

//hooks.c
struct kunit_hook_table kunit_hooks;

// in test.c
// here all the functions should be in scope for us to use
static void kunit_set_hooks(void)
{
kunit_hooks.fail_current_test = __kunit_fail_current_test;
...
}

static int __init kunit_init(void)
{
...
kunit_set_hooks();
...
}

static void __exit kunit_exit(void)
{
...
memset(&kunit_hooks, 0, sizeof(kunit_hooks));
}

> KUnit is disabled, or the module is not loaded, these function pointers
> are all NULL. This shouldn't be a problem, as they're all used behind
> wrappers which check kunit_running and/or that the pointer is non-NULL.
>
> This can then be extended for future features which require similar
> "hook" behaviour, such as static stubs:
> https://lore.kernel.org/all/20221208061841.2186447-1-davidgow@xxxxxxxxxx/
>
> Signed-off-by: David Gow <davidgow@xxxxxxxxxx>