From: Zhiyi Zhang <zzhang@codeweavers.com>
Subject: Re: [PATCH 1/2] kernelbase: Don't return ERROR_INSUFFICIENT_BUFFER from GetEnvironmentVariableW.
Message-Id: <8bd11304-59d3-abc3-604f-11e84ceef2dc@codeweavers.com>
Date: Thu, 16 Jan 2020 16:43:26 +0800
In-Reply-To: <20200116083242.688444-1-git@vladimir.panteleev.md>
References: <20200116083242.688444-1-git@vladimir.panteleev.md>



On 1/16/20 4:32 PM, Vladimir Panteleev wrote:
> Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48471
> Signed-off-by: Vladimir Panteleev <git@vladimir.panteleev.md>
> ---
>  dlls/kernelbase/process.c         |  3 +-
>  dlls/kernelbase/tests/Makefile.in |  1 +
>  dlls/kernelbase/tests/process.c   | 52 +++++++++++++++++++++++++++++++
>  3 files changed, 55 insertions(+), 1 deletion(-)
>  create mode 100644 dlls/kernelbase/tests/process.c
>
> diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c
> index a07dddb1fc..788f03f220 100644
> --- a/dlls/kernelbase/process.c
> +++ b/dlls/kernelbase/process.c
> @@ -1274,7 +1274,8 @@ DWORD WINAPI DECLSPEC_HOTPATCH GetEnvironmentVariableW( LPCWSTR name, LPWSTR val
>  
>      status = RtlQueryEnvironmentVariable_U( NULL, &us_name, &us_value );
>      len = us_value.Length / sizeof(WCHAR);
> -    if (!set_ntstatus( status )) return (status == STATUS_BUFFER_TOO_SMALL) ? len + 1 : 0;
> +    if (status == STATUS_BUFFER_TOO_SMALL) return len + 1;
> +    if (!set_ntstatus( status )) return 0;
>      if (size) val[len] = 0;
>      return len;
>  }
> diff --git a/dlls/kernelbase/tests/Makefile.in b/dlls/kernelbase/tests/Makefile.in
> index 22e4a17a58..4c90809012 100644
> --- a/dlls/kernelbase/tests/Makefile.in
> +++ b/dlls/kernelbase/tests/Makefile.in
> @@ -2,4 +2,5 @@ TESTDLL   = kernelbase.dll
>  
>  C_SRCS = \
>  	path.c \
> +	process.c \
>  	sync.c
> diff --git a/dlls/kernelbase/tests/process.c b/dlls/kernelbase/tests/process.c
> new file mode 100644
> index 0000000000..24ab2c4b71
> --- /dev/null
> +++ b/dlls/kernelbase/tests/process.c
> @@ -0,0 +1,52 @@
> +/*
> + * Process tests for kernelbase.dll
> + *
> + * Copyright 2020 Vladimir Panteleev
> + *
> + * 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"
> +
> +static DWORD (WINAPI *pGetEnvironmentVariableW)(LPCWSTR, LPWSTR, DWORD);
> +static BOOL (WINAPI *pSetEnvironmentVariableW)(LPCWSTR, LPCWSTR);
> +
> +static void test_GetEnvironmentVariableW(void)
> +{
> +    DWORD gle;
> +
> +    pSetEnvironmentVariableW(L"TESTVAR", L"abc");
> +    SetLastError(0xdeadbeef);
> +    pGetEnvironmentVariableW(L"TESTVAR", NULL, 0);
> +    gle = GetLastError();
> +    ok(gle == 0xdeadbeef, "got %d\n", gle);
> +}
> +
> +START_TEST(process)
> +{
> +    HMODULE hmod;
> +
> +    hmod = LoadLibraryA("kernelbase.dll");
> +    pGetEnvironmentVariableW = (void *)GetProcAddress(hmod, "GetEnvironmentVariableW");
> +    pSetEnvironmentVariableW = (void *)GetProcAddress(hmod, "SetEnvironmentVariableW");
> +
> +    test_GetEnvironmentVariableW();
> +}
Wine already has tests for SetEnvironmentVariableW in dlls/kernel32/tests/environ.c.
I think you should add new tests there instead of creating a new one in kernelbase.