Re: [PATCH v3 2/2] riched20: Only pass valid length to ME_InsertTextFromCursor

[Huw Davies]

On Mon, Jan 09, 2017 at 06:08:12PM +0100, Fabian Maurer wrote:
> This only affects ME_SetText and fnTextSrv_TxSetText,
> should make it a bit more readable.
> Signed-off-by: Fabian Maurer
>
> dlls/riched20/caret.c  | 2 --
>  dlls/riched20/editor.c | 4 +++-
>  dlls/riched20/txtsrv.c | 2 +-
>  3 files changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/dlls/riched20/caret.c b/dlls/riched20/caret.c
> index 26af743518..94794890b6 100644
> --- a/dlls/riched20/caret.c
> +++ b/dlls/riched20/caret.c
> @@ -526,8 +526,6 @@ void ME_InsertTextFromCursor(ME_TextEditor *editor, int nCursor,
>    assert(style);
>
> assert(nCursor>=0 && nCursor<editor->nCursors);
> -
>
> if (len == -1)
> -
>
> len = lstrlenW(str);
>
> /* grow the text limit to fit our text */
> if(editor->nTextLimit < oldLen +len)
>
> diff --git a/dlls/riched20/editor.c b/dlls/riched20/editor.c
> index 1ce444e604..8873861aee 100644
> --- a/dlls/riched20/editor.c
> +++ b/dlls/riched20/editor.c
> @@ -3323,7 +3323,9 @@ static void ME_SetText(ME_TextEditor *editor, void *text, BOOL unicode)
>      int len = -1;
>  /* uses default style! */
> - if (!(editor->styleFlags & ES_MULTILINE))
> - if (editor->styleFlags & ES_MULTILINE)
> -
>    len = lstrlenW(wszText);
> - else
> {
>   WCHAR *p = wszText;
>
> diff --git a/dlls/riched20/txtsrv.c b/dlls/riched20/txtsrv.c
> index 307b484352..a21ef5a02f 100644
> --- a/dlls/riched20/txtsrv.c
> +++ b/dlls/riched20/txtsrv.c
> @@ -288,7 +288,7 @@ DECLSPEC_HIDDEN HRESULT WINAPI fnTextSrv_TxSetText(ITextServices *iface, LPCWSTR
>     ME_SetCursorToStart(This->editor, &cursor);
>     ME_InternalDeleteText(This->editor, &cursor, ME_GetTextLength(This->editor), FALSE);
>     if(pszText)
> -
>   ME_InsertTextFromCursor(This->editor, 0, pszText, -1, This->editor->pBuffer->pDefaultStyle);
> -
>   ME_InsertTextFromCursor(This->editor, 0, pszText, lstrlenW(pszText), This->editor->pBuffer->pDefaultStyle);
> ME_SetSelection(This->editor, 0, 0);
>   This->editor->nModifyStep = 0;
>   OleFlushClipboard();

I'm not convinced this is an improvement.

Huw.

[PATCH v3 2/2] riched20: Only pass valid length to ME_InsertTextFromCursor

[Fabian Maurer]

On Tuesday, January 10, 2017 10:07:48 AM CET Huw Davies wrote:
> I'm not convinced this is an improvement.

I figured it would be nice for uniformity. I find it cleaner to always calculate 
the size before - or in the function. Currently we have only 2 cases where the 
size is calculated inside, in all other cases it's passed.

As it is now, you can pass the function a null pointer, but only if the length 
is 0. If length is >0, or -1, it crashes.
After my patch, the function requires the length being set properly, but 
always works with a nullpointer.

Fabian