Re: [PATCH v2 0/1] MR4720: Implement test_continue() for amd64

[Alexandre Julliard (＠julliard)]

On Fri Dec 22 18:49:57 2023 +0000, Dāvis Mosāns (davispuh) wrote:
> > [...] consider introducing abstraction or factoring out
>
> architecture-agnostic logic into own function *without* #if(def)s.
> I think that's probably better and quite readable, something like:
> static void invoke_continue_test_func(TEST_CASE test_case, context_pair *contexts)
> {
>     NTSTATUS (*func_ptr)( TEST_CASE test_case, struct context_pair *,
> void *arg2, void *continue_func, void *capture_func ) = code_mem;
>     BYTE call_func[] = continue_test_func; /* arch specific */
>     memcpy( func_ptr, call_func, sizeof(call_func) );
>     FlushInstructionCache( GetCurrentProcess(), func_ptr,
> sizeof(call_func) );
>     if (test_case == TEST_CASE_ALERT)
>     {
>         func_ptr( test_case, &contexts, TRUE, NtContinue,
> pRtlCaptureContext );
>     } else
>     {
>         func_ptr( test_case, &contexts, FALSE, NtContinue,
> pRtlCaptureContext );
>     }
> }
> static void test_continue(void)
> {
>     struct context_pair {
>         CONTEXT before;
>         CONTEXT after;
>     } contexts;
>     if (!pRtlCaptureContext)
>     {
>         win_skip("RtlCaptureContext is not available.\n");
>         return;
>     }
>     invoke_continue_test_func(TEST_CASE_0, &contexts);
>     verify_continue_result(TEST_CASE_0, &contexts);
>     invoke_continue_test_func(TEST_CASE_ALERT, &contexts);
>     verify_continue_result(TEST_CASE_ALERT, &contexts);
>     /* Test some CONTEXT flag magic, on other architectures this could
> be no-op */
>     invoke_continue_test_func(TEST_CASE_2, &contexts);
>     verify_continue_result(TEST_CASE_2, &contexts);
> }

In practice trying to make that sort of code generic adds more complexity than it's worth. Especially as soon as there's some platform-specific quirk that you want to test.