[PATCH 1/3] kernel32/tests/fiber: Add tests for fiber-local storage.

[André Hentschel]

Am 02.07.2016 um 04:41 schrieb John Sheu:
> Signed-off-by: John Sheu <sheu at google.com>

Hi and welcome to Wine development!
May I ask if you could name an application which will benefit from your fixes?
Your tests currently fail on our testbot as you can see here: https://source.winehq.org/patches/
Some inline comments:

>  static VOID WINAPI FiberMainProc(LPVOID lpFiberParameter)
>  {
>      BYTE *tparam = (BYTE *)lpFiberParameter;
> -    cbCount++;
> +    fiberCount++;
>      ok(*tparam == 185, "Parameterdata expected not to be changed\n");
> +    if (fls_index_to_set != FLS_OUT_OF_INDEXES)
> +    {
> +        if (!pFlsSetValue || !pFlsGetValue)
> +        {
> +            win_skip( "Fiber Local Storage not supported\n" );
> +        }
> +        else
> +        {
> +            PVOID pret;
> +            BOOL bret;
> +
> +            pret = FlsGetValue(fls_index_to_set);
> +            ok(pret == NULL, "FlsGetValue returned %p, expected NULL\n", pret);
> +
> +            /* Set the FLS value */
> +            SetLastError( 0xdeadbeef );
> +            bret = pFlsSetValue(fls_index_to_set, fls_value_to_set);
> +            ok(bret, "FlsSetValue failed with error %u\n", GetLastError());
> +
> +            /* Verify that FlsGetValue retrieves the value set by FlsSetValue */
> +            SetLastError( 0xdeadbeef );
> +            pret = FlsGetValue(fls_index_to_set);
> +            ok(pret == fls_value_to_set,
> +               "FlsGetValue returned %p, expected %p\n", pret, fls_value_to_set);
> +            ok(GetLastError() == ERROR_SUCCESS,
> +	       "FlsGetValue error %u\n", GetLastError());
> +        }
> +    }
>      pSwitchToFiber(fibers[0]);
>  }

You are mixing tabs and spaces in FiberMainProc, spaces are preferred. Further you can avoid some line brakes in ok() calls, only wrap at around 100 

> +static void test_FiberLocalStorageWithFibers(PFLS_CALLBACK_FUNCTION cbfunc) {
> +    PVOID val1 = (PVOID) 0x314;
> +    PVOID val2 = (PVOID) 0x152;
> +    BOOL ret;
> +
> +    if (!pConvertThreadToFiber || !pSwitchToFiber || !pDeleteFiber)
> +    {
> +        win_skip( "Fibers not supported\n" );
> +        return;
> +    }
> +    if (!pFlsAlloc || !pFlsFree || !pFlsSetValue || !pFlsGetValue)
> +    {
> +        win_skip( "Fiber Local Storage not supported\n" );
> +        return;
> +    }
> +
> +
> +    fls_index_to_set = pFlsAlloc(cbfunc);
> +    ok(fls_index_to_set != FLS_OUT_OF_INDEXES, "FlsAlloc failed with error %d\n", GetLastError());
> +
> +    fiberCount = 0;
> +    cbCount = 0;
> +    fibers[0] = pConvertThreadToFiber(&testparam);

Why do you convert the current thread into a fiber? It seems to me that is causing the crash on w2k3.
You could try to run that test in a seperate thread

> +    fibers[1] = pCreateFiber(0,FiberMainProc,&testparam);
> +    fibers[2] = pCreateFiber(0,FiberMainProc,&testparam);
> +    ok(fibers[1] != 0, "CreateFiber failed with error %d\n", GetLastError());
> +    ok(fibers[2] != 0, "CreateFiber failed with error %d\n", GetLastError());
> +    ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount);
> +    ok(cbCount == 0, "Wrong callback count: %d\n", cbCount);
> +
> +    fiberCount = 0;
> +    cbCount = 0;
> +    fls_value_to_set = val1;
> +    pSwitchToFiber(fibers[1]);
> +    ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount);
> +    ok(cbCount == 0, "Wrong callback count: %d\n", cbCount);
> +
> +    fiberCount = 0;
> +    cbCount = 0;
> +    fls_value_to_set = val2;
> +    pSwitchToFiber(fibers[2]);
> +    ok(fiberCount == 1, "Wrong fiber count: %d\n", fiberCount);
> +    ok(cbCount == 0, "Wrong callback count: %d\n", cbCount);
> +
> +    fls_value_to_set = val2;
> +    ret = pFlsSetValue(fls_index_to_set, fls_value_to_set);
> +    ok(ret, "FlsSetValue failed\n");
> +    ok(val2 == pFlsGetValue(fls_index_to_set), "FlsGetValue failed\n");
> +
> +    fiberCount = 0;
> +    cbCount = 0;
> +    fls_value_to_set = val1;
> +    pDeleteFiber(fibers[1]);
> +    ok(fiberCount == 0, "Wrong fiber count: %d\n", fiberCount);
> +    todo_wine
> +    {
> +        ok(cbCount == 1, "Wrong callback count: %d\n", cbCount);
> +    }

You can make that a one liner:
todo_wine ok();




Hope that helps
Have fun!