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

[Nikolay Sivov]

On 10/24/2018 07:51 PM, Sergio Gómez Del Real wrote:

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

So does strcpy, or memcpy.

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

There is no tests for buf != NULL && size == 0, or for multiline style.

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

I think lstrcpynW is doing just that.