[PATCH v2] kernel32: Implement FindNLSStringEx. Tests included.
Fixes PowerShell issue at https://bugs.winehq.org/show_bug.cgi?id=41911 v2: Some tests are broken on Windows Vista and Windows Server 2003 and 2008. Signed-off-by: Sergio Gómez Del Real <sdelreal(a)codeweavers.com> --- dlls/kernel32/kernel32.spec | 2 +- dlls/kernel32/locale.c | 107 +++++++++++++++++++++++++++++++++++++++ dlls/kernel32/tests/locale.c | 116 +++++++++++++++++++++++++++++++++++++++++++ include/winnls.h | 5 ++ 4 files changed, 229 insertions(+), 1 deletion(-) diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index 60809f45fd..3a0bef93e7 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -505,7 +505,7 @@ @ stub FindNextVolumeMountPointW @ stdcall FindNextVolumeW(long ptr long) # @ stub FindNLSString -# @ stub FindNLSStringEx +@ stdcall FindNLSStringEx(wstr long wstr long wstr long ptr ptr ptr long) @ stdcall FindResourceA(long str str) @ stdcall FindResourceExA(long str str long) @ stdcall FindResourceExW(long wstr wstr long) diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index e4c323ead6..123f8b2f3e 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -5941,3 +5941,110 @@ INT WINAPI ResolveLocaleName(LPCWSTR name, LPWSTR localename, INT len) SetLastError(ERROR_CALL_NOT_IMPLEMENTED); return 0; } + +/****************************************************************************** + * FindNLSStringEx (KERNEL32.@) + */ + +INT WINAPI FindNLSStringEx(const WCHAR *localename, DWORD flags, const WCHAR *src, + INT src_size, const WCHAR *value, INT value_size, + INT *found, LPNLSVERSIONINFO version_info, LPVOID reserved, + LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); + + if (version_info != NULL || reserved != NULL || sort_handle != 0) + { + SetLastError(ERROR_INVALID_PARAMETER); + return -1; + } + if (localename == NULL || !IsValidLocaleName(localename)) + { + SetLastError(ERROR_INVALID_PARAMETER); + return -1; + } + if (src == NULL || src_size == 0 || src_size < -1 || + value == NULL || value_size == 0 || value_size < -1) + { + SetLastError(ERROR_INVALID_PARAMETER); + return -1; + } + if ((flags & (FIND_FROMSTART | FIND_FROMEND | FIND_STARTSWITH | FIND_ENDSWITH)) == 0) + flags |= FIND_FROMSTART; + else + { + /* flags are mutually exclusive */ + mask &= (FIND_FROMSTART | FIND_FROMEND | FIND_STARTSWITH | FIND_ENDSWITH); + while ((mask & 1) == 0) + mask >>= 1; + mask >>=1; + if (mask) + { + SetLastError(ERROR_INVALID_FLAGS); + return -1; + } + } + /* caller can delegate string length, but promises to zero-end it */ + if (src_size == -1) + src_size = strlenW(src); + if (value_size == -1) + value_size = strlenW(value); + + src_size -= value_size; + if (src_size < 0) + { + SetLastError(ERROR_SUCCESS); + return -1; + } + + mask = flags & ~(FIND_FROMSTART | FIND_FROMEND | FIND_STARTSWITH | FIND_ENDSWITH); + if (flags & FIND_FROMSTART) + { + int j; + for (j = 0; j < src_size; j++) + { + if (CompareStringEx(localename, mask, src + j, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return j; + } + } + SetLastError(ERROR_SUCCESS); + return -1; + } + else if (flags & FIND_STARTSWITH) + { + if (CompareStringEx(localename, mask, src, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return 0; + } + SetLastError(ERROR_SUCCESS); + return -1; + } + else if (flags & FIND_ENDSWITH) + { + if (CompareStringEx(localename, mask, src + src_size, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return src_size; + } + SetLastError(ERROR_SUCCESS); + return -1; + } + else + { + FIXME("unsupported FINDFROMEND flag\n"); + SetLastError(ERROR_CALL_NOT_IMPLEMENTED); + return -1; + } +} diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index ce2f13b6dc..12af40e0c6 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -103,6 +103,7 @@ static BOOL (WINAPI *pGetThreadPreferredUILanguages)(DWORD, ULONG*, WCHAR*, ULON static BOOL (WINAPI *pGetUserPreferredUILanguages)(DWORD, ULONG*, WCHAR*, ULONG*); static WCHAR (WINAPI *pRtlUpcaseUnicodeChar)(WCHAR); static INT (WINAPI *pGetNumberFormatEx)(LPCWSTR, DWORD, LPCWSTR, const NUMBERFMTW *, LPWSTR, int); +static INT (WINAPI *pFindNLSStringEx)(LPCWSTR, DWORD, LPCWSTR, INT, LPCWSTR, INT, LPINT, LPNLSVERSIONINFO, LPVOID, LPARAM); static void InitFunctionPointers(void) { @@ -135,6 +136,7 @@ static void InitFunctionPointers(void) X(GetThreadPreferredUILanguages); X(GetUserPreferredUILanguages); X(GetNumberFormatEx); + X(FindNLSStringEx); mod = GetModuleHandleA("ntdll"); X(RtlUpcaseUnicodeChar); @@ -5329,6 +5331,119 @@ static void test_GetUserPreferredUILanguages(void) HeapFree(GetProcessHeap(), 0, buffer); } +static void test_FindNLSStringEx(void) +{ + INT res; + static WCHAR en_simpleW[] = {'S','i','m','p','l','e','T','e','s','t',0}; + static WCHAR en_pletW[] = {'p','l','e','T',0}; + static WCHAR en_simpW[] = {'S','i','m','p',0}; + static WCHAR en_testW[] = {'T','e','s','t',0}; + static WCHAR comb_s_accent1W[] = {0x1e69, 'o','u','r','c','e',0}; + static WCHAR comb_s_accent2W[] = {0x0073,0x323,0x307,'o','u','r','c','e',0}; + static WCHAR comb_q_accent1W[] = {0x0071,0x0307,0x323,'u','o','t','e',0}; + static WCHAR comb_q_accent2W[] = {0x0071,0x0323,0x307,'u','o','t','e',0}; + struct test_data { + const WCHAR *locale; + DWORD flags; + WCHAR *src; + INT src_size; + WCHAR *value; + INT val_size; + INT found; + INT expected_ret; + INT expected_found; + int todo; + BOOL broken_vista_servers; + }; + + static struct test_data test_arr[] = + { + { localeW, FIND_FROMSTART, en_simpleW, sizeof(en_simpleW)/sizeof(WCHAR)-1, + en_pletW, sizeof(en_pletW)/sizeof(WCHAR)-1, 0, 3, 4, 0, FALSE}, + { localeW, FIND_STARTSWITH, en_simpleW, sizeof(en_simpleW)/sizeof(WCHAR)-1, + en_simpW, sizeof(en_simpW)/sizeof(WCHAR)-1, 0, 0, 4, 0, FALSE }, + { localeW, FIND_ENDSWITH, en_simpleW, sizeof(en_simpleW)/sizeof(WCHAR)-1, + en_testW, sizeof(en_testW)/sizeof(WCHAR)-1, 0, 6, 4, 0, FALSE }, + { localeW, FIND_FROMSTART, comb_s_accent1W, sizeof(comb_s_accent1W)/sizeof(WCHAR)-1, + comb_s_accent2W, sizeof(comb_s_accent2W)/sizeof(WCHAR)-1, 0, 0, 6, 1, TRUE }, + { localeW, FIND_FROMSTART, comb_q_accent1W, sizeof(comb_q_accent1W)/sizeof(WCHAR)-1, + comb_q_accent2W, sizeof(comb_q_accent2W)/sizeof(WCHAR)-1, 0, 0, 7, 1, FALSE }, + { 0 } + }; + struct test_data *ptest; + + if (!pFindNLSStringEx) + { + win_skip("FindNLSStringEx is not available.\n"); + return; + } + + res = pFindNLSStringEx(invalidW, FIND_FROMSTART, fooW, 3, fooW, + 3, NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART | FIND_ENDSWITH, fooW, 3, fooW, + 3, NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_FLAGS == GetLastError(), + "Expected ERROR_INVALID_FLAGS as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, NULL, 3, fooW, 3, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, fooW, -5, fooW, 3, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, fooW, 3, NULL, 3, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, fooW, 3, fooW, -5, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + for (ptest = test_arr; ptest->src != NULL; ptest++) + { + todo_wine_if(ptest->todo) + { + res = pFindNLSStringEx(ptest->locale, ptest->flags, ptest->src, ptest->src_size, + ptest->value, ptest->val_size, &ptest->found, NULL, NULL, 0); + if (ptest->broken_vista_servers) + { + ok(res == ptest->expected_ret || /* Win 7 onwards */ + broken(res == -1), /* Win Vista, Server 2003 and 2008 */ + "Expected FindNLSStringEx to return %d. Returned value was %d\n", + ptest->expected_ret, res); + ok(ptest->found == ptest->expected_found || /* Win 7 onwards */ + broken(ptest->found == 0), /* Win Vista, Server 2003 and 2008 */ + "Expected FindNLSStringEx to output %d. Value was %d\n", + ptest->expected_found, ptest->found); + } + else + { + ok(res == ptest->expected_ret, + "Expected FindNLSStringEx to return %d. Returned value was %d\n", + ptest->expected_ret, res); + ok(ptest->found == ptest->expected_found, + "Expected FindNLSStringEx to output %d. Value was %d\n", + ptest->expected_found, ptest->found); + } + } + } +} + START_TEST(locale) { InitFunctionPointers(); @@ -5375,6 +5490,7 @@ START_TEST(locale) test_GetSystemPreferredUILanguages(); test_GetThreadPreferredUILanguages(); test_GetUserPreferredUILanguages(); + test_FindNLSStringEx(); /* this requires collation table patch to make it MS compatible */ if (0) test_sorting(); } diff --git a/include/winnls.h b/include/winnls.h index 8cb8af6e8e..ab22bc3cac 100644 --- a/include/winnls.h +++ b/include/winnls.h @@ -352,6 +352,10 @@ static const WCHAR LOCALE_NAME_SYSTEM_DEFAULT[] = {'!','s','y','s','-','d','e',' #define NORM_IGNOREKANATYPE 0x00010000 #define NORM_IGNOREWIDTH 0x00020000 #define NORM_LINGUISTIC_CASING 0x08000000 +#define FIND_STARTSWITH 0x00100000 +#define FIND_ENDSWITH 0x00200000 +#define FIND_FROMSTART 0x00400000 +#define FIND_FROMEND 0x00800000 #define CP_ACP 0 #define CP_OEMCP 1 @@ -961,6 +965,7 @@ WINBASEAPI BOOL WINAPI SetThreadLocale(LCID); WINBASEAPI LANGID WINAPI SetThreadUILanguage(LANGID); WINBASEAPI BOOL WINAPI SetUserGeoID(GEOID); WINBASEAPI INT WINAPI WideCharToMultiByte(UINT,DWORD,LPCWSTR,INT,LPSTR,INT,LPCSTR,LPBOOL); +WINBASEAPI INT WINAPI FindNLSStringEx(const WCHAR *,DWORD,const WCHAR *,INT,const WCHAR *,INT,INT *,LPNLSVERSIONINFO,void *,LPARAM); #ifdef __cplusplus } -- 2.14.1
On Thu, Mar 08, 2018 at 02:35:59PM -0500, Sergio Gómez Del Real wrote:
Fixes PowerShell issue at https://bugs.winehq.org/show_bug.cgi?id=41911
v2: Some tests are broken on Windows Vista and Windows Server 2003 and 2008.
Signed-off-by: Sergio Gómez Del Real <sdelreal(a)codeweavers.com>
Hi Sergio, You can remove 'Tests included' from the commit message.
--- dlls/kernel32/kernel32.spec | 2 +- dlls/kernel32/locale.c | 107 +++++++++++++++++++++++++++++++++++++++ dlls/kernel32/tests/locale.c | 116 +++++++++++++++++++++++++++++++++++++++++++ include/winnls.h | 5 ++ 4 files changed, 229 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index 60809f45fd..3a0bef93e7 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -505,7 +505,7 @@ @ stub FindNextVolumeMountPointW @ stdcall FindNextVolumeW(long ptr long) # @ stub FindNLSString -# @ stub FindNLSStringEx +@ stdcall FindNLSStringEx(wstr long wstr long wstr long ptr ptr ptr long) @ stdcall FindResourceA(long str str) @ stdcall FindResourceExA(long str str long) @ stdcall FindResourceExW(long wstr wstr long) diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index e4c323ead6..123f8b2f3e 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -5941,3 +5941,110 @@ INT WINAPI ResolveLocaleName(LPCWSTR name, LPWSTR localename, INT len) SetLastError(ERROR_CALL_NOT_IMPLEMENTED); return 0; } + +/****************************************************************************** + * FindNLSStringEx (KERNEL32.@) + */ + +INT WINAPI FindNLSStringEx(const WCHAR *localename, DWORD flags, const WCHAR *src, + INT src_size, const WCHAR *value, INT value_size, + INT *found, LPNLSVERSIONINFO version_info, LPVOID reserved,
NLSVERSIONINFO * and void *. Likewise in winnls.h
+ LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n");
I don't think we want the noise that this FIXME would generate. Just add a comment.
+ + if (version_info != NULL || reserved != NULL || sort_handle != 0) ^ double space. + { + SetLastError(ERROR_INVALID_PARAMETER); + return -1; + } + if (localename == NULL || !IsValidLocaleName(localename))
No need for the localname == NULL check since IsValideLocaleName() does that.
+ { + SetLastError(ERROR_INVALID_PARAMETER); + return -1; + } + if (src == NULL || src_size == 0 || src_size < -1 || + value == NULL || value_size == 0 || value_size < -1) + { + SetLastError(ERROR_INVALID_PARAMETER); + return -1; + }
Since the above three if blocks all do the same thing, combine them into one.
+ if ((flags & (FIND_FROMSTART | FIND_FROMEND | FIND_STARTSWITH | FIND_ENDSWITH)) == 0) + flags |= FIND_FROMSTART; + else + {
You can probably skip the more-than-one-flags-set test. It's unlikely an app will rely on this. If you do keep it, there are better ways of doing it (popcount for example). Also beware of inconsistent whitespace ("mask >>=1").
+ /* flags are mutually exclusive */ + mask &= (FIND_FROMSTART | FIND_FROMEND | FIND_STARTSWITH | FIND_ENDSWITH); + while ((mask & 1) == 0) + mask >>= 1; + mask >>=1; + if (mask) + { + SetLastError(ERROR_INVALID_FLAGS); + return -1; + } + } + /* caller can delegate string length, but promises to zero-end it */
Drop this comment, the code is self-explanatory.
+ if (src_size == -1) + src_size = strlenW(src); + if (value_size == -1) + value_size = strlenW(value); + + src_size -= value_size; + if (src_size < 0) + { + SetLastError(ERROR_SUCCESS);
Setting last error to ERROR_SUCCESS is generally wrong, this applies to all the cases below.
+ return -1; + } + + mask = flags & ~(FIND_FROMSTART | FIND_FROMEND | FIND_STARTSWITH | FIND_ENDSWITH); + if (flags & FIND_FROMSTART) + { + int j; + for (j = 0; j < src_size; j++) + { + if (CompareStringEx(localename, mask, src + j, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return j; + } + } + SetLastError(ERROR_SUCCESS); + return -1; + } + else if (flags & FIND_STARTSWITH) + { + if (CompareStringEx(localename, mask, src, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return 0; + } + SetLastError(ERROR_SUCCESS); + return -1; + } + else if (flags & FIND_ENDSWITH) + { + if (CompareStringEx(localename, mask, src + src_size, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return src_size; + } + SetLastError(ERROR_SUCCESS); + return -1; + } + else + {
Any reason you didn't add this bit? It should be easy enough.
+ FIXME("unsupported FINDFROMEND flag\n"); + SetLastError(ERROR_CALL_NOT_IMPLEMENTED); + return -1; + } +} diff --git a/dlls/kernel32/tests/locale.c b/dlls/kernel32/tests/locale.c index ce2f13b6dc..12af40e0c6 100644 --- a/dlls/kernel32/tests/locale.c +++ b/dlls/kernel32/tests/locale.c @@ -103,6 +103,7 @@ static BOOL (WINAPI *pGetThreadPreferredUILanguages)(DWORD, ULONG*, WCHAR*, ULON static BOOL (WINAPI *pGetUserPreferredUILanguages)(DWORD, ULONG*, WCHAR*, ULONG*); static WCHAR (WINAPI *pRtlUpcaseUnicodeChar)(WCHAR); static INT (WINAPI *pGetNumberFormatEx)(LPCWSTR, DWORD, LPCWSTR, const NUMBERFMTW *, LPWSTR, int); +static INT (WINAPI *pFindNLSStringEx)(LPCWSTR, DWORD, LPCWSTR, INT, LPCWSTR, INT, LPINT, LPNLSVERSIONINFO, LPVOID, LPARAM);
static void InitFunctionPointers(void) { @@ -135,6 +136,7 @@ static void InitFunctionPointers(void) X(GetThreadPreferredUILanguages); X(GetUserPreferredUILanguages); X(GetNumberFormatEx); + X(FindNLSStringEx);
mod = GetModuleHandleA("ntdll"); X(RtlUpcaseUnicodeChar); @@ -5329,6 +5331,119 @@ static void test_GetUserPreferredUILanguages(void) HeapFree(GetProcessHeap(), 0, buffer); }
+static void test_FindNLSStringEx(void) +{ + INT res; + static WCHAR en_simpleW[] = {'S','i','m','p','l','e','T','e','s','t',0}; + static WCHAR en_pletW[] = {'p','l','e','T',0}; + static WCHAR en_simpW[] = {'S','i','m','p',0}; + static WCHAR en_testW[] = {'T','e','s','t',0}; + static WCHAR comb_s_accent1W[] = {0x1e69, 'o','u','r','c','e',0}; + static WCHAR comb_s_accent2W[] = {0x0073,0x323,0x307,'o','u','r','c','e',0}; + static WCHAR comb_q_accent1W[] = {0x0071,0x0307,0x323,'u','o','t','e',0}; + static WCHAR comb_q_accent2W[] = {0x0071,0x0323,0x307,'u','o','t','e',0}; + struct test_data { + const WCHAR *locale; + DWORD flags; + WCHAR *src; + INT src_size; + WCHAR *value; + INT val_size; + INT found; + INT expected_ret; + INT expected_found; + int todo; + BOOL broken_vista_servers; + }; + + static struct test_data test_arr[] = + { + { localeW, FIND_FROMSTART, en_simpleW, sizeof(en_simpleW)/sizeof(WCHAR)-1, + en_pletW, sizeof(en_pletW)/sizeof(WCHAR)-1, 0, 3, 4, 0, FALSE}, + { localeW, FIND_STARTSWITH, en_simpleW, sizeof(en_simpleW)/sizeof(WCHAR)-1, + en_simpW, sizeof(en_simpW)/sizeof(WCHAR)-1, 0, 0, 4, 0, FALSE }, + { localeW, FIND_ENDSWITH, en_simpleW, sizeof(en_simpleW)/sizeof(WCHAR)-1, + en_testW, sizeof(en_testW)/sizeof(WCHAR)-1, 0, 6, 4, 0, FALSE }, + { localeW, FIND_FROMSTART, comb_s_accent1W, sizeof(comb_s_accent1W)/sizeof(WCHAR)-1, + comb_s_accent2W, sizeof(comb_s_accent2W)/sizeof(WCHAR)-1, 0, 0, 6, 1, TRUE }, + { localeW, FIND_FROMSTART, comb_q_accent1W, sizeof(comb_q_accent1W)/sizeof(WCHAR)-1, + comb_q_accent2W, sizeof(comb_q_accent2W)/sizeof(WCHAR)-1, 0, 0, 7, 1, FALSE }, + { 0 } + }; + struct test_data *ptest; + + if (!pFindNLSStringEx) + { + win_skip("FindNLSStringEx is not available.\n"); + return; + } + + res = pFindNLSStringEx(invalidW, FIND_FROMSTART, fooW, 3, fooW, + 3, NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART | FIND_ENDSWITH, fooW, 3, fooW, + 3, NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_FLAGS == GetLastError(), + "Expected ERROR_INVALID_FLAGS as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, NULL, 3, fooW, 3, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, fooW, -5, fooW, 3, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, fooW, 3, NULL, 3, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + res = pFindNLSStringEx(localeW, FIND_FROMSTART, fooW, 3, fooW, -5, + NULL, NULL, NULL, 0); + ok(res, "Expected failure of FindNLSStringEx. Return value was %d\n", res); + ok(ERROR_INVALID_PARAMETER == GetLastError(), + "Expected ERROR_INVALID_PARAMETER as last error; got %d\n", GetLastError()); + + for (ptest = test_arr; ptest->src != NULL; ptest++) + { + todo_wine_if(ptest->todo) + { + res = pFindNLSStringEx(ptest->locale, ptest->flags, ptest->src, ptest->src_size, + ptest->value, ptest->val_size, &ptest->found, NULL, NULL, 0); + if (ptest->broken_vista_servers) + { + ok(res == ptest->expected_ret || /* Win 7 onwards */ + broken(res == -1), /* Win Vista, Server 2003 and 2008 */ + "Expected FindNLSStringEx to return %d. Returned value was %d\n", + ptest->expected_ret, res); + ok(ptest->found == ptest->expected_found || /* Win 7 onwards */ + broken(ptest->found == 0), /* Win Vista, Server 2003 and 2008 */ + "Expected FindNLSStringEx to output %d. Value was %d\n", + ptest->expected_found, ptest->found); + } + else + { + ok(res == ptest->expected_ret, + "Expected FindNLSStringEx to return %d. Returned value was %d\n", + ptest->expected_ret, res); + ok(ptest->found == ptest->expected_found, + "Expected FindNLSStringEx to output %d. Value was %d\n", + ptest->expected_found, ptest->found); + } + } + } +} + START_TEST(locale) { InitFunctionPointers(); @@ -5375,6 +5490,7 @@ START_TEST(locale) test_GetSystemPreferredUILanguages(); test_GetThreadPreferredUILanguages(); test_GetUserPreferredUILanguages(); + test_FindNLSStringEx(); /* this requires collation table patch to make it MS compatible */ if (0) test_sorting(); } diff --git a/include/winnls.h b/include/winnls.h index 8cb8af6e8e..ab22bc3cac 100644 --- a/include/winnls.h +++ b/include/winnls.h @@ -352,6 +352,10 @@ static const WCHAR LOCALE_NAME_SYSTEM_DEFAULT[] = {'!','s','y','s','-','d','e',' #define NORM_IGNOREKANATYPE 0x00010000 #define NORM_IGNOREWIDTH 0x00020000 #define NORM_LINGUISTIC_CASING 0x08000000 +#define FIND_STARTSWITH 0x00100000 +#define FIND_ENDSWITH 0x00200000 +#define FIND_FROMSTART 0x00400000 +#define FIND_FROMEND 0x00800000
#define CP_ACP 0 #define CP_OEMCP 1 @@ -961,6 +965,7 @@ WINBASEAPI BOOL WINAPI SetThreadLocale(LCID); WINBASEAPI LANGID WINAPI SetThreadUILanguage(LANGID); WINBASEAPI BOOL WINAPI SetUserGeoID(GEOID); WINBASEAPI INT WINAPI WideCharToMultiByte(UINT,DWORD,LPCWSTR,INT,LPSTR,INT,LPCSTR,LPBOOL); +WINBASEAPI INT WINAPI FindNLSStringEx(const WCHAR *,DWORD,const WCHAR *,INT,const WCHAR *,INT,INT *,LPNLSVERSIONINFO,void *,LPARAM);
#ifdef __cplusplus } -- 2.14.1
On Mon, Mar 12, 2018 at 09:25:25AM +0000, Huw Davies wrote:
On Thu, Mar 08, 2018 at 02:35:59PM -0500, Sergio Gómez Del Real wrote:
+ if (flags & FIND_FROMSTART) + { + int j; + for (j = 0; j < src_size; j++) + { + if (CompareStringEx(localename, mask, src + j, value_size, value, value_size, NULL, NULL, 0) == CSTR_EQUAL) + { + if (found) + *found = value_size; + return j; + } + } + SetLastError(ERROR_SUCCESS); + return -1; + }
And actually, with the correct settings for the start, end and increment values of the above loop, you could use it to handle all four FIND_ flags, which may end up looking rather cleaner. Huw.
On 3/12/2018 12:25 PM, Huw Davies wrote:
+ LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n");
I don't think we want the noise that this FIXME would generate. Just add a comment.
Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
On 12/03/18 06:03, Nikolay Sivov wrote:
On 3/12/2018 12:25 PM, Huw Davies wrote:
+ LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); I don't think we want the noise that this FIXME would generate. Just add a comment. Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
Yeah, you are right Nikolai; I just tested on Windows and it seems that CompareString() shares the same comparison semantics with FindNLSStringEx(). On Wine it fails, however, so I guess I'd code FindNLSStringEx() assuming a working CompareString(), and then see what is missing there. I actually had it like this in my first patch, relying on CompareString (assuming the shared semantics). I wanted to normalize first in this v2 patch so that the substring search would be worst case o(n) instead of o(n.m). However, reading the Unicode standard, it seems that I can make some assumptions about the maximum expansion factor in decomposition (when assuming canonical decomposition). /"There is also a Unicode Consortium stability policy that canonical mappings are always limited in all versions of Unicode, so that no string when decomposed with NFC expands to more than 3× in length (measured in code units). This is true whether the text is in UTF-8, UTF-16, or UTF-32. This guarantee also allows for certain optimizations in processing, especially in determining buffer sizes" /Although it seems that the worst case possible is a 18x expansion factor when using normalization form NFKC, it looks like these functions only test for canonical equivalence, so I guess it would be ok to assume a worst case of 3x for the length to keep things o(n). Does this sound right to you? //
On Mon, Mar 12, 2018 at 09:39:16AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 06:03, Nikolay Sivov wrote: On 3/12/2018 12:25 PM, Huw Davies wrote: + LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); I don't think we want the noise that this FIXME would generate. Just add a comment. Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
Yeah, you are right Nikolai; I just tested on Windows and it seems that CompareString() shares the same comparison semantics with FindNLSStringEx(). On Wine it fails, however, so I guess I'd code FindNLSStringEx() assuming a working CompareString(), and then see what is missing there. I actually had it like this in my first patch, relying on CompareString (assuming the shared semantics). I wanted to normalize first in this v2 patch so that the substring search would be worst case o(n) instead of o(n.m). However, reading the Unicode standard, it seems that I can make some assumptions about the maximum expansion factor in decomposition (when assuming canonical decomposition).
If CompareStringEx handles the normailization, then you'd call it in pretty much the same way as you do in the current patch (though you'd loop right through to the end, not stopping value_size from the end). The tricky bit would be getting the 'found' length. For that you'd probably use an internal version of CompareStringEx that returned this info. For now, I'd stick with the fixme comment. Huw.
On 12/03/18 10:13, Huw Davies wrote:
On Mon, Mar 12, 2018 at 09:39:16AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 06:03, Nikolay Sivov wrote: On 3/12/2018 12:25 PM, Huw Davies wrote: + LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); I don't think we want the noise that this FIXME would generate. Just add a comment. Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
Yeah, you are right Nikolai; I just tested on Windows and it seems that CompareString() shares the same comparison semantics with FindNLSStringEx(). On Wine it fails, however, so I guess I'd code FindNLSStringEx() assuming a working CompareString(), and then see what is missing there. I actually had it like this in my first patch, relying on CompareString (assuming the shared semantics). I wanted to normalize first in this v2 patch so that the substring search would be worst case o(n) instead of o(n.m). However, reading the Unicode standard, it seems that I can make some assumptions about the maximum expansion factor in decomposition (when assuming canonical decomposition). If CompareStringEx handles the normailization, then you'd call it in pretty much the same way as you do in the current patch (though you'd loop right through to the end, not stopping value_size from the end). The tricky bit would be getting the 'found' length. For that you'd probably use an internal version of CompareStringEx that returned this info.
For now, I'd stick with the fixme comment.
Huw. Hi, Huw, Maybe I'm not following well, but in the current patch I'm always calling CompareStringEx() with the same string length for both strings (value_size). Being this the case, I'm not sure if CompareStringEx() would succeed when it needs to, even if it did normalization.
For example if we have 2 strings (let's hope they get well rendered in your environment): src -> "Factorizâtiôn is better" val -> "a^tio^n" (assume â decomposes to 'a','^' and ô to 'o','^'). A call to FindNLSStringEx() with the flag FIND_FROMSTART would mean doing a substring search of 'val' in 'src'. But CompareStringEx() does not do substring search; it just compares 2 strings of the given lengths. So in this example, 'val' is 7 characters long (value_size=7). Looping through 'src' and always passing this value_size for both strings to CompareStringEx() would not succeed, even when FindNLSString() with FIND_FROMSTART should succeed; we have 'val' as a substring of 'src'. That is, FindNLSStringEx(...,src, 5, val, 7,...) should succeed. Again, I might be missing something from your comment...
On Mon, Mar 12, 2018 at 10:31:31AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 10:13, Huw Davies wrote:
On Mon, Mar 12, 2018 at 09:39:16AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 06:03, Nikolay Sivov wrote: On 3/12/2018 12:25 PM, Huw Davies wrote: + LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); I don't think we want the noise that this FIXME would generate. Just add a comment. Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
Yeah, you are right Nikolai; I just tested on Windows and it seems that CompareString() shares the same comparison semantics with FindNLSStringEx(). On Wine it fails, however, so I guess I'd code FindNLSStringEx() assuming a working CompareString(), and then see what is missing there. I actually had it like this in my first patch, relying on CompareString (assuming the shared semantics). I wanted to normalize first in this v2 patch so that the substring search would be worst case o(n) instead of o(n.m). However, reading the Unicode standard, it seems that I can make some assumptions about the maximum expansion factor in decomposition (when assuming canonical decomposition). If CompareStringEx handles the normailization, then you'd call it in pretty much the same way as you do in the current patch (though you'd loop right through to the end, not stopping value_size from the end). The tricky bit would be getting the 'found' length. For that you'd probably use an internal version of CompareStringEx that returned this info.
For now, I'd stick with the fixme comment.
Huw. Hi, Huw, Maybe I'm not following well, but in the current patch I'm always calling CompareStringEx() with the same string length for both strings (value_size). Being this the case, I'm not sure if CompareStringEx() would succeed when it needs to, even if it did normalization.
Grrr, right. This is why you'd do the normailization before calling CompareStringEx(). Huw.
On 12/03/18 10:36, Huw Davies wrote:
On Mon, Mar 12, 2018 at 10:31:31AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 10:13, Huw Davies wrote:
On Mon, Mar 12, 2018 at 09:39:16AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 06:03, Nikolay Sivov wrote: On 3/12/2018 12:25 PM, Huw Davies wrote: + LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); I don't think we want the noise that this FIXME would generate. Just add a comment. Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
Yeah, you are right Nikolai; I just tested on Windows and it seems that CompareString() shares the same comparison semantics with FindNLSStringEx(). On Wine it fails, however, so I guess I'd code FindNLSStringEx() assuming a working CompareString(), and then see what is missing there. I actually had it like this in my first patch, relying on CompareString (assuming the shared semantics). I wanted to normalize first in this v2 patch so that the substring search would be worst case o(n) instead of o(n.m). However, reading the Unicode standard, it seems that I can make some assumptions about the maximum expansion factor in decomposition (when assuming canonical decomposition). If CompareStringEx handles the normailization, then you'd call it in pretty much the same way as you do in the current patch (though you'd loop right through to the end, not stopping value_size from the end). The tricky bit would be getting the 'found' length. For that you'd probably use an internal version of CompareStringEx that returned this info.
For now, I'd stick with the fixme comment.
Huw. Hi, Huw, Maybe I'm not following well, but in the current patch I'm always calling CompareStringEx() with the same string length for both strings (value_size). Being this the case, I'm not sure if CompareStringEx() would succeed when it needs to, even if it did normalization. Grrr, right. This is why you'd do the normailization before calling CompareStringEx().
Huw.
I'm not really sure what assumptions I can make to avoid an o(n.m), since both of the strings can be presented with characters in any (pre)composed form... I don't know if the trouble trying to find a way to upper-bound the search with a length (3x? 5x?) would be worth considering that it seems to be the normal case that we don't get too long strings to compare, and relatively little use of the function itself. Would brute-force o(n.m) be /too/ bad?
On 12/03/18 10:52, Sergio Gómez Del Real wrote:
On 12/03/18 10:36, Huw Davies wrote:
On Mon, Mar 12, 2018 at 10:31:31AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 10:13, Huw Davies wrote:
On Mon, Mar 12, 2018 at 09:39:16AM -0500, Sergio Gómez Del Real wrote:
On 12/03/18 06:03, Nikolay Sivov wrote: On 3/12/2018 12:25 PM, Huw Davies wrote: + LPARAM sort_handle) +{ + + DWORD mask = flags; + + TRACE("%s %x %s %d %s %d %p %p %p %ld\n", wine_dbgstr_w(localename), flags, + wine_dbgstr_w(src), src_size, wine_dbgstr_w(value), value_size, found, + version_info, reserved, sort_handle); + FIXME("strings should be normalized once NormalizeString() is implemented\n"); I don't think we want the noise that this FIXME would generate. Just add a comment. Actually it might be possible that CompareString() handles decomposed case on its own, I haven't tested that.
Yeah, you are right Nikolai; I just tested on Windows and it seems that CompareString() shares the same comparison semantics with FindNLSStringEx(). On Wine it fails, however, so I guess I'd code FindNLSStringEx() assuming a working CompareString(), and then see what is missing there. I actually had it like this in my first patch, relying on CompareString (assuming the shared semantics). I wanted to normalize first in this v2 patch so that the substring search would be worst case o(n) instead of o(n.m). However, reading the Unicode standard, it seems that I can make some assumptions about the maximum expansion factor in decomposition (when assuming canonical decomposition). If CompareStringEx handles the normailization, then you'd call it in pretty much the same way as you do in the current patch (though you'd loop right through to the end, not stopping value_size from the end). The tricky bit would be getting the 'found' length. For that you'd probably use an internal version of CompareStringEx that returned this info.
For now, I'd stick with the fixme comment.
Huw. Hi, Huw, Maybe I'm not following well, but in the current patch I'm always calling CompareStringEx() with the same string length for both strings (value_size). Being this the case, I'm not sure if CompareStringEx() would succeed when it needs to, even if it did normalization. Grrr, right. This is why you'd do the normailization before calling CompareStringEx().
Huw.
I'm not really sure what assumptions I can make to avoid an o(n.m), since both of the strings can be presented with characters in any (pre)composed form... I don't know if the trouble trying to find a way to upper-bound the search with a length (3x? 5x?) would be worth considering that it seems to be the normal case that we don't get too long strings to compare, and relatively little use of the function itself. Would brute-force o(n.m) be /too/ bad?
That is, it seems that the substring search for Unicode strings can get arbitrarily complex, so as to make o(n.m) unavoidable.
On Mon, Mar 12, 2018 at 10:52:17AM -0500, Sergio Gómez Del Real wrote:
I'm not really sure what assumptions I can make to avoid an o(n.m), since both of the strings can be presented with characters in any (pre)composed form... I don't know if the trouble trying to find a way to upper-bound the search with a length (3x? 5x?) would be worth considering that it seems to be the normal case that we don't get too long strings to compare, and relatively little use of the function itself. Would brute-force o(n.m) be too bad?
Just send in v2 of the patch with the changes I suggested. Normalization needs to be implemented before we need to worry about any of this. Huw.
participants (3)
-
Huw Davies -
Nikolay Sivov -
Sergio Gómez Del Real