Re: [PATCH] riched20: Check for NULL in fnTextSrv_TxSetText and add test

[Nikolay Sivov]

On 02.01.2017 23:19, Fabian Maurer wrote:
> Fixes https://bugs.winehq.org/show_bug.cgi?id=12185
> Signed-off-by: Fabian Maurer
>
> dlls/riched20/tests/txtsrv.c | 3 +++
>  dlls/riched20/txtsrv.c       | 7 +++----
>  2 files changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/riched20/tests/txtsrv.c b/dlls/riched20/tests/txtsrv.c
> index 1a6ca61052..6fd77d1b59 100644
> --- a/dlls/riched20/tests/txtsrv.c
> +++ b/dlls/riched20/tests/txtsrv.c
> @@ -681,6 +681,9 @@ static void test_TxSetText(void)
>      ok(memcmp(rettext,settext,SysStringByteLen(rettext)) == 0,
>                   "String returned differs\n");
> - /* Test if we crash when using NULL as text */
> - ITextServices_TxSetText(txtserv, NULL);
> - SysFreeString(rettext);
>    ITextServices_Release(txtserv);
>    ITextHost_Release(host);

It's important to test if NULL text actually discards existing content
or just does nothing.
> diff --git a/dlls/riched20/txtsrv.c b/dlls/riched20/txtsrv.c
> index 3df779834b..307b484352 100644
> --- a/dlls/riched20/txtsrv.c
> +++ b/dlls/riched20/txtsrv.c
> @@ -286,10 +286,9 @@ DECLSPEC_HIDDEN HRESULT WINAPI fnTextSrv_TxSetText(ITextServices *iface, LPCWSTR
>     ME_Cursor cursor;
> ME_SetCursorToStart(This->editor, &cursor);
> - ME_InternalDeleteText(This->editor, &cursor,
> -
>                     ME_GetTextLength(This->editor), FALSE);
> - ME_InsertTextFromCursor(This->editor, 0, pszText, -1,
> -
>                       This->editor->pBuffer->pDefaultStyle);
> - ME_InternalDeleteText(This->editor, &cursor, ME_GetTextLength(This->editor), FALSE);
> - if(pszText)
> -
>   ME_InsertTextFromCursor(This->editor, 0, pszText, -1, This->editor->pBuffer->pDefaultStyle);
> ME_SetSelection(This->editor, 0, 0);
>   This->editor->nModifyStep = 0;
>   OleFlushClipboard();

Maybe it's better to add a check to ME_InsertTextFromCursor?

[PATCH] riched20: Check for NULL in fnTextSrv_TxSetText and add test

[Fabian Maurer]

On Monday, January 2, 2017 11:24:31 PM CET Nikolay Sivov wrote:
> It's important to test if NULL text actually discards existing content
> or just does nothing.

Yeah, you're probably right. Thanks, I'll add checks for that.
> Maybe it's better to add a check to ME_InsertTextFromCursor?

I'm not sure if only this function needs it, or if others do, too. But I could 
move the check.