[PATCH 1/2] kernelbase: Don't return ERROR_INSUFFICIENT_BUFFER from GetEnvironmentVariableW.
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48471 Signed-off-by: Vladimir Panteleev <git(a)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(); +} -- 2.25.0
Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48471 Signed-off-by: Vladimir Panteleev <git(a)vladimir.panteleev.md> --- dlls/kernelbase/process.c | 3 ++- dlls/kernelbase/tests/process.c | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 788f03f220..612b0b2a2f 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -1276,7 +1276,8 @@ DWORD WINAPI DECLSPEC_HOTPATCH GetEnvironmentVariableW( LPCWSTR name, LPWSTR val len = us_value.Length / sizeof(WCHAR); if (status == STATUS_BUFFER_TOO_SMALL) return len + 1; if (!set_ntstatus( status )) return 0; - if (size) val[len] = 0; + if (!size) return len + 1; + val[len] = 0; return len; } diff --git a/dlls/kernelbase/tests/process.c b/dlls/kernelbase/tests/process.c index 24ab2c4b71..55d96d8659 100644 --- a/dlls/kernelbase/tests/process.c +++ b/dlls/kernelbase/tests/process.c @@ -31,13 +31,17 @@ static BOOL (WINAPI *pSetEnvironmentVariableW)(LPCWSTR, LPCWSTR); static void test_GetEnvironmentVariableW(void) { - DWORD gle; + DWORD gle, ret; pSetEnvironmentVariableW(L"TESTVAR", L"abc"); SetLastError(0xdeadbeef); pGetEnvironmentVariableW(L"TESTVAR", NULL, 0); gle = GetLastError(); ok(gle == 0xdeadbeef, "got %d\n", gle); + + pSetEnvironmentVariableW(L"TESTVAR", L""); + ret = pGetEnvironmentVariableW(L"TESTVAR", NULL, 0); + ok(ret == 1, "got %d\n", ret); } START_TEST(process) -- 2.25.0
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=63286 Your paranoid android. === wxppro (32 bit report) === kernelbase: 09dc:process: unhandled exception c0000005 at 00000000 === w2003std (32 bit report) === kernelbase: 01e8:process: unhandled exception c0000005 at 00000000 === wvistau64 (32 bit report) === kernelbase: 0284:process: unhandled exception c0000005 at 00000000 === w2008s64 (32 bit report) === kernelbase: 0510:process: unhandled exception c0000005 at 00000000 === wvistau64 (64 bit report) === kernelbase: 0914:process: unhandled exception c0000005 at 0000000000000000 === w2008s64 (64 bit report) === kernelbase: 0390:process: unhandled exception c0000005 at 0000000000000000
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(a)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.
On Thu, 16 Jan 2020 at 08:43, Zhiyi Zhang <zzhang(a)codeweavers.com> wrote:
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.
Good idea, thanks. It looks like there is another problem. Wine's current behavior for the thing I'm trying to fix is "correct" in that it correctly emulates Windows versions before 7. However, the affected Windows program expects the W7+ behavior, and doesn't support older Windows versions anyway. What can be done about this? if (...GetVersion()...) ...old behavior... else ...w7+ behavior...? Or should it be changed to just do what W7 does?
On 1/16/20 10:26 PM, Vladimir Panteleev wrote:
On Thu, 16 Jan 2020 at 08:43, Zhiyi Zhang <zzhang(a)codeweavers.com> wrote:
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.
Good idea, thanks.
It looks like there is another problem. Wine's current behavior for the thing I'm trying to fix is "correct" in that it correctly emulates Windows versions before 7. However, the affected Windows program expects the W7+ behavior, and doesn't support older Windows versions anyway. What can be done about this? if (...GetVersion()...) ...old behavior... else ...w7+ behavior...? Or should it be changed to just do what W7 does?
As far as I'm aware it's generally fine to change Wine to (unconditionally) emulate newer Windows behaviour rather than older if a program depends on it, especially if the older behaviour is obviously "broken" in some manner (e.g. contradicts the documentation or is otherwise insane.)
Hi, While running your changed tests, I think I found new failures. Being a bot and all I'm not very good at pattern recognition, so I might be wrong, but could you please double-check? Full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=63285 Your paranoid android. === wxppro (32 bit report) === kernelbase: 09dc:process: unhandled exception c0000005 at 00000000 === w2003std (32 bit report) === kernelbase: 01e8:process: unhandled exception c0000005 at 00000000 === wvistau64 (32 bit report) === kernelbase: 0914:process: unhandled exception c0000005 at 00000000 === w2008s64 (32 bit report) === kernelbase: 0a74:process: unhandled exception c0000005 at 00000000 === wvistau64 (64 bit report) === kernelbase: 0914:process: unhandled exception c0000005 at 0000000000000000 === w2008s64 (64 bit report) === kernelbase: 0390:process: unhandled exception c0000005 at 0000000000000000
participants (4)
-
Marvin -
Vladimir Panteleev -
Zebediah Figura -
Zhiyi Zhang