[PATCH v2 1/3] kernel32/tests: Fix GetLastError() checks in environment tests.
Lack of SetLastError() calls caused tests to compare stale errors, instead of the effects of the tested (immediately previously called) functions. Add SetLastError calls, and fix a broken test (copy check from A to W tests). Signed-off-by: Vladimir Panteleev <git(a)vladimir.panteleev.md> --- dlls/kernel32/tests/environ.c | 29 ++++++++++++++++++++++++++--- 1 file changed, 26 insertions(+), 3 deletions(-) diff --git a/dlls/kernel32/tests/environ.c b/dlls/kernel32/tests/environ.c index 44a6a0cff0..128a5fdbe5 100644 --- a/dlls/kernel32/tests/environ.c +++ b/dlls/kernel32/tests/environ.c @@ -131,6 +131,7 @@ static void test_GetSetEnvironmentVariableA(void) ok(ret == TRUE, "should erase existing variable\n"); lstrcpyA(buf, "foo"); + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableA(name, buf, lstrlenA(value) + 1); ok(lstrcmpA(buf, "foo") == 0, "should not touch the buffer\n"); ok(ret_size == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND, @@ -163,16 +164,19 @@ static void test_GetSetEnvironmentVariableA(void) name, ret_size, GetLastError(), buf); /* Test the limits */ + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableA(NULL, NULL, 0); ok(ret_size == 0 && (GetLastError() == ERROR_INVALID_PARAMETER || GetLastError() == ERROR_ENVVAR_NOT_FOUND), "should not find variable but ret_size=%d GetLastError=%d\n", ret_size, GetLastError()); + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableA(NULL, buf, lstrlenA(value) + 1); ok(ret_size == 0 && (GetLastError() == ERROR_INVALID_PARAMETER || GetLastError() == ERROR_ENVVAR_NOT_FOUND), "should not find variable but ret_size=%d GetLastError=%d\n", ret_size, GetLastError()); + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableA("", buf, lstrlenA(value) + 1); ok(ret_size == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND, "should not find variable but ret_size=%d GetLastError=%d\n", @@ -233,6 +237,7 @@ static void test_GetSetEnvironmentVariableW(void) ok(ret == TRUE, "should erase existing variable\n"); lstrcpyW(buf, fooW); + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableW(name, buf, lstrlenW(value) + 1); ok(lstrcmpW(buf, fooW) == 0, "should not touch the buffer\n"); ok(ret_size == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND, @@ -255,25 +260,31 @@ static void test_GetSetEnvironmentVariableW(void) ok(ret == TRUE, "should not fail with empty value but GetLastError=%d\n", GetLastError()); lstrcpyW(buf, fooW); + SetLastError(0); ret_size = GetEnvironmentVariableW(name, buf, lstrlenW(value) + 1); - ok(ret_size == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND, - "should not find variable but ret_size=%d GetLastError=%d\n", + ok(ret_size == 0 && + ((GetLastError() == 0 && lstrcmpW(buf, empty_strW) == 0) || + (GetLastError() == ERROR_ENVVAR_NOT_FOUND)), + "should be set to \"\" (NT) or removed (Win9x) but ret_size=%d GetLastError=%d\n", ret_size, GetLastError()); ok(lstrcmpW(buf, empty_strW) == 0, "should copy an empty string\n"); /* Test the limits */ + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableW(NULL, NULL, 0); - ok(ret_size == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND, + ok(ret_size == 0 && (GetLastError() == ERROR_INVALID_PARAMETER || GetLastError() == ERROR_ENVVAR_NOT_FOUND), "should not find variable but ret_size=%d GetLastError=%d\n", ret_size, GetLastError()); if (0) /* Both tests crash on Vista */ { + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableW(NULL, buf, lstrlenW(value) + 1); ok(ret_size == 0 && GetLastError() == ERROR_ENVVAR_NOT_FOUND, "should not find variable but ret_size=%d GetLastError=%d\n", ret_size, GetLastError()); + SetLastError(0xdeadbeef); ret = SetEnvironmentVariableW(NULL, NULL); ok(ret == FALSE && (GetLastError() == ERROR_INVALID_PARAMETER || GetLastError() == ERROR_ENVVAR_NOT_FOUND), "should fail with NULL, NULL but ret=%d and GetLastError=%d\n", @@ -342,6 +353,7 @@ static void test_ExpandEnvironmentStringsA(void) ret_size = ExpandEnvironmentStringsA(buf, buf2, sizeof(buf2)); ok(!strcmp(buf, buf2), "ExpandEnvironmentStrings failed %s vs %s. ret_size = %d\n", buf, buf2, ret_size); + SetLastError(0xdeadbeef); ret_size1 = GetWindowsDirectoryA(buf1,256); ok ((ret_size1 >0) && (ret_size1<256), "GetWindowsDirectory Failed\n"); ret_size = ExpandEnvironmentStringsA("%SystemRoot%",buf,sizeof(buf)); @@ -374,6 +386,7 @@ static void test_GetComputerName(void) int name_len; size = 0; + SetLastError(0xdeadbeef); ret = GetComputerNameA((LPSTR)0xdeadbeef, &size); error = GetLastError(); ok(!ret && error == ERROR_BUFFER_OVERFLOW, "GetComputerNameA should have failed with ERROR_BUFFER_OVERFLOW instead of %d\n", error); @@ -433,6 +446,7 @@ static void test_GetComputerNameExA(void) } size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExA(ComputerNameDnsDomain, (LPSTR)0xdeadbeef, &size); error = GetLastError(); ok(ret == 0, "Expected 0, got %d\n", ret); @@ -452,6 +466,7 @@ static void test_GetComputerNameExA(void) HeapFree(GetProcessHeap(), 0, name); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExA(ComputerNameDnsFullyQualified, (LPSTR)0xdeadbeef, &size); error = GetLastError(); ok(ret == 0, "Expected 0, got %d\n", ret); @@ -468,6 +483,7 @@ static void test_GetComputerNameExA(void) HeapFree(GetProcessHeap(), 0, name); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExA(ComputerNameDnsHostname, (LPSTR)0xdeadbeef, &size); error = GetLastError(); ok(ret == 0, "Expected 0, got %d\n", ret); @@ -484,12 +500,14 @@ static void test_GetComputerNameExA(void) HeapFree(GetProcessHeap(), 0, name); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExA(ComputerNameNetBIOS, (LPSTR)0xdeadbeef, &size); error = GetLastError(); ok(ret == 0, "Expected 0, got %d\n", ret); ok(error == ERROR_MORE_DATA, "Expected ERROR_MORE_DATA, got %d\n", error); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExA(ComputerNameNetBIOS, NULL, &size); error = GetLastError(); ok(ret == 0, "Expected 0, got %d\n", ret); @@ -520,6 +538,7 @@ static void test_GetComputerNameExW(void) } size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExW(ComputerNameDnsDomain, (LPWSTR)0xdeadbeef, &size); error = GetLastError(); ok(!ret && error == ERROR_MORE_DATA, "GetComputerNameExW should have failed with ERROR_MORE_DATA instead of %d\n", error); @@ -530,6 +549,7 @@ static void test_GetComputerNameExW(void) HeapFree(GetProcessHeap(), 0, nameW); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExW(ComputerNameDnsFullyQualified, (LPWSTR)0xdeadbeef, &size); error = GetLastError(); ok(!ret && error == ERROR_MORE_DATA, "GetComputerNameExW should have failed with ERROR_MORE_DATA instead of %d\n", error); @@ -540,6 +560,7 @@ static void test_GetComputerNameExW(void) HeapFree(GetProcessHeap(), 0, nameW); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExW(ComputerNameDnsHostname, (LPWSTR)0xdeadbeef, &size); error = GetLastError(); ok(!ret && error == ERROR_MORE_DATA, "GetComputerNameExW should have failed with ERROR_MORE_DATA instead of %d\n", error); @@ -550,6 +571,7 @@ static void test_GetComputerNameExW(void) HeapFree(GetProcessHeap(), 0, nameW); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExW(ComputerNameNetBIOS, (LPWSTR)0xdeadbeef, &size); error = GetLastError(); ok(!ret && error == ERROR_MORE_DATA, "GetComputerNameExW should have failed with ERROR_MORE_DATA instead of %d\n", error); @@ -560,6 +582,7 @@ static void test_GetComputerNameExW(void) HeapFree(GetProcessHeap(), 0, nameW); size = 0; + SetLastError(0xdeadbeef); ret = pGetComputerNameExW(ComputerNameNetBIOS, NULL, &size); error = GetLastError(); ok(!ret && error == ERROR_MORE_DATA, "GetComputerNameExW should have failed with ERROR_MORE_DATA instead of %d\n", error); -- 2.25.0
Windows doesn't do this (except XP, and then only for empty variables). Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48471 Signed-off-by: Vladimir Panteleev <git(a)vladimir.panteleev.md> --- dlls/kernel32/tests/environ.c | 3 +++ dlls/kernelbase/process.c | 3 ++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/environ.c b/dlls/kernel32/tests/environ.c index 128a5fdbe5..7eaafc30fd 100644 --- a/dlls/kernel32/tests/environ.c +++ b/dlls/kernel32/tests/environ.c @@ -206,10 +206,13 @@ static void test_GetSetEnvironmentVariableW(void) GetLastError()); /* Try to retrieve the environment variable we just set */ + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableW(name, NULL, 0); ok(ret_size == lstrlenW(value) + 1, "should return length with terminating 0 ret_size=%d\n", ret_size); + ok(GetLastError() == 0xdeadbeef, + "should not fail with zero size but GetLastError=%d\n", GetLastError()); lstrcpyW(buf, fooW); ret_size = GetEnvironmentVariableW(name, buf, lstrlenW(value)); 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; } -- 2.25.0
Do the same thing as Vista and up. Wine-Bug: https://bugs.winehq.org/show_bug.cgi?id=48471 Signed-off-by: Vladimir Panteleev <git(a)vladimir.panteleev.md> --- dlls/kernel32/tests/environ.c | 10 ++++++++++ dlls/kernelbase/process.c | 3 ++- 2 files changed, 12 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/tests/environ.c b/dlls/kernel32/tests/environ.c index 7eaafc30fd..ca01167d89 100644 --- a/dlls/kernel32/tests/environ.c +++ b/dlls/kernel32/tests/environ.c @@ -272,6 +272,16 @@ static void test_GetSetEnvironmentVariableW(void) ret_size, GetLastError()); ok(lstrcmpW(buf, empty_strW) == 0, "should copy an empty string\n"); + SetLastError(0xdeadbeef); + ret_size = GetEnvironmentVariableW(name, NULL, 0); + ok(ret_size == 1 || + broken(ret_size == 0), /* XP */ + "should return 1 for empty string but ret_size=%d GetLastError=%d\n", + ret_size, GetLastError()); + ok(GetLastError() == 0xdeadbeef || + broken(GetLastError() == ERROR_MORE_DATA), /* XP */ + "should not fail with zero size but GetLastError=%d\n", GetLastError()); + /* Test the limits */ SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableW(NULL, NULL, 0); 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; } -- 2.25.0
Vladimir Panteleev <git(a)vladimir.panteleev.md> writes:
@@ -255,25 +260,31 @@ static void test_GetSetEnvironmentVariableW(void) ok(ret == TRUE, "should not fail with empty value but GetLastError=%d\n", GetLastError());
lstrcpyW(buf, fooW); + SetLastError(0);
You should probably use 0xdeadbeef here (and also in the A test that this was copied from). -- Alexandre Julliard julliard(a)winehq.org
participants (2)
-
Alexandre Julliard -
Vladimir Panteleev