[PATCH v2 0/1] MR2941: riched20: Set empty text by default in CreateTextServices.
This is to prevent NULL pointers when creating a standalone text service that doesn't have any text set and then using functions like EM_SETSEL. This NULL pointers doesn't happen when creating a richedit windows, because it sets an empty text when the richedit window procedure handles the WM_CREATE event. -- v2: riched20: Set empty text by default in CreateTextServices. https://gitlab.winehq.org/wine/wine/-/merge_requests/2941
From: Santino Mazza <smazza(a)codeweavers.com> This is to prevent NULL pointers when creating a standalone text service that doesn't have any text set and then using functions like EM_SETSEL. This NULL pointers doesn't happen when creating a richedit windows, because it sets an empty text when the richedit window procedure handles the WM_CREATE event. --- dlls/riched20/tests/txtsrv.c | 22 +++++++++++++++++++++- dlls/riched20/txtsrv.c | 17 ++++++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/dlls/riched20/tests/txtsrv.c b/dlls/riched20/tests/txtsrv.c index d75db465ce4..79f0cee6f55 100644 --- a/dlls/riched20/tests/txtsrv.c +++ b/dlls/riched20/tests/txtsrv.c @@ -620,7 +620,7 @@ static BOOL init_texthost(ITextServices **txtserv, ITextHost **ret) memset(&dummyTextHost->char_format, 0, sizeof(dummyTextHost->char_format)); dummyTextHost->char_format.cbSize = sizeof(dummyTextHost->char_format); dummyTextHost->char_format.dwMask = CFM_ALL2; - dummyTextHost->scrollbars = 0; + dummyTextHost->scrollbars = ES_AUTOVSCROLL; dummyTextHost->props = 0; hf = GetStockObject(DEFAULT_GUI_FONT); hf_to_cf(hf, &dummyTextHost->char_format); @@ -746,6 +746,25 @@ static void test_TxSetText(void) ITextHost_Release(host); } +static void test_set_selection_message(void) +{ + ITextServices *txtserv; + ITextHost *host; + HRESULT result; + + if (!init_texthost(&txtserv, &host)) + return; + + result = ITextServices_TxSendMessage(txtserv, EM_SETSEL, 0, 20, NULL); + ok(result == S_OK, "ITextServices_TxSendMessaage failed (result = %lx)\n", result); + + result = ITextServices_TxSendMessage(txtserv, EM_SETSEL, 20, 0, NULL); + ok(result == S_OK, "ITextServices_TxSendMessaage failed (result = %lx)\n", result); + + ITextServices_Release(txtserv); + ITextHost_Release(host); +} + #define CHECK_TXGETNATURALSIZE(res,width,height,hdc,rect,string) \ _check_txgetnaturalsize(res, width, height, hdc, rect, string, __LINE__) static void _check_txgetnaturalsize(HRESULT res, LONG width, LONG height, HDC hdc, RECT rect, LPCWSTR string, int line) @@ -1267,6 +1286,7 @@ START_TEST( txtsrv ) test_TxGetText(); test_TxSetText(); + test_set_selection_message(); test_TxGetNaturalSize(); test_TxDraw(); test_QueryInterface(); diff --git a/dlls/riched20/txtsrv.c b/dlls/riched20/txtsrv.c index 55e0c7e64bb..36146016ced 100644 --- a/dlls/riched20/txtsrv.c +++ b/dlls/riched20/txtsrv.c @@ -608,5 +608,20 @@ HRESULT create_text_services( IUnknown *outer, ITextHost *text_host, IUnknown ** */ HRESULT WINAPI CreateTextServices( IUnknown *outer, ITextHost *text_host, IUnknown **unk ) { - return create_text_services( outer, text_host, unk, FALSE ); + HRESULT hr; + ITextServices *serv; + + hr = create_text_services( outer, text_host, unk, FALSE ); + if (hr != S_OK) return hr; + + hr = IUnknown_QueryInterface(*unk, &IID_ITextServices, (void**)&serv); + if (hr != S_OK) { + IUnknown_Release(*unk); + return hr; + } + + ITextServices_TxSetText(serv, NULL); + ITextServices_Release(serv); + + return S_OK; } -- GitLab https://gitlab.winehq.org/wine/wine/-/merge_requests/2941
What are the NULL ptrs that need setting? We should likely do this in `create_text_services()` itself. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2941#note_34346
The NULL ptrs appear because there isn't any row created, so for example when calling EM_SETSEL without setting a text it will call editor_ensure_visible which will make the program crash because there are no rows, in windows this doesn't happen. I did the call to TxSetText in CreateTextServices because the call to create_text_service is also made in WM_CREATE of the richedit window and it made the call to TxSetText by it self. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2941#note_34353
I'm going to change everything, I realized that the main problem comes because the function `row_from_cursor` never returns NULL if it fails to find a row, it returns the result of doing `&item->member.row` which will be NULL + the offset of where the property is. With this I also discovered other places on the code where it's possible to get into a segfault, for example when sending the `EM_LINEINDEX` message, this function will check if `row` is null, but because the `row_from_cursor` never return NULL it will never be able to validate that. -- https://gitlab.winehq.org/wine/wine/-/merge_requests/2941#note_34389
participants (3)
-
Huw Davies (＠huw) -
Santino Mazza
-
Santino Mazza (＠tati1454)