[PATCH v4 2/7] kernel32: Handle GEOCLASS_REGION in Get/SetUserGeoID.
Signed-off-by: João Diogo Craveiro Ferreira <devilj(a)outlook.pt> --- 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); -- 2.23.0
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
João Diogo Ferreira <devilj(a)outlook.pt> 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 julliard(a)winehq.org
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 <julliard(a)winehq.org> Sent: Tuesday, November 5, 2019 4:26:28 PM To: João Diogo Ferreira <devilj(a)outlook.pt> Cc: wine-devel(a)winehq.org <wine-devel(a)winehq.org> Subject: Re: [PATCH v4 2/7] kernel32: Handle GEOCLASS_REGION in Get/SetUserGeoID. João Diogo Ferreira <devilj(a)outlook.pt> 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 julliard(a)winehq.org ________________________________ From: Alexandre Julliard <julliard(a)winehq.org> Sent: Tuesday, November 5, 2019 4:26:28 PM To: João Diogo Ferreira <devilj(a)outlook.pt> Cc: wine-devel(a)winehq.org <wine-devel(a)winehq.org> Subject: Re: [PATCH v4 2/7] kernel32: Handle GEOCLASS_REGION in Get/SetUserGeoID. João Diogo Ferreira <devilj(a)outlook.pt> 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 julliard(a)winehq.org
participants (3)
-
Alexandre Julliard -
João Diogo Ferreira -
Marvin