Fixing heap mismatches introduced with !1520.
-- v2: comctl32: Use CRT heap allocation for some A/W string copies.
From: Rémi Bernon rbernon@codeweavers.com
For strings later freed with CRT heap, to avoid heap mismatch. --- dlls/comctl32/comctl32.h | 25 +++++++++++++++++++++++++ dlls/comctl32/header.c | 12 +++++------- dlls/comctl32/listview.c | 16 +++++++--------- 3 files changed, 37 insertions(+), 16 deletions(-)
diff --git a/dlls/comctl32/comctl32.h b/dlls/comctl32/comctl32.h index f9ed8b84ffd..6d3148b3759 100644 --- a/dlls/comctl32/comctl32.h +++ b/dlls/comctl32/comctl32.h @@ -28,6 +28,7 @@ #include <stdarg.h> #endif
+#include <stdlib.h> #include "windef.h" #include "winbase.h" #include "wingdi.h" @@ -194,6 +195,30 @@ BOOL Str_SetPtrAtoW (LPWSTR *lppDest, LPCSTR lpSrc) DECLSPEC_HIDDEN; BOOL Str_SetPtrWtoA (LPSTR *lppDest, LPCWSTR lpSrc) DECLSPEC_HIDDEN; BOOL imagelist_has_alpha(HIMAGELIST, UINT) DECLSPEC_HIDDEN;
+static inline WCHAR *strdupAW( const char *src ) +{ + WCHAR *dst = NULL; + if (src) + { + int len = MultiByteToWideChar( CP_ACP, 0, src, -1, NULL, 0 ); + if ((dst = malloc( len * sizeof(WCHAR) ))) + MultiByteToWideChar( CP_ACP, 0, src, -1, dst, len ); + } + return dst; +} + +static inline char *strdupWA( const WCHAR *src ) +{ + char *dst = NULL; + if (src) + { + int len = WideCharToMultiByte( CP_ACP, 0, src, -1, NULL, 0, NULL, NULL ); + if ((dst = malloc( len ))) + WideCharToMultiByte( CP_ACP, 0, src, -1, dst, len, NULL, NULL ); + } + return dst; +} + #define COMCTL32_VERSION_MINOR 81
/* Our internal stack structure of the window procedures to subclass */ diff --git a/dlls/comctl32/header.c b/dlls/comctl32/header.c index e905d18e2fa..7a0383909f4 100644 --- a/dlls/comctl32/header.c +++ b/dlls/comctl32/header.c @@ -147,9 +147,9 @@ static void HEADER_StoreHDItemInHeader(HEADER_ITEM *lpItem, UINT mask, const HDI { const WCHAR *pszText = phdi->pszText != NULL ? phdi->pszText : L""; if (fUnicode) - Str_SetPtrW(&lpItem->pszText, pszText); + lpItem->pszText = wcsdup(pszText); else - Str_SetPtrAtoW(&lpItem->pszText, (LPCSTR)pszText); + lpItem->pszText = strdupAW((LPCSTR)pszText); lpItem->callbackMask &= ~HDI_TEXT; } else @@ -854,15 +854,13 @@ static void HEADER_CopyHDItemForNotify(const HEADER_INFO *infoPtr, HDITEMW *dest { if (fSourceUnicode && infoPtr->nNotifyFormat != NFR_UNICODE) { - dest->pszText = NULL; - Str_SetPtrWtoA((LPSTR *)&dest->pszText, src->pszText); + dest->pszText = (LPWSTR)strdupWA(src->pszText); *ppvScratch = dest->pszText; }
if (!fSourceUnicode && infoPtr->nNotifyFormat == NFR_UNICODE) { - dest->pszText = NULL; - Str_SetPtrAtoW(&dest->pszText, (LPSTR)src->pszText); + dest->pszText = strdupAW((LPSTR)src->pszText); *ppvScratch = dest->pszText; } } @@ -1029,7 +1027,7 @@ HEADER_PrepareCallbackItems(const HEADER_INFO *infoPtr, INT iItem, INT reqMask) } else { - Str_SetPtrAtoW(&lpItem->pszText, (LPSTR)dispInfo.pszText); + lpItem->pszText = strdupAW((LPSTR)dispInfo.pszText); free(pvBuffer); } } diff --git a/dlls/comctl32/listview.c b/dlls/comctl32/listview.c index 116c3c5cf94..53d78d37b02 100644 --- a/dlls/comctl32/listview.c +++ b/dlls/comctl32/listview.c @@ -505,16 +505,16 @@ static BOOL textsetptrT(LPWSTR *dest, LPCWSTR src, BOOL isW) { BOOL bResult = TRUE;
+ if (is_text(*dest)) free(*dest); if (src == LPSTR_TEXTCALLBACKW) - { - if (is_text(*dest)) free(*dest); *dest = LPSTR_TEXTCALLBACKW; - } else { LPWSTR pszText = textdupTtoW(src, isW); - if (*dest == LPSTR_TEXTCALLBACKW) *dest = NULL; - bResult = Str_SetPtrW(dest, pszText); + if (!pszText) + *dest = NULL; + else if (!(*dest = wcsdup(pszText))) + bResult = FALSE; textfreeT(pszText, isW); } return bResult; @@ -788,16 +788,14 @@ static LRESULT notify_forward_header(const LISTVIEW_INFO *infoPtr, NMHEADERW *lp if (lpnmh->pitem->mask & HDI_TEXT) { text = (LPCWSTR)lpnmh->pitem->pszText; - lpnmh->pitem->pszText = NULL; - Str_SetPtrWtoA(&lpnmh->pitem->pszText, text); + lpnmh->pitem->pszText = strdupWA(text); } /* convert filter text */ if ((lpnmh->pitem->mask & HDI_FILTER) && (lpnmh->pitem->type == HDFT_ISSTRING) && lpnmh->pitem->pvFilter) { filter = (LPCWSTR)((HD_TEXTFILTERA*)lpnmh->pitem->pvFilter)->pszText; - ((HD_TEXTFILTERA*)lpnmh->pitem->pvFilter)->pszText = NULL; - Str_SetPtrWtoA(&((HD_TEXTFILTERA*)lpnmh->pitem->pvFilter)->pszText, filter); + ((HD_TEXTFILTERA*)lpnmh->pitem->pvFilter)->pszText = strdupWA(filter); } } lpnmh->hdr.code = get_ansi_notification(lpnmh->hdr.code);
Hi,
It looks like your patch introduced the new failures shown below. Please investigate and fix them before resubmitting your patch. If they are not new, fixing them anyway would help a lot. Otherwise please ask for the known failures list to be updated.
The tests also ran into some preexisting test failures. If you know how to fix them that would be helpful. See the TestBot job for the details:
The full results can be found at: https://testbot.winehq.org/JobDetails.pl?Key=127070
Your paranoid android.
=== debian11 (build log) ===
WineRunWineTest.pl:error: The previous 1 run(s) terminated abnormally
v2: Fix one more mismatch in listview.
Feel free to merge this into https://gitlab.winehq.org/wine/wine/-/merge_requests/1625, or do it differently (like converting all allocations at once).
On Thu Dec 1 16:17:58 2022 +0000, Rémi Bernon wrote:
Feel free to merge this into https://gitlab.winehq.org/wine/wine/-/merge_requests/1625, or do it differently (like converting all allocations at once).
Yes, converting at once seems better at this point. There is too much to miss. Those _StrW helpers could stay, because of a side effect when setting null strings.
This was reverted to avoid such mismatches for now, with !1758.
This merge request was closed by Nikolay Sivov.
-
Marvin -
Nikolay Sivov (＠nsivov) -
Rémi Bernon
-
Rémi Bernon (＠rbernon)