From c01b010fd76421f7f498870e44d32f5a1c17027d Mon Sep 17 00:00:00 2001
From: Pietro Gagliardi <pietro10@mac.com>
Date: Wed, 13 Jun 2018 08:30:00 -0400
Subject: [PATCH] Fixed memory corruption issues. See code for details.

---
 windows/table.cpp  | 37 ++++++-------------------------------
 windows/table.hpp  |  5 -----
 windows/winapi.hpp |  1 -
 3 files changed, 6 insertions(+), 37 deletions(-)

diff --git a/windows/table.cpp b/windows/table.cpp
index c573f97c..26b8b406 100644
--- a/windows/table.cpp
+++ b/windows/table.cpp
@@ -81,28 +81,20 @@ static LRESULT onLVN_GETDISPINFO(uiTable *t, NMLVDISPINFOW *nm)
 	static uiprivTableColumnParams *p;
 	uiTableData *data;
 	WCHAR *wstr;
-	HDC dc;
-	IWICBitmap *wb;
-	HBITMAP b;
-	int checked;
-	bool queueUpdated = false;
 	HRESULT hr;
 
-	wstr = t->dispinfoStrings->front();
-	if (wstr != NULL)
-		uiprivFree(wstr);
-	t->dispinfoStrings->pop();
-
 	p = (*(t->columns))[nm->item.iSubItem];
-nm->item.pszText=L"abcdefg";
 	if ((nm->item.mask & LVIF_TEXT) != 0)
 		if (p->textModelColumn != -1) {
 			data = (*(t->model->mh->CellValue))(t->model->mh, t->model, nm->item.iItem, p->textModelColumn);
 			wstr = toUTF16(uiTableDataString(data));
 			uiFreeTableData(data);
-			nm->item.pszText = wstr;
-			t->dispinfoStrings->push(wstr);
-			queueUpdated = true;
+			// we could just make pszText into a freshly allocated conversion and avoid the limitation of cchTextMax
+			// but then we would have to keep things around for some amount of time (some pages on MSDN say 2 additional LVN_GETDISPINFO messages)
+			// and in practice, anything that results in extra LVN_GETDISPINFO messages (such as fillSubitemDrawParams() below) will break this counting
+			// TODO make it so we don't have to make a copy; instead we can convert directly into pszText (this will also avoid the risk of having a dangling surrogate pair at the end)
+			wcscpy_s(nm->item.pszText, nm->item.cchTextMax, wstr);
+			uiprivFree(wstr);
 		}
 
 	hr = uiprivLVN_GETDISPINFOImagesCheckboxes(t, nm, p);
@@ -110,9 +102,6 @@ nm->item.pszText=L"abcdefg";
 		// TODO
 	}
 
-	// we don't want to pop from an empty queue, so if nothing updated the queue (no info was filled in above), just push NULL
-	if (!queueUpdated)
-		t->dispinfoStrings->push(NULL);
 	return 0;
 }
 
@@ -285,7 +274,6 @@ static void uiTableDestroy(uiControl *c)
 	uiTable *t = uiTable(c);
 	uiTableModel *model = t->model;
 	std::vector<uiTable *>::iterator it;
-	WCHAR *wstr;
 
 	uiWindowsUnregisterWM_NOTIFYHandler(t->hwnd);
 	uiWindowsEnsureDestroyWindow(t->hwnd);
@@ -296,14 +284,6 @@ static void uiTableDestroy(uiControl *c)
 			break;
 		}
 	}
-	// empty the string queue
-	while (t->dispinfoStrings->size() != 0) {
-		wstr = t->dispinfoStrings->front();
-		if (wstr != NULL)
-			uiprivFree(wstr);
-		t->dispinfoStrings->pop();
-	}
-	delete t->dispinfoStrings;
 	// free the columns
 	for (auto col : *(t->columns))
 		uiprivFree(col);
@@ -464,11 +444,6 @@ uiTable *uiNewTable(uiTableModel *model)
 
 	t->backgroundColumn = -1;
 
-	t->dispinfoStrings = new std::queue<WCHAR *>;
-	// this encodes the idea that two LVN_GETDISPINFOs must complete before we can free a string: the first real one is for the fourth call to free
-	for (i = 0; i < uiprivNumLVN_GETDISPINFOSkip; i++)
-		t->dispinfoStrings->push(NULL);
-
 	hr = uiprivTableSetupImagesCheckboxes(t);
 	if (hr != S_OK) {
 		// TODO
diff --git a/windows/table.hpp b/windows/table.hpp
index 514af352..b1b53c66 100644
--- a/windows/table.hpp
+++ b/windows/table.hpp
@@ -30,11 +30,6 @@ struct uiTable {
 	WPARAM nColumns;
 	int backgroundColumn;
 
-	// owner data state
-	// MSDN says we have to keep LVN_GETDISPINFO strings we allocate around at least until "two additional LVN_GETDISPINFO messages have been sent".
-	// we'll use this queue to do so; the "two additional" part is encoded in the initial state of the queue
-	std::queue<WCHAR *> *dispinfoStrings;
-
 	// tableimages.cpp
 	// TODO make sure what we're doing is even allowed
 	HIMAGELIST smallImages;
diff --git a/windows/winapi.hpp b/windows/winapi.hpp
index 297726e3..19426844 100644
--- a/windows/winapi.hpp
+++ b/windows/winapi.hpp
@@ -60,5 +60,4 @@
 #include <unordered_map>
 #include <sstream>
 #include <functional>
-#include <queue>
 #endif