[PATCH v2] kernel32: Implement FindNLSStringEx. Tests included.
Huw Davies
huw at codeweavers.com
Mon Mar 12 04:25:25 CDT 2018
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 at 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
>
>
>
More information about the wine-devel
mailing list