kernelbase: Preserve last error when GetEnvironmentVariableA succeeds.

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>
Signed-off-by: Alexandre Julliard <julliard@winehq.org>

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);

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());

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 );
+    us_value.Length = 0;
-    ret = GetEnvironmentVariableW( us_name.Buffer, valueW, size);
+    us_value.MaximumLength = (size ? size - 1 : 0) * sizeof(WCHAR);
-    if (ret && ret < size) WideCharToMultiByte( CP_ACP, 0, valueW, ret + 1, value, size, NULL, NULL );
+    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;
+    }
 
-    /* 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;
     RtlFreeUnicodeString( &us_name );
     HeapFree( GetProcessHeap(), 0, valueW );
     return ret;