[PATCH 1/3] kernel32: Fix null pointer access in GetGeoInfoW().
We were failing the conditional that would point «str» to «buffW» when retrieving the GEO_ISO_UN_NUMBER of a location with UN code 0. Some locations will keep having no UN code, such as Guantanamo Bay or Johnston Atoll, since that's how those are defined in Windows. Signed-off-by: João Diogo Craveiro Ferreira <devilj(a)outlook.pt> --- dlls/kernel32/locale.c | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index 0f91728e84..086389b45f 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -4109,10 +4109,10 @@ BOOL WINAPI SetUserGeoID(GEOID geoid) INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, LANGID lang) { const struct geoinfo_t *ptr; - const WCHAR *str = NULL; WCHAR buffW[12]; - LONG val = 0; - INT len; + const WCHAR *str = buffW; + int len; + static const WCHAR fmtW[] = {'%','d',0}; TRACE("%d %d %p %d %d\n", geoid, geotype, data, data_len, lang); @@ -4123,20 +4123,20 @@ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, switch (geotype) { case GEO_NATION: - val = geoid; + sprintfW(buffW, fmtW, ptr->id); break; case GEO_ISO_UN_NUMBER: - val = ptr->uncode; + sprintfW(buffW, fmtW, ptr->uncode); break; case GEO_PARENT: - val = ptr->parent; + sprintfW(buffW, fmtW, ptr->parent); break; case GEO_ISO2: + str = ptr->iso2W; + break; case GEO_ISO3: - { - str = geotype == GEO_ISO2 ? ptr->iso2W : ptr->iso3W; + str = ptr->iso3W; break; - } case GEO_RFC1766: case GEO_LCID: case GEO_FRIENDLYNAME: @@ -4154,12 +4154,6 @@ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, return 0; } - if (val) { - static const WCHAR fmtW[] = {'%','d',0}; - sprintfW(buffW, fmtW, val); - str = buffW; - } - len = strlenW(str) + 1; if (!data || !data_len) return len; -- 2.24.0
I think I got everything. These were painstakingly tested against Windows 10 v1909. Signed-off-by: João Diogo Craveiro Ferreira <devilj(a)outlook.pt> --- dlls/kernel32/locale.c | 65 +++++++++++++++++++++--------------------- 1 file changed, 33 insertions(+), 32 deletions(-) diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index 086389b45f..5ee50272e7 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -3883,12 +3883,12 @@ static const struct geoinfo_t geoinfodata[] = { { 300, {'A','I',0}, {'A','I','A',0}, 10039880, 660 }, /* Anguilla */ { 301, {'A','Q',0}, {'A','T','A',0}, 39070, 10 }, /* Antarctica */ { 302, {'A','W',0}, {'A','B','W',0}, 10039880, 533 }, /* Aruba */ - { 303, {'X','X',0}, {'X','X',0}, 39070 }, /* Ascension Island */ + { 303, {'X','X',0}, {'X','X',0}, 343 }, /* Ascension Island */ { 304, {'X','X',0}, {'X','X',0}, 10210825 }, /* Ashmore and Cartier Islands */ { 305, {'X','X',0}, {'X','X',0}, 161832256 }, /* Baker Island */ { 306, {'B','V',0}, {'B','V','T',0}, 39070, 74 }, /* Bouvet Island */ { 307, {'K','Y',0}, {'C','Y','M',0}, 10039880, 136 }, /* Cayman Islands */ - { 308, {'X','X',0}, {'X','X',0}, 10210824, 0, LOCATION_BOTH }, /* Channel Islands */ + { 308, {'X','X',0}, {'X','X',0}, 10210824, 830, LOCATION_BOTH }, /* Channel Islands */ { 309, {'C','X',0}, {'C','X','R',0}, 12, 162 }, /* Christmas Island */ { 310, {'X','X',0}, {'X','X',0}, 27114 }, /* Clipperton Island */ { 311, {'C','C',0}, {'C','C','K',0}, 10210825, 166 }, /* Cocos (Keeling) Islands */ @@ -3924,48 +3924,49 @@ static const struct geoinfo_t geoinfodata[] = { { 343, {'S','H',0}, {'S','H','N',0}, 42483, 654 }, /* St. Helena */ { 346, {'X','X',0}, {'X','X',0}, 337 }, /* Tinian Island */ { 347, {'T','K',0}, {'T','K','L',0}, 26286, 772 }, /* Tokelau */ - { 348, {'X','X',0}, {'X','X',0}, 39070 }, /* Tristan da Cunha */ + { 348, {'X','X',0}, {'X','X',0}, 343 }, /* Tristan da Cunha */ { 349, {'T','C',0}, {'T','C','A',0}, 10039880, 796 }, /* Turks and Caicos Islands */ { 351, {'V','G',0}, {'V','G','B',0}, 10039880, 92 }, /* Virgin Islands, British */ { 352, {'W','F',0}, {'W','L','F',0}, 26286, 876 }, /* Wallis and Futuna */ - { 742, {'X','X',0}, {'X','X',0}, 39070, 0, LOCATION_REGION }, /* Africa */ - { 2129, {'X','X',0}, {'X','X',0}, 39070, 0, LOCATION_REGION }, /* Asia */ - { 10541, {'X','X',0}, {'X','X',0}, 39070, 0, LOCATION_REGION }, /* Europe */ + { 742, {'X','X',0}, {'X','X',0}, 39070, 2, LOCATION_REGION }, /* Africa */ + { 2129, {'X','X',0}, {'X','X',0}, 39070, 142, LOCATION_REGION }, /* Asia */ + { 10541, {'X','X',0}, {'X','X',0}, 39070, 150, LOCATION_REGION }, /* Europe */ { 15126, {'I','M',0}, {'I','M','N',0}, 10039882, 833 }, /* Man, Isle of */ { 19618, {'M','K',0}, {'M','K','D',0}, 47610, 807 }, /* Macedonia, Former Yugoslav Republic of */ - { 20900, {'X','X',0}, {'X','X',0}, 27114, 0, LOCATION_REGION }, /* Melanesia */ - { 21206, {'X','X',0}, {'X','X',0}, 27114, 0, LOCATION_REGION }, /* Micronesia */ + { 20900, {'X','X',0}, {'X','X',0}, 27114, 54, LOCATION_REGION }, /* Melanesia */ + { 21206, {'X','X',0}, {'X','X',0}, 27114, 57, LOCATION_REGION }, /* Micronesia */ { 21242, {'X','X',0}, {'X','X',0}, 161832256 }, /* Midway Islands */ - { 23581, {'X','X',0}, {'X','X',0}, 10026358, 0, LOCATION_REGION }, /* Northern America */ - { 26286, {'X','X',0}, {'X','X',0}, 27114, 0, LOCATION_REGION }, /* Polynesia */ - { 27082, {'X','X',0}, {'X','X',0}, 161832257, 0, LOCATION_REGION }, /* Central America */ - { 27114, {'X','X',0}, {'X','X',0}, 39070, 0, LOCATION_REGION }, /* Oceania */ + { 23581, {'X','X',0}, {'X','X',0}, 10026358, 21, LOCATION_REGION }, /* Northern America */ + { 26286, {'X','X',0}, {'X','X',0}, 27114, 61, LOCATION_REGION }, /* Polynesia */ + { 27082, {'X','X',0}, {'X','X',0}, 161832257, 13, LOCATION_REGION }, /* Central America */ + { 27114, {'X','X',0}, {'X','X',0}, 39070, 9, LOCATION_REGION }, /* Oceania */ { 30967, {'S','X',0}, {'S','X','M',0}, 10039880, 534 }, /* Sint Maarten (Dutch part) */ - { 31396, {'X','X',0}, {'X','X',0}, 161832257, 0, LOCATION_REGION }, /* South America */ + { 31396, {'X','X',0}, {'X','X',0}, 161832257, 5, LOCATION_REGION }, /* South America */ { 31706, {'M','F',0}, {'M','A','F',0}, 10039880, 663 }, /* Saint Martin (French part) */ - { 39070, {'X','X',0}, {'X','X',0}, 39070, 0, LOCATION_REGION }, /* World */ - { 42483, {'X','X',0}, {'X','X',0}, 742, 0, LOCATION_REGION }, /* Western Africa */ - { 42484, {'X','X',0}, {'X','X',0}, 742, 0, LOCATION_REGION }, /* Middle Africa */ - { 42487, {'X','X',0}, {'X','X',0}, 742, 0, LOCATION_REGION }, /* Northern Africa */ - { 47590, {'X','X',0}, {'X','X',0}, 2129, 0, LOCATION_REGION }, /* Central Asia */ - { 47599, {'X','X',0}, {'X','X',0}, 2129, 0, LOCATION_REGION }, /* South-Eastern Asia */ - { 47600, {'X','X',0}, {'X','X',0}, 2129, 0, LOCATION_REGION }, /* Eastern Asia */ - { 47603, {'X','X',0}, {'X','X',0}, 742, 0, LOCATION_REGION }, /* Eastern Africa */ - { 47609, {'X','X',0}, {'X','X',0}, 10541, 0, LOCATION_REGION }, /* Eastern Europe */ - { 47610, {'X','X',0}, {'X','X',0}, 10541, 0, LOCATION_REGION }, /* Southern Europe */ - { 47611, {'X','X',0}, {'X','X',0}, 2129, 0, LOCATION_REGION }, /* Middle East */ - { 47614, {'X','X',0}, {'X','X',0}, 2129, 0, LOCATION_REGION }, /* Southern Asia */ + { 39070, {'X','X',0}, {'X','X',0}, 39070, 1, LOCATION_REGION }, /* World */ + { 42483, {'X','X',0}, {'X','X',0}, 742, 11, LOCATION_REGION }, /* Western Africa */ + { 42484, {'X','X',0}, {'X','X',0}, 742, 17, LOCATION_REGION }, /* Middle Africa */ + { 42487, {'X','X',0}, {'X','X',0}, 742, 15, LOCATION_REGION }, /* Northern Africa */ + { 47590, {'X','X',0}, {'X','X',0}, 2129, 143, LOCATION_REGION }, /* Central Asia */ + { 47599, {'X','X',0}, {'X','X',0}, 2129, 35, LOCATION_REGION }, /* South-Eastern Asia */ + { 47600, {'X','X',0}, {'X','X',0}, 2129, 30, LOCATION_REGION }, /* Eastern Asia */ + { 47603, {'X','X',0}, {'X','X',0}, 742, 14, LOCATION_REGION }, /* Eastern Africa */ + { 47609, {'X','X',0}, {'X','X',0}, 10541, 151, LOCATION_REGION }, /* Eastern Europe */ + { 47610, {'X','X',0}, {'X','X',0}, 10541, 39, LOCATION_REGION }, /* Southern Europe */ + { 47611, {'X','X',0}, {'X','X',0}, 2129, 145, LOCATION_REGION }, /* Middle East */ + { 47614, {'X','X',0}, {'X','X',0}, 2129, 34, LOCATION_REGION }, /* Southern Asia */ { 7299303, {'T','L',0}, {'T','L','S',0}, 47599, 626 }, /* Democratic Republic of Timor-Leste */ - { 10026358, {'X','X',0}, {'X','X',0}, 39070, 0, LOCATION_REGION }, /* Americas */ + { 9914689, {'X','K',0}, {'X','K','S',0}, 47610, 906 }, /* Kosovo */ + { 10026358, {'X','X',0}, {'X','X',0}, 39070, 19, LOCATION_REGION }, /* Americas */ { 10028789, {'A','X',0}, {'A','L','A',0}, 10039882, 248 }, /* Åland Islands */ - { 10039880, {'X','X',0}, {'X','X',0}, 161832257, 0, LOCATION_REGION }, /* Caribbean */ - { 10039882, {'X','X',0}, {'X','X',0}, 10541, 0, LOCATION_REGION }, /* Northern Europe */ - { 10039883, {'X','X',0}, {'X','X',0}, 742, 0, LOCATION_REGION }, /* Southern Africa */ - { 10210824, {'X','X',0}, {'X','X',0}, 10541, 0, LOCATION_REGION }, /* Western Europe */ - { 10210825, {'X','X',0}, {'X','X',0}, 27114, 0, LOCATION_REGION }, /* Australia and New Zealand */ + { 10039880, {'X','X',0}, {'X','X',0}, 161832257, 29, LOCATION_REGION }, /* Caribbean */ + { 10039882, {'X','X',0}, {'X','X',0}, 10541, 154, LOCATION_REGION }, /* Northern Europe */ + { 10039883, {'X','X',0}, {'X','X',0}, 742, 18, LOCATION_REGION }, /* Southern Africa */ + { 10210824, {'X','X',0}, {'X','X',0}, 10541, 155, LOCATION_REGION }, /* Western Europe */ + { 10210825, {'X','X',0}, {'X','X',0}, 27114, 53, LOCATION_REGION }, /* Australia and New Zealand */ { 161832015, {'B','L',0}, {'B','L','M',0}, 10039880, 652 }, /* Saint Barthélemy */ { 161832256, {'U','M',0}, {'U','M','I',0}, 27114, 581 }, /* U.S. Minor Outlying Islands */ - { 161832257, {'X','X',0}, {'X','X',0}, 10026358, 0, LOCATION_REGION }, /* Latin America and the Caribbean */ + { 161832257, {'X','X',0}, {'X','X',0}, 10026358, 419, LOCATION_REGION }, /* Latin America and the Caribbean */ }; static const struct geoinfo_t *get_geoinfo_dataptr(GEOID geoid) -- 2.24.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=60335 Your paranoid android. === debian10 (32 bit Chinese:China report) === kernel32: comm.c:919: Test failed: OutQueue should not be empty === debian10 (32 bit WoW report) === kernel32: comm.c:919: Test failed: OutQueue should not be empty === debian10 (64 bit WoW report) === kernel32: debugger: Timeout
UN codes are always formatted in groups of three digits in Windows. Also added more geotypes to the list of cases known but unhandled. Signed-off-by: João Diogo Craveiro Ferreira <devilj(a)outlook.pt> --- dlls/kernel32/locale.c | 46 ++++++++++++++++++++++++++++++++---- dlls/kernel32/tests/locale.c | 31 ++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 4 deletions(-) diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index 5ee50272e7..2001a5b940 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -4106,6 +4106,33 @@ BOOL WINAPI SetUserGeoID(GEOID geoid) /****************************************************************************** * GetGeoInfoW (KERNEL32.@) + * + * Retrieves information about a geographic location by its GeoID. + * + * PARAMS + * geoid [I] The GeoID of the location of interest. + * geotype [I] The type of information to be retrieved (SYSGEOTYPE enum from "winnls.h"). + * data [O] The output buffer to store the information. + * data_len [I] The length of the buffer, measured in WCHARs and including the null terminator. + * lang [I] Language identifier. Must be 0 unless geotype is GEO_RFC1766 or GEO_LCID. + * + * RETURNS + * SUCCESS: The number of WCHARs (including null) written to the buffer -or- + * if no buffer was provided, the minimum length required to hold the full data. + * FAILURE: 0. Call GetLastError() to determine the cause. + * + * NOTES + * On failure, GetLastError() will return one of the following values: + * - ERROR_INVALID_PARAMETER: the GeoID provided was invalid. + * - ERROR_INVALID_FLAGS: the specified geotype was invalid. + * - ERROR_INSUFFICIENT_BUFFER: the provided buffer was too small to hold the full data. + * - ERROR_CALL_NOT_IMPLEMENTED: (Wine implementation) we don't handle that geotype yet. + * + * The list of available GeoIDs can be retrieved with EnumSystemGeoID(). + * + * TODO + * Currently, we only handle the following geotypes: GEO_ID, GEO_NATION, GEO_ISO_UN_NUMBER, + * GEO_ISO2, GEO_ISO3 and GEO_NAME. */ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, LANGID lang) { @@ -4113,7 +4140,8 @@ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, WCHAR buffW[12]; const WCHAR *str = buffW; int len; - static const WCHAR fmtW[] = {'%','d',0}; + static const WCHAR id_fmtW[] = {'%','d',0}; + static const WCHAR un_fmtW[] = {'%','0','3','d',0}; TRACE("%d %d %p %d %d\n", geoid, geotype, data, data_len, lang); @@ -4123,14 +4151,15 @@ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, } switch (geotype) { + case GEO_ID: case GEO_NATION: - sprintfW(buffW, fmtW, ptr->id); + sprintfW(buffW, id_fmtW, ptr->id); break; case GEO_ISO_UN_NUMBER: - sprintfW(buffW, fmtW, ptr->uncode); + sprintfW(buffW, un_fmtW, ptr->uncode); break; case GEO_PARENT: - sprintfW(buffW, fmtW, ptr->parent); + sprintfW(buffW, id_fmtW, ptr->parent); break; case GEO_ISO2: str = ptr->iso2W; @@ -4138,6 +4167,12 @@ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, case GEO_ISO3: str = ptr->iso3W; break; + case GEO_NAME: + if (ptr->uncode && !strcmpW((WCHAR[]){'X','X',0}, ptr->iso2W)) + sprintfW(buffW, un_fmtW, ptr->uncode); + else + str = ptr->iso2W; + break; case GEO_RFC1766: case GEO_LCID: case GEO_FRIENDLYNAME: @@ -4146,6 +4181,9 @@ INT WINAPI GetGeoInfoW(GEOID geoid, GEOTYPE geotype, LPWSTR data, int data_len, case GEO_OFFICIALLANGUAGES: case GEO_LATITUDE: case GEO_LONGITUDE: + case GEO_DIALINGCODE: + case GEO_CURRENCYCODE: + case GEO_CURRENCYSYMBOL: FIXME("type %d is not supported\n", geotype); SetLastError(ERROR_CALL_NOT_IMPLEMENTED); return 0; diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index 81e74531ea..432d76b396 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -4798,6 +4798,17 @@ static void test_GetGeoInfo(void) ok(ret == 4, "got %d\n", ret); ok(!strcmp(buffA, "203"), "got %s\n", buffA); + /* GEO_ID should behave like GEO_NATION */ + buffA[0] = 0; + ret = pGetGeoInfoA(203, GEO_ID, buffA, 20, 0); + if (ret) + { + ok(ret == 4, "got %d\n", ret); + ok(!strcmp(buffA, "203"), "got %s\n", buffA); + } + else + win_skip("GEO_ID not supported.\n"); + /* GEO_PARENT */ buffA[0] = 0; ret = pGetGeoInfoA(203, GEO_PARENT, buffA, 20, 0); @@ -4819,6 +4830,26 @@ static void test_GetGeoInfo(void) ok(!strcmp(buffA, "643"), "got %s\n", buffA); } + buffA[0] = 0; + ret = pGetGeoInfoA(193, GEO_NAME, buffA, 20, 0); /* LOCATION_NATION */ + if (ret == 0) + win_skip("GEO_NAME not supported.\n"); + else + { + ok(ret == 3, "expected ret == 3, got %d", ret); + ok(!strcmp(buffA, "PT"), "expected PT, got %s\n", buffA); + + buffA[0] = 0; + ret = pGetGeoInfoA(39070, GEO_NAME, buffA, 20, 0); /* LOCATION_REGION */ + ok(ret == 4, "expected ret == 4, got %d", ret); + ok(!strcmp(buffA, "001"), "expected 001, got %s\n", buffA); + + buffA[0] = 0; + ret = pGetGeoInfoA(333, GEO_NAME, buffA, 20, 0); /* LOCATION_BOTH */ + ok(ret == 3, "expected ret == 3, got %d", ret); + ok(!strcmp(buffA, "AN"), "expected AN, got %s\n", buffA); + } + /* try invalid type value */ SetLastError(0xdeadbeef); ret = pGetGeoInfoA(203, GEO_ID + 1, NULL, 0, 0); -- 2.24.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=60336 Your paranoid android. === debian10 (32 bit report) === kernel32: change.c:320: Test failed: should be ready debugger.c:320: Test failed: GetThreadContext failed: 5 === debian10 (32 bit Chinese:China report) === kernel32: debugger.c:320: Test failed: GetThreadContext failed: 5
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=60334 Your paranoid android. === debian10 (32 bit report) === kernel32: debugger.c:320: Test failed: GetThreadContext failed: 5 === debian10 (32 bit Chinese:China report) === kernel32: comm.c:919: Test failed: OutQueue should not be empty debugger.c:320: Test failed: GetThreadContext failed: 5 debugger.c:320: Test failed: GetThreadContext failed: 5 debugger.c:320: Test failed: GetThreadContext failed: 5 === debian10 (32 bit WoW report) === kernel32: debugger: Timeout
participants (2)
-
João Diogo Ferreira -
Marvin