From: Vladimir Panteleev <git(a)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(a)vladimir.panteleev.md>
Signed-off-by: Gijs Vermeulen <gijsvrm(a)gmail.com>
---
dlls/kernel32/tests/environ.c | 8 ++++----
dlls/kernelbase/process.c | 30 +++++++++++++++++++-----------
2 files changed, 23 insertions(+), 15 deletions(-)
diff --git a/dlls/kernel32/tests/environ.c b/dlls/kernel32/tests/environ.c
index 26f24caad2..41d4095772 100644
--- a/dlls/kernel32/tests/environ.c
+++ b/dlls/kernel32/tests/environ.c
@@ -157,10 +157,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);
@@ -262,10 +262,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 c426978114..b06706f11b 100644
--- a/dlls/kernelbase/process.c
+++ b/dlls/kernelbase/process.c
@@ -1345,24 +1345,32 @@ BOOL WINAPI DECLSPEC_HOTPATCH SetEnvironmentStringsW( WCHAR *env )
*/
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;
--
2.26.2