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

[Nikolay Sivov]

On 10/22/2018 06:46 PM, Sergio Gómez Del Real wrote:
> -
>   LPWSTR cue_banner_text;          /* cue banner string */

Please use WCHAR * for this, I know comctl32 is not very consistent 
about this. You can remove comment too, field name is self explanatory.
> +static BOOL EDIT_EM_SetCueBanner(EDITSTATE *es, BOOL focus, const WCHAR *cue_text)

Is wParam doing anything?
> - es->cue_banner_text = heap_alloc_zero(ROUND_TO_GROW((str_size + 1) * sizeof(WCHAR)));
> - if (!es->cue_banner_text)
> -
>    return FALSE;

Why size alignment and zero-ing?
> -
>    if (size > strlenW(es->cue_banner_text))
> -
>        lstrcpynW(buf, es->cue_banner_text, strlenW(es->cue_banner_text)+1);
> -
>    else if (size > 0)
> -
>    {
> -
>        memcpy(buf, es->cue_banner_text, (size - 1) * sizeof(WCHAR));
> -
>        buf[size-1] = 0;
> -
>    }
> -
>    ret

It doesn't look like such separation is necessary.
> @@ -4490,6 +4545,7 @@ static LRESULT EDIT_WM_NCDestroy(EDITSTATE *es)
>       SetWindowLongPtrW( es->hwndSelf, 0, 0 );
>       heap_free(es->undo_text);
>       heap_free(es);
> - heap_free(es->cue_banner_text);

This is wrong.
> - ret = (BOOL)SendMessageW(hwnd_edit, EM_GETCUEBANNER, 0, 0);
> - ok(ret == FALSE, "EM_GETCUEBANNER should have returned FALSE.\n");

I don't think you need a cast.
> - ret = (BOOL)SendMessageW(hwnd_edit, EM_GETCUEBANNER, (WPARAM)testW, (LPARAM)lstrlenW(testcmp1W)+1);
> - ok(ret == TRUE ||
> -
>        broken(ret == FALSE), /* Win XP */
> -
>        "EM_GETCUEBANNER should have returned TRUE.\n");

This should be a first thing to try, followed by win_skip().