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

[Sergio Gómez Del Real]

On 24/10/18 12:39 p. m., Nikolay Sivov wrote:

> 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.
>
Ok.
>>
>>> +    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.
>
Ok, I'll add tests for those cases.
>>>> +        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.
Ok.