Skip to content

Commit

Permalink
[win32] LvData: Initialize HDITEM::mask.
Browse files Browse the repository at this point in the history
This somehow managed to mostly work on Windows 7, and even for a while
on Windows 10. Starting with commit 59c3d23
in v2.4, opening the ROM Properties tab would usually crash for EXE/DLL
files due to the Import and/or Export tabs, which have RFT_LISTDATA.

I wasn't able to reproduce this in any Debug builds, but it showed up on
Windows 10 in Release builds with MSVC 17.6.5. The effects were seemingly
random, depending on what code I moved around; sometimes it'd crash, other
times it'd cause the column titles to show up as garbage. Using a TCHAR[]
buffer instead of a tstring seemed to have fixed the problem, until the
garbage titles showed up. (This merely moved the problem around due to
the stack layout.) Making the TCHAR[] buffer static made the problem
worse, since it *removed* stuff from the stack.

In the end, the problem was really very obvious once I saw it. Disabling
LvData's sorting functions caused everything to "just work". When I looked
at the functions, I saw Header_GetItem() called immediately after the
HDITEM variable was declared. HDITEM, like LVCOLUMN, has a mask field which
*must* be filled in for both Set and Get. Otherwise, it might try reading
text into whatever's in HDITEM::pszText, which is likely the cause of the
crashes. The garbage text could be explained by somehow getting through
Header_GetItem(), only to have garbage text set by Header_SetItem().

Fixes #432: [Bug Report] EXE/DLL files causes a crash
Reported by @xxmichibxx.

Affects: v2.4 - v2.4.1 (Windows only), but it's entirely possible that
older versions could have random glitches as well.
  • Loading branch information
GerbilSoft committed Dec 7, 2024
1 parent 3f384ec commit 47fb743
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 3 deletions.
5 changes: 5 additions & 0 deletions NEWS.md
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,11 @@
* Bug fixes:
* Amiibo: Fix an error that can cause the wrong Character Variant to be
displayed for certain amiibo.
* Windows: Fix seemingly random crashes and glitches (mostly on Windows 10)
that may occur when viewing the ROM Properties tab for a file type that
shows ListViews, e.g. EXE and DLL files.
* Fixes #432: [Bug Report] EXE/DLL files causes a crash
* Reported by @xxmichibxx.

## v2.4.1 (released 2024/11/12)

Expand Down
5 changes: 4 additions & 1 deletion src/win32/LvData.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ void LvData::setInitialSort(int column, RomFields::ColSortOrder direction)

// Update the header item.
HDITEM hdi;
hdi.mask = HDI_FORMAT;
Header_GetItem(hHeader, column, &hdi);
switch (direction) {
default:
Expand Down Expand Up @@ -140,11 +141,13 @@ BOOL LvData::toggleSortColumn(int iSubItem)
return FALSE;
}

HDITEM hdi;
hdi.mask = HDI_FORMAT;

// Adjust header item states.
RomFields::ColSortOrder direction = RomFields::COLSORTORDER_ASCENDING;
const int itemCount = Header_GetItemCount(hHeader);
for (int i = 0; i < itemCount; i++) {
HDITEM hdi;
Header_GetItem(hHeader, i, &hdi);

if (i == iSubItem) {
Expand Down
4 changes: 2 additions & 2 deletions src/win32/RP_ShellPropSheetExt.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,6 @@ int RP_ShellPropSheetExt_Private::initListData(_In_ HWND hWndTab,
if (listDataDesc.names) {
auto iter = listDataDesc.names->cbegin();
for (int col = 0; col < colCount; ++iter, col++, align >>= RomFields::TXA_BITS) {
lvColumn.mask = LVCF_TEXT | LVCF_FMT;
lvColumn.fmt = align_tbl[align & RomFields::TXA_MASK];

const string &str = *iter;
Expand All @@ -904,13 +903,14 @@ int RP_ShellPropSheetExt_Private::initListData(_In_ HWND hWndTab,
tstr += _T("...");
}
lvData.col_widths[col] = LibWin32UI::measureStringForListView(hDC, tstr);
lvColumn.mask = LVCF_TEXT | LVCF_FMT;
lvColumn.pszText = const_cast<LPTSTR>(tstr.c_str());
ListView_InsertColumn(hListView, col, &lvColumn);
} else {
// Don't show this column.
// FIXME: Zero-width column is a bad hack...
lvColumn.pszText = const_cast<LPTSTR>(_T(""));
lvColumn.mask |= LVCF_WIDTH;
lvColumn.mask = LVCF_TEXT | LVCF_FMT | LVCF_WIDTH;
lvColumn.cx = 0;
ListView_InsertColumn(hListView, col, &lvColumn);
}
Expand Down

0 comments on commit 47fb743

Please sign in to comment.