[PATCH v2] kernel32: Implement FindNLSStringEx. Tests included.

[Sergio Gómez Del Real]

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...