Re: [PATCH v2 2/2] kernelbase/tests: Add test for GetSystemFirmwareTable regression

[Zhiyi Zhang]

On 11/20/19 3:07 PM, John Chadwick wrote:
> This adds a test to ensure that a typical usage of
> GetSystemFirmwareTable with a preflight call works correctly.
> Signed-off-by: John Chadwick
>
> dlls/kernelbase/tests/Makefile.in |  1 +
>  dlls/kernelbase/tests/memory.c    | 62 +++++++++++++++++++++++++++++++
>  2 files changed, 63 insertions(+)
>  create mode 100644 dlls/kernelbase/tests/memory.c
>
> diff --git a/dlls/kernelbase/tests/Makefile.in b/dlls/kernelbase/tests/Makefile.in
> index 22e4a17a58..f269147733 100644
> --- a/dlls/kernelbase/tests/Makefile.in
> +++ b/dlls/kernelbase/tests/Makefile.in
> @@ -1,5 +1,6 @@
>  TESTDLL   = kernelbase.dll
>
> C_SRCS = \
> - memory.c \
> path.c \
> sync.c
>
> diff --git a/dlls/kernelbase/tests/memory.c b/dlls/kernelbase/tests/memory.c
> new file mode 100644
> index 0000000000..f68077678a
> --- /dev/null
> +++ b/dlls/kernelbase/tests/memory.c
> @@ -0,0 +1,62 @@
> +/*
> -
> - Memory tests for kernelbase.dll
> -
> -
> -
> - Copyright 2019 John Chadwick
> -
> -
> -
> - This library is free software; you can redistribute it and/or
> -
> - modify it under the terms of the GNU Lesser General Public
> -
> - License as published by the Free Software Foundation; either
> -
> - version 2.1 of the License, or (at your option) any later version.
> -
> -
> -
> - This library is distributed in the hope that it will be useful,
> -
> - but WITHOUT ANY WARRANTY; without even the implied warranty of
> -
> - MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
> -
> - Lesser General Public License for more details.
> -
> -
> -
> - You should have received a copy of the GNU Lesser General Public
> -
> - License along with this library; if not, write to the Free Software
> -
> - Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301, USA
> - */
> -
>
> +#include <stdarg.h>
> +#include <windef.h>
> +#include <winbase.h>
> +#include <stdlib.h>
> +#include <winerror.h>
> -
>
> +#include "wine/test.h"
> -
>
> +#define RSMB 0x52534D42
> -
>
> +static UINT (WINAPI *pGetSystemFirmwareTable)(DWORD, DWORD, void *, DWORD);
> -
>
> +static void test_GetSystemFirmwareTable(void)
> +{
> - DWORD size, bytes_returned;
> - void *data;
> -
> - if (!pGetSystemFirmwareTable)
> - {
> -
>    win_skip("GetSystemFirmwareTable not available\n");
> -
>    return;
> - }
> -
> - size = pGetSystemFirmwareTable(RSMB, 0, NULL, 0);
> - ok(size > 0, "got %d", size);
> -
> - data = HeapAlloc(GetProcessHeap(), 0, size);
> - ok(data != NULL, "alloc failed");

Hi John,

kernel32/tests/version.c already has test_GetSystemFirmwareTable().
So I think you should add the tests there. And it would be better to add the tests
first to demonstrate the breakage (adding a todo_wine) and a fix after that to remove
todo_wine.

Also data is leaked.

Thanks,
Zhiyi
> -
> - bytes_returned = pGetSystemFirmwareTable(RSMB, 0, data, size);
> - ok(bytes_returned == size, "expect %d, got %d\n", size, bytes_returned);
>
> +}
> -
>
> +START_TEST(memory)
> +{
> - HMODULE hmod;
> -
> - hmod = LoadLibraryA("kernelbase.dll");
> - pGetSystemFirmwareTable = (void *)GetProcAddress(hmod, "GetSystemFirmwareTable");
> -
> - test_GetSystemFirmwareTable();
>
> +}