[PATCH v4] comctl32: Implement handling of EM_SETCUEBANNER/EM_GETCUEBANNER messages.

[Sergio Gómez Del Real]

On 24/10/18 11:36 a. m., Nikolay Sivov wrote:

> On 10/24/2018 07:02 PM, Sergio Gómez Del Real wrote:
>
>> +static inline WCHAR *heap_strdupW(const WCHAR *str)
>> +{
>> +    int len = strlenW(str) + 1;
>> +    WCHAR *ret = heap_alloc(len * sizeof(WCHAR));
>> +    lstrcpynW(ret, str, len);
>> +    return ret;
>> +}
> Now there will be two different functions under the same name (and you 
> don't need lstrcpynW if you know the length).
>
There is a lot of duplication of this simple function; I found it 
copy-pasted in at least 10 places. Ideally that function should be in 
just on place, but I didn't find any gain in moving it to a header for 
just this case while it is being duplicated 10 times in other places.

What do you mean that I don't need lstrcpynW if I know the length? I use 
this function because it copies the null char.

>
>> +    if (!es->cue_banner_text)
>> +    {
>> +        if (!(es->style & ES_MULTILINE) && buf && size)
>> +            *buf = 0;
>> +        return FALSE;
>> +    }
> I don't think tests cover that.
>
I added a test (line 3083 in the tests) for this case specifically.
>> +        if (buf && size)
>> +        {
>> +            if (size > strlenW(es->cue_banner_text))
>> +                size = strlenW(es->cue_banner_text) + 1;
>> +            lstrcpynW(buf, es->cue_banner_text, size);
>> +        }
> Why this size fixup is necessary?
>
We could get a size for a buffer greater than the length of the banner 
text. It should be sized so we don't copy more than necessary.
>> +static void test_cue_banner(void)
>> +{
>> +    HWND hwnd_edit;
>> +    BOOL ret;
>> +    static WCHAR testW[] = {'T','e','s','t',0};
>> +    static WCHAR testcmp1W[] = {'T','e','s','t',0};
>> +    static WCHAR testcmp2W[] = {'T','e','s',0};
>> +    static WCHAR getcuetestW[5] = {'T',0};
>> +    static WCHAR emptyW[] = {0};
> One constant for text and one buffer to get it back is enough.
>
The thing here is that I use testW in various ways; I memset it to 0 to 
test against testcmp1W and testcmp2W. getcuetestW is user for other 
cases. The reason for various strings is that they get modified in 
GETCUEBANNER, so I think it is easier just to use various. I could see 
how I could simplify it though.
>> +    ret = SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 5);
>> +    ok(ret == TRUE, "EM_GETCUEBANNER should have returned FALSE.\n");
> Typo.
>
Ok.