[PATCH v4 2/7] kernel32: Handle GEOCLASS_REGION in Get/SetUserGeoID.

[João Diogo Ferreira]

Signed-off-by: João Diogo Craveiro Ferreira 
---
Supersedes: 172368
V4: Include documentation.
V3: Fix build error.
V2: Introduce trace logs.
---
 dlls/kernel32/locale.c | 66 ++++++++++++++++++++++++++++++++----------
 1 file changed, 50 insertions(+), 16 deletions(-)

diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c
index 884709730c..cee501278a 100644
--- a/dlls/kernel32/locale.c
+++ b/dlls/kernel32/locale.c
@@ -3996,12 +3996,22 @@ static const struct geoinfo_t *get_geoinfo_dataptr(GEOID geoid)

/******************************************************************************
  *           GetUserGeoID (KERNEL32.@)
+ *
+ * Retrieves the ID of the user's geographic nation or region.
+ *
+ * PARAMS
+ *   GeoClass [I] One of GEOCLASS_NATION or GEOCLASS_REGION (SYSGEOCLASS enum from "winnls.h").
+ *
+ * RETURNS
+ *   SUCCESS: The ID of the specified geographic class.
+ *   FAILURE: GEOID_NOT_AVAILABLE.
  */
 GEOID WINAPI GetUserGeoID(GEOCLASS geoclass)
 {
     GEOID ret = GEOID_NOT_AVAILABLE;
     static const WCHAR geoW[] = {'G','e','o',0};
     static const WCHAR nationW[] = {'N','a','t','i','o','n',0};
+    static const WCHAR regionW[] = {'R','e','g','i','o','n',0};
     WCHAR bufferW[40], *end;
     HANDLE hkey, hsubkey = 0;
     UNICODE_STRING keyW;
@@ -4009,20 +4019,29 @@ GEOID WINAPI GetUserGeoID(GEOCLASS geoclass)
     DWORD count = sizeof(bufferW);
     RtlInitUnicodeString(&keyW, nationW);

+    if (geoclass == GEOCLASS_NATION)
+    {
+        TRACE("(GEOCLASS_NATION)\n");
+        RtlInitUnicodeString(&keyW, nationW);
+    }
+    else if (geoclass == GEOCLASS_REGION)
+    {
+        TRACE("(GEOCLASS_REGION)\n");
+        RtlInitUnicodeString(&keyW, regionW);
+    }
+    else
+    {
+        WARN("Unknown geoclass %d\n", geoclass);
+        return ret;
+    }
+
     if (!(hkey = create_registry_key())) return ret;

-    switch (geoclass) {
-    case GEOCLASS_NATION:
-        if ((hsubkey = NLS_RegOpenKey(hkey, geoW)))
-        {
-            if ((NtQueryValueKey(hsubkey, &keyW, KeyValuePartialInformation,
-                                 bufferW, count, &count) == STATUS_SUCCESS ) && info->DataLength)
-                ret = strtolW((const WCHAR*)info->Data, &end, 10);
-        }
-        break;
-    case GEOCLASS_REGION:
-        FIXME("GEOCLASS_REGION not handled yet\n");
-        break;
+    if ((hsubkey = NLS_RegOpenKey(hkey, geoW)))
+    {
+        if((NtQueryValueKey(hsubkey, &keyW, KeyValuePartialInformation,
+                            bufferW, count, &count) == STATUS_SUCCESS ) && info->DataLength)
+            ret = strtolW((const WCHAR*)info->Data, &end, 10);
     }

NtClose(hkey);
@@ -4035,15 +4054,26 @@ GEOID WINAPI GetUserGeoID(GEOCLASS geoclass)
  */
 BOOL WINAPI SetUserGeoID(GEOID geoid)
 {
+    const struct geoinfo_t *geoinfo = get_geoinfo_dataptr(geoid);
     static const WCHAR geoW[] = {'G','e','o',0};
     static const WCHAR nationW[] = {'N','a','t','i','o','n',0};
+    static const WCHAR regionW[] = {'R','e','g','i','o','n',0};
     static const WCHAR formatW[] = {'%','i',0};
     UNICODE_STRING nameW, keyW;
     WCHAR bufferW[10];
     OBJECT_ATTRIBUTES attr;
     HANDLE hkey;

-    if (!(hkey = create_registry_key())) return FALSE;
+    if (!geoinfo)
+    {
+        SetLastError(ERROR_INVALID_PARAMETER);
+        return FALSE;
+    }
+    if (!(hkey = create_registry_key()))
+    {
+        SetLastError(ERROR_INTERNAL_ERROR);
+        return FALSE;
+    }

attr.Length = sizeof(attr);
     attr.RootDirectory = hkey;
@@ -4052,16 +4082,20 @@ BOOL WINAPI SetUserGeoID(GEOID geoid)
     attr.SecurityDescriptor = NULL;
     attr.SecurityQualityOfService = NULL;
     RtlInitUnicodeString(&nameW, geoW);
-    RtlInitUnicodeString(&keyW, nationW);

-    if (NtCreateKey(&hkey, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL) != STATUS_SUCCESS)
+    if (geoinfo->kind == LOCATION_NATION)
+        RtlInitUnicodeString(&keyW, nationW);
+    else
+        RtlInitUnicodeString(&keyW, regionW);

+    if (NtCreateKey(&hkey, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL) != STATUS_SUCCESS)
     {
         NtClose(attr.RootDirectory);
+        SetLastError(ERROR_INTERNAL_ERROR);
         return FALSE;
     }

-    sprintfW(bufferW, formatW, geoid);
+    sprintfW(bufferW, formatW, geoinfo->id);
     NtSetValueKey(hkey, &keyW, 0, REG_SZ, bufferW, (strlenW(bufferW) + 1) * sizeof(WCHAR));
     NtClose(attr.RootDirectory);
     NtClose(hkey);

[Marvin]

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=58782

Your paranoid android.

=== debian10 (32 bit report) ===

kernel32:
debugger.c:320: Test failed: GetThreadContext failed: 5

=== debian10 (32 bit Chinese:China report) ===

kernel32:
debugger: Timeout

=== debian10 (32 bit WoW report) ===

kernel32:
debugger: Timeout

[Alexandre Julliard]

João Diogo Ferreira  writes:
> - if (!(hkey = create_registry_key())) return FALSE;
> - if (!geoinfo)
> - {
> -
>    SetLastError(ERROR_INVALID_PARAMETER);
> -
>    return FALSE;
> - }
> - if (!(hkey = create_registry_key()))
> - {
> -
>    SetLastError(ERROR_INTERNAL_ERROR);
> -
>    return FALSE;
> - }

It's not useful to set last error, especially not to a meaningless value
like ERROR_INTERNAL_ERROR, unless there are test cases demonstrating
that Windows does the same thing.
> - if (NtCreateKey(&hkey, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL) != STATUS_SUCCESS)
> {
>     NtClose(attr.RootDirectory);
> -
>    SetLastError(ERROR_INTERNAL_ERROR);

Same here.

[João Diogo Ferreira]

Got it. I suppose the same is true of an unverified
ERROR_BADDB in patch #4, as well as an improvised
ERROR_INSUFFICIENT_MEMORY in that same patch.

I'll fix all these ASAP. Thanks for reviewing.

________________________________
From: Alexandre Julliard 
Sent: Tuesday, November 5, 2019 4:26:28 PM
To: João Diogo Ferreira 
Cc: [e-mail] 
Subject: Re: [PATCH v4 2/7] kernel32: Handle GEOCLASS_REGION in Get/SetUserGeoID.

João Diogo Ferreira  writes:
> - if (!(hkey = create_registry_key())) return FALSE;
> - if (!geoinfo)
> - {
> -
>    SetLastError(ERROR_INVALID_PARAMETER);
> -
>    return FALSE;
> - }
> - if (!(hkey = create_registry_key()))
> - {
> -
>    SetLastError(ERROR_INTERNAL_ERROR);
> -
>    return FALSE;
> - }

It's not useful to set last error, especially not to a meaningless value
like ERROR_INTERNAL_ERROR, unless there are test cases demonstrating
that Windows does the same thing.
> - if (NtCreateKey(&hkey, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL) != STATUS_SUCCESS)
> {
>     NtClose(attr.RootDirectory);
> -
>    SetLastError(ERROR_INTERNAL_ERROR);

Same here.

--
Alexandre Julliard
[e-mail]
________________________________
From: Alexandre Julliard 
Sent: Tuesday, November 5, 2019 4:26:28 PM
To: João Diogo Ferreira 
Cc: [e-mail] 
Subject: Re: [PATCH v4 2/7] kernel32: Handle GEOCLASS_REGION in Get/SetUserGeoID.

João Diogo Ferreira  writes:
> - if (!(hkey = create_registry_key())) return FALSE;
> - if (!geoinfo)
> - {
> -
>    SetLastError(ERROR_INVALID_PARAMETER);
> -
>    return FALSE;
> - }
> - if (!(hkey = create_registry_key()))
> - {
> -
>    SetLastError(ERROR_INTERNAL_ERROR);
> -
>    return FALSE;
> - }

It's not useful to set last error, especially not to a meaningless value
like ERROR_INTERNAL_ERROR, unless there are test cases demonstrating
that Windows does the same thing.
> - if (NtCreateKey(&hkey, KEY_ALL_ACCESS, &attr, 0, NULL, 0, NULL) != STATUS_SUCCESS)
> {
>     NtClose(attr.RootDirectory);
> -
>    SetLastError(ERROR_INTERNAL_ERROR);

Same here.

--
Alexandre Julliard
[e-mail]