From: Vladimir Panteleev git@vladimir.panteleev.md
Avoid clobbering last error with NO_ERROR when GetEnvironmentVariableA succeeds, matching the behavior of GetEnvironmentVariableW and Windows.
Instead of naively saving and restoring the last error, call RtlQueryEnvironmentVariable_U directly to avoid unnecessarily setting it in the first place.
Signed-off-by: Vladimir Panteleev git@vladimir.panteleev.md Signed-off-by: Gijs Vermeulen gijsvrm@gmail.com --- dlls/advapi32/tests/security.c | 2 ++ dlls/kernel32/tests/environ.c | 8 ++++---- dlls/kernelbase/process.c | 30 +++++++++++++++++++----------- 3 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/dlls/advapi32/tests/security.c b/dlls/advapi32/tests/security.c index c6f5d4690a..825f845190 100644 --- a/dlls/advapi32/tests/security.c +++ b/dlls/advapi32/tests/security.c @@ -5099,6 +5099,7 @@ static void test_GetUserNameA(void) ok(buffer_len == required_len || broken(buffer_len == required_len / sizeof(WCHAR)), /* XP+ */ "Outputted buffer length was %u\n", buffer_len); + ok(GetLastError() == 0xdeadbeef, "Last error was %u\n", GetLastError());
/* Use the reported buffer size from the last GetUserNameA call and pass * a length that is one less than the required value. */ @@ -5168,6 +5169,7 @@ static void test_GetUserNameW(void) ok(ret == TRUE, "GetUserNameW returned %d, last error %u\n", ret, GetLastError()); ok(memcmp(buffer, filler, sizeof(filler)) != 0, "Output buffer was untouched\n"); ok(buffer_len == required_len, "Outputted buffer length was %u\n", buffer_len); + ok(GetLastError() == 0xdeadbeef, "Last error was %u\n", GetLastError());
/* GetUserNameW on XP and newer writes a truncated portion of the username string to the buffer. */ SetLastError(0xdeadbeef); diff --git a/dlls/kernel32/tests/environ.c b/dlls/kernel32/tests/environ.c index 128a5fdbe5..561fc71c4c 100644 --- a/dlls/kernel32/tests/environ.c +++ b/dlls/kernel32/tests/environ.c @@ -155,10 +155,10 @@ static void test_GetSetEnvironmentVariableA(void) "should not fail with empty value but GetLastError=%d\n", GetLastError());
lstrcpyA(buf, "foo"); - SetLastError(0); + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableA(name, buf, lstrlenA(value) + 1); ok(ret_size == 0 && - ((GetLastError() == 0 && lstrcmpA(buf, "") == 0) || + ((GetLastError() == 0xdeadbeef && lstrcmpA(buf, "") == 0) || (GetLastError() == ERROR_ENVVAR_NOT_FOUND)), "%s should be set to "" (NT) or removed (Win9x) but ret_size=%d GetLastError=%d and buf=%s\n", name, ret_size, GetLastError(), buf); @@ -260,10 +260,10 @@ static void test_GetSetEnvironmentVariableW(void) ok(ret == TRUE, "should not fail with empty value but GetLastError=%d\n", GetLastError());
lstrcpyW(buf, fooW); - SetLastError(0); + SetLastError(0xdeadbeef); ret_size = GetEnvironmentVariableW(name, buf, lstrlenW(value) + 1); ok(ret_size == 0 && - ((GetLastError() == 0 && lstrcmpW(buf, empty_strW) == 0) || + ((GetLastError() == 0xdeadbeef && 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()); diff --git a/dlls/kernelbase/process.c b/dlls/kernelbase/process.c index 3369180206..4df5408ea9 100644 --- a/dlls/kernelbase/process.c +++ b/dlls/kernelbase/process.c @@ -1234,24 +1234,32 @@ LPWSTR WINAPI DECLSPEC_HOTPATCH GetEnvironmentStringsW(void) */ DWORD WINAPI DECLSPEC_HOTPATCH GetEnvironmentVariableA( LPCSTR name, LPSTR value, DWORD size ) { - UNICODE_STRING us_name; + UNICODE_STRING us_name, us_value; PWSTR valueW; - DWORD ret; + NTSTATUS status; + DWORD len, ret;
/* limit the size to sane values */ size = min( size, 32767 ); if (!(valueW = HeapAlloc( GetProcessHeap(), 0, size * sizeof(WCHAR) ))) return 0;
RtlCreateUnicodeStringFromAsciiz( &us_name, name ); - SetLastError( 0 ); - ret = GetEnvironmentVariableW( us_name.Buffer, valueW, size); - if (ret && ret < size) WideCharToMultiByte( CP_ACP, 0, valueW, ret + 1, value, size, NULL, NULL ); - - /* this is needed to tell, with 0 as a return value, the difference between: - * - an error (GetLastError() != 0) - * - returning an empty string (in this case, we need to update the buffer) - */ - if (ret == 0 && size && GetLastError() == 0) value[0] = 0; + us_value.Length = 0; + us_value.MaximumLength = (size ? size - 1 : 0) * sizeof(WCHAR); + us_value.Buffer = valueW; + + status = RtlQueryEnvironmentVariable_U( NULL, &us_name, &us_value ); + len = us_value.Length / sizeof(WCHAR); + if (status == STATUS_BUFFER_TOO_SMALL) ret = len + 1; + else if (!set_ntstatus( status )) ret = 0; + else if (!size) ret = len + 1; + else + { + if (len) WideCharToMultiByte( CP_ACP, 0, valueW, len + 1, value, size, NULL, NULL ); + value[len] = 0; + ret = len; + } + RtlFreeUnicodeString( &us_name ); HeapFree( GetProcessHeap(), 0, valueW ); return ret;