On 21.08.2024 23:32, Rae Moar wrote:
On Wed, Aug 21, 2024 at 10:43 AM Michal Wajdeczko michal.wajdeczko@intel.com wrote:
Hello!
This is looking good and seems to be working well. I just had some questions below.
Thanks! -Rae
Currently, the 'static stub' API only allows function redirection for calls made from the kthread of the current test, which prevents the use of this functionalty when the tested code is also used by
A slight typo here: functionality
other threads, outside of the KUnit test, like from the workqueue.
Add another set of macros to allow redirection to the replacement functions, which, unlike the KUNIT_STATIC_STUB_REDIRECT, will affect all calls done during the test execution.
Signed-off-by: Michal Wajdeczko michal.wajdeczko@intel.com
Cc: David Gow davidgow@google.com Cc: Daniel Latypov dlatypov@google.com Cc: Lucas De Marchi lucas.demarchi@intel.com
include/kunit/static_stub.h | 80 +++++++++++++++++++++++++++++++++++++ lib/kunit/static_stub.c | 21 ++++++++++ 2 files changed, 101 insertions(+)
diff --git a/include/kunit/static_stub.h b/include/kunit/static_stub.h index bf940322dfc0..3dd98c8f3f1f 100644 --- a/include/kunit/static_stub.h +++ b/include/kunit/static_stub.h @@ -12,6 +12,7 @@
/* If CONFIG_KUNIT is not enabled, these stubs quietly disappear. */ #define KUNIT_STATIC_STUB_REDIRECT(real_fn_name, args...) do {} while (0) +#define KUNIT_FIXED_STUB_REDIRECT(stub, args...) do {} while (0)
#else
@@ -109,5 +110,84 @@ void __kunit_activate_static_stub(struct kunit *test, */ void kunit_deactivate_static_stub(struct kunit *test, void *real_fn_addr);
+/**
- KUNIT_FIXED_STUB_REDIRECT() - Call a fixed function stub if activated.
- @stub: The location of the function stub pointer
- @args: All of the arguments passed to this stub
- This is a function prologue which is used to allow calls to the current
- function to be redirected if a KUnit is running. If the stub is NULL or
- the KUnit is not running the function will continue execution as normal.
- The function stub pointer must be stored in a place that is accessible both
- from the test code that will activate this stub and from the function where
- we will do the redirection.
- Unlike the KUNIT_STATIC_STUB_REDIRECT(), this redirection will work
- even if the caller is not in a KUnit context (like a worker thread).
- Example:
- .. code-block:: c
static int (*func_stub)(int n);
int real_func(int n)
{
KUNIT_FIXED_STUB_REDIRECT(func_stub, n);
return n + 1;
}
int replacement_func(int n)
{
return n + 100;
}
void example_test(struct kunit *test)
{
KUNIT_EXPECT_EQ(test, real_func(1), 2);
func_stub = replacement_func;
KUNIT_EXPECT_EQ(test, real_func(1), 101);I think we should model activating the stub here in the example to make it a bit clearer.
oops, this DOC was taken almost as-is from my Xe series mentioned in the cover letter, where I didn't have any "activate_fixed_stub" code yet
will fix
}- */
+#define KUNIT_FIXED_STUB_REDIRECT(stub, args...) do { \
typeof(stub) replacement = (stub); \if (kunit_is_running()) { \if (unlikely(replacement)) { \pr_info(KUNIT_SUBTEST_INDENT "# %s: calling stub %ps\n", \__func__, replacement); \return replacement(args); \} \} \+} while (0)
+void __kunit_activate_fixed_stub(struct kunit *test, void *stub_ptr, void *replacement_func);
+/**
- kunit_activate_fixed_stub() - Setup a fixed function stub.
- @test: Test case that wants to activate a fixed function stub
- @stub: The location of the function stub pointer
- @replacement: The replacement function
- This helper setups a function stub with the replacement function.
- It will also automatically restore stub to NULL at the test end.
- */
+#define kunit_activate_fixed_stub(test, stub, replacement) do { \
typecheck_pointer(stub); \typecheck_fn(typeof(stub), replacement); \typeof(stub) *stub_ptr = &(stub); \__kunit_activate_fixed_stub((test), stub_ptr, (replacement)); \+} while (0)
+/**
- kunit_deactivate_fixed_stub() - Disable a fixed function stub.
- @test: Test case that wants to deactivate a fixed function stub (unused for now)
- @stub: The location of the function stub pointer
- */
+#define kunit_deactivate_fixed_stub(test, stub) do { \
typecheck(struct kunit *, (test)); \(stub) = NULL; \+} while (0)
#endif #endif diff --git a/lib/kunit/static_stub.c b/lib/kunit/static_stub.c index 92b2cccd5e76..1b50cf457e89 100644 --- a/lib/kunit/static_stub.c +++ b/lib/kunit/static_stub.c @@ -121,3 +121,24 @@ void __kunit_activate_static_stub(struct kunit *test, } } EXPORT_SYMBOL_GPL(__kunit_activate_static_stub);
+static void nullify_stub_ptr(void *data) +{
void **ptr = data;*ptr = NULL;+}
+/*
- Helper function for kunit_activate_static_stub(). The macro does
- typechecking, so use it instead.
- */
+void __kunit_activate_fixed_stub(struct kunit *test, void *stub_ptr, void *replacement_func) +{
void **stub = stub_ptr;KUNIT_ASSERT_NOT_NULL(test, stub_ptr);Should we check here if the replacement_func is null and if so deactivate the stub similar to the static stubbing?
I had KUNIT_ASSERT_NOT_NULL(test, replacement_func) but decided to drop that to also allow using 'activate_stub(NULL)' to deactivate the stub - but that was before I introduced a separate 'deactivate_stub()'
will bring that back
*stub = replacement_func;I do really appreciate this simplicity. But I wonder if David has any thoughts on the security of direct replacement rather than using the resource API?
note that at redirection point we will not be able to use resource API since that could be run in a non-kunit context
KUNIT_ASSERT_EQ(test, 0, kunit_add_action_or_reset(test, nullify_stub_ptr, stub_ptr));+}
+EXPORT_SYMBOL_GPL(__kunit_activate_fixed_stub);
2.43.0
-- You received this message because you are subscribed to the Google Groups "KUnit Development" group. To unsubscribe from this group and stop receiving emails from it, send an email to kunit-dev+unsubscribe@googlegroups.com. To view this discussion on the web visit https://groups.google.com/d/msgid/kunit-dev/20240821144305.1958-4-michal.waj....