Re: [PATCH] kernel32: Implement CompareStringOrdinal. (try 2)
On 10/30/2012 01:43, Christian Costa wrote:
Try 2: - Don't use CompareStringEx. - Add tests --- dlls/kernel32/kernel32.spec | 1 + dlls/kernel32/locale.c | 28 ++++++++++++++++++++ dlls/kernel32/tests/locale.c | 60 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index b7efa0f..0bd1adc 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -201,6 +201,7 @@ @ stdcall CompareStringA(long long str long str long) @ stdcall CompareStringW(long long wstr long wstr long) @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) +@ stdcall CompareStringOrdinal(wstr long wstr long long) @ stdcall ConnectNamedPipe(long ptr) @ stub ConsoleMenuControl @ stub ConsoleSubst diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index c41442c..07eb688 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -3047,6 +3047,34 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, return ret; }
+/****************************************************************************** + * CompareStringOrdinal (KERNEL32.@) + */ +INT WINAPI CompareStringOrdinal(const WCHAR *str1, INT len1, const WCHAR *str2, INT len2, BOOL ignore_case) +{ + int ret, len; + + if (!str1 || !str2) + { + SetLastError(ERROR_INVALID_PARAMETER); + return 0; + } + if (len1 < 0) len1 = strlenW(str1); + if (len2 < 0) len2 = strlenW(str2); + + len = min(len1, len2); + if (ignore_case) + ret = strncmpiW(str1, str2, len); + else + ret = strncmpW(str1, str2, len); + + if ((ret < 0) || (!ret && (len1 < len2))) + return CSTR_LESS_THAN; + else if ((ret > 0) || (!ret && (len1 > len2))) + return CSTR_GREATER_THAN; + return CSTR_EQUAL; +} + This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.
Le 30/10/2012 08:21, Nikolay Sivov a écrit :
On 10/30/2012 01:43, Christian Costa wrote:
Try 2: - Don't use CompareStringEx. - Add tests --- dlls/kernel32/kernel32.spec | 1 + dlls/kernel32/locale.c | 28 ++++++++++++++++++++ dlls/kernel32/tests/locale.c | 60 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index b7efa0f..0bd1adc 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -201,6 +201,7 @@ @ stdcall CompareStringA(long long str long str long) @ stdcall CompareStringW(long long wstr long wstr long) @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) +@ stdcall CompareStringOrdinal(wstr long wstr long long) @ stdcall ConnectNamedPipe(long ptr) @ stub ConsoleMenuControl @ stub ConsoleSubst diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index c41442c..07eb688 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -3047,6 +3047,34 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, return ret; } +/****************************************************************************** + * CompareStringOrdinal (KERNEL32.@) + */ +INT WINAPI CompareStringOrdinal(const WCHAR *str1, INT len1, const WCHAR *str2, INT len2, BOOL ignore_case) +{ + int ret, len; + + if (!str1 || !str2) + { + SetLastError(ERROR_INVALID_PARAMETER); + return 0; + } + if (len1 < 0) len1 = strlenW(str1); + if (len2 < 0) len2 = strlenW(str2); + + len = min(len1, len2); + if (ignore_case) + ret = strncmpiW(str1, str2, len); + else + ret = strncmpW(str1, str2, len); + + if ((ret < 0) || (!ret && (len1 < len2))) + return CSTR_LESS_THAN; + else if ((ret > 0) || (!ret && (len1 > len2))) + return CSTR_GREATER_THAN; + return CSTR_EQUAL; +} + This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.
Well, It's not the same implementation. It's ordinal now. I have test for that. And how can memcmp be used for insensitive comparison? Do you have a concrete example that does not work with this implementation?
On Tue, 2012-10-30 at 08:42 +0100, Christian Costa wrote:
This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.
Well, It's not the same implementation. It's ordinal now. I have test for that. And how can memcmp be used for insensitive comparison?
Have a look at memicmpW.
Do you have a concrete example that does not work with this implementation
How about CompareStringOrdinal( "a\0a", 3, "a\0b", 3 )?
Le 30/10/2012 09:18, Hans Leidekker a écrit :
On Tue, 2012-10-30 at 08:42 +0100, Christian Costa wrote:
This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.
Well, It's not the same implementation. It's ordinal now. I have test for that. And how can memcmp be used for insensitive comparison? Have a look at memicmpW. Just by curiosity. What the difference with strncmpW?
Do you have a concrete example that does not work with this implementation How about CompareStringOrdinal( "a\0a", 3, "a\0b", 3 )?
There is a similar test in the patch: WCHAR test1[] = { 't','e','s','t',0 }; WCHAR test3[] = { 't','e','s','t','3',0 }; ret = pCompareStringOrdinal(test1, 5, test3, 5, TRUE); ok(ret == CSTR_LESS_THAN, "Got %u, expected %u\n", ret, CSTR_LESS_THAN);
On Tue, 2012-10-30 at 09:50 +0100, Christian Costa wrote:
Just by curiosity. What the difference with strncmpW?
strncmp tests the first string for null termination. You should really determine that by yourself though :-)
Do you have a concrete example that does not work with this implementation How about CompareStringOrdinal( "a\0a", 3, "a\0b", 3 )?
There is a similar test in the patch:
WCHAR test1[] = { 't','e','s','t',0 }; WCHAR test3[] = { 't','e','s','t','3',0 };
ret = pCompareStringOrdinal(test1, 5, test3, 5, TRUE); ok(ret == CSTR_LESS_THAN, "Got %u, expected %u\n", ret, CSTR_LESS_THAN);
It's not the same test. The nulls are not at the same index and you're not asking CompareStringOrdinal to read past a null.
2012/10/30 Hans Leidekker <hans(a)codeweavers.com>
On Tue, 2012-10-30 at 09:50 +0100, Christian Costa wrote:
Just by curiosity. What the difference with strncmpW?
strncmp tests the first string for null termination. You should really determine that by yourself though :-)
Do you have a concrete example that does not work with this implementation How about CompareStringOrdinal( "a\0a", 3, "a\0b", 3 )?
There is a similar test in the patch:
WCHAR test1[] = { 't','e','s','t',0 }; WCHAR test3[] = { 't','e','s','t','3',0 };
ret = pCompareStringOrdinal(test1, 5, test3, 5, TRUE); ok(ret == CSTR_LESS_THAN, "Got %u, expected %u\n", ret, CSTR_LESS_THAN);
It's not the same test. The nulls are not at the same index and you're not asking CompareStringOrdinal to read past a null.
I just wanted to compare 0 with 3 but you're right, I'll add such a test.
On 10/30/2012 10:42, Christian Costa wrote:
Le 30/10/2012 08:21, Nikolay Sivov a écrit :
On 10/30/2012 01:43, Christian Costa wrote:
Try 2: - Don't use CompareStringEx. - Add tests --- dlls/kernel32/kernel32.spec | 1 + dlls/kernel32/locale.c | 28 ++++++++++++++++++++ dlls/kernel32/tests/locale.c | 60 +++++++++++++++++++++++++++++++++++++++++- 3 files changed, 88 insertions(+), 1 deletion(-)
diff --git a/dlls/kernel32/kernel32.spec b/dlls/kernel32/kernel32.spec index b7efa0f..0bd1adc 100644 --- a/dlls/kernel32/kernel32.spec +++ b/dlls/kernel32/kernel32.spec @@ -201,6 +201,7 @@ @ stdcall CompareStringA(long long str long str long) @ stdcall CompareStringW(long long wstr long wstr long) @ stdcall CompareStringEx(wstr long wstr long wstr long ptr ptr long) +@ stdcall CompareStringOrdinal(wstr long wstr long long) @ stdcall ConnectNamedPipe(long ptr) @ stub ConsoleMenuControl @ stub ConsoleSubst diff --git a/dlls/kernel32/locale.c b/dlls/kernel32/locale.c index c41442c..07eb688 100644 --- a/dlls/kernel32/locale.c +++ b/dlls/kernel32/locale.c @@ -3047,6 +3047,34 @@ INT WINAPI CompareStringA(LCID lcid, DWORD flags, return ret; } +/******************************************************************************
+ * CompareStringOrdinal (KERNEL32.@) + */ +INT WINAPI CompareStringOrdinal(const WCHAR *str1, INT len1, const WCHAR *str2, INT len2, BOOL ignore_case) +{ + int ret, len; + + if (!str1 || !str2) + { + SetLastError(ERROR_INVALID_PARAMETER); + return 0; + } + if (len1 < 0) len1 = strlenW(str1); + if (len2 < 0) len2 = strlenW(str2); + + len = min(len1, len2); + if (ignore_case) + ret = strncmpiW(str1, str2, len); + else + ret = strncmpW(str1, str2, len); + + if ((ret < 0) || (!ret && (len1 < len2))) + return CSTR_LESS_THAN; + else if ((ret > 0) || (!ret && (len1 > len2))) + return CSTR_GREATER_THAN; + return CSTR_EQUAL; +} + This is almost the same as your first try. You're testing a trivial case only - ASCII range. I feel like it should behave more like memcmp for case insensitive comparison, that's what documentation mildly suggests.
Well, It's not the same implementation. It's ordinal now. I have test for that. And how can memcmp be used for insensitive comparison? Yeah, sorry for a noise, I thought strncmpW is still too smart for some reason. And I meant memcmp for sensitive case of course, but that's what strncmpW already is. Do you have a concrete example that does not work with this implementation? No, it would be nice to have some tests outside ascii range, but this could be added later.
participants (3)
-
Christian Costa -
Hans Leidekker -
Nikolay Sivov