linuxdcpp-team team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 2756
committer: poy <poy@xxxxxxxxxx>
branch nick: trunk
timestamp: Mon 2011-12-26 17:12:50 +0100
message:
  plug mem leaks: managed trees were not being cleared at all; managed tables too late
modified:
  changelog.txt
  dwt/include/dwt/widgets/Control.h
  win32/DirectoryListingFrame.cpp
  win32/DirectoryListingFrame.h
  win32/FinishedFrameBase.h
  win32/QueueFrame.cpp
  win32/QueueFrame.h
  win32/TransferView.cpp
  win32/TransferView.h
  win32/TypedTable.h
  win32/TypedTree.h
  win32/forward.h


--
lp:dcplusplus
https://code.launchpad.net/~dcplusplus-team/dcplusplus/trunk

Your team Dcplusplus-team is subscribed to branch lp:dcplusplus.
To unsubscribe from this branch go to https://code.launchpad.net/~dcplusplus-team/dcplusplus/trunk/+edit-subscription

=== modified file 'changelog.txt'
--- changelog.txt	2011-12-19 20:49:15 +0000
+++ changelog.txt	2011-12-26 16:12:50 +0000
@@ -55,6 +55,7 @@
 * Improve list filters, add one to filter search results (poy)
 * [L#901237] Fix a possible crash on parital list removal from the queue (thanks bigmuscle)
 * [L#900650] Fix removal of same ADC users logged into multiple hubs when they go offline (emtee)
+* Plug memory leaks in list and tree controls (poy)
 
 -- 0.782 2011-03-05 --
 * Prevent a remote crash triggered via malformed user commands (poy)

=== modified file 'dwt/include/dwt/widgets/Control.h'
--- dwt/include/dwt/widgets/Control.h	2011-11-19 00:10:54 +0000
+++ dwt/include/dwt/widgets/Control.h	2011-12-26 16:12:50 +0000
@@ -92,6 +92,14 @@
 		});
 	}
 
+	/// Callback to execute right before destroying the control.
+	void onDestroy(std::function<void ()> f) {
+		addCallback(Message(WM_DESTROY), [f](const MSG&, LRESULT&) -> bool {
+			f();
+			return false;
+		});
+	}
+
 	/**
 	* add a combination of keys that will launch a function when they are hit. see the ACCEL
 	* structure doc for information about the "fVirt" and "key" arguments.

=== modified file 'win32/DirectoryListingFrame.cpp'
--- win32/DirectoryListingFrame.cpp	2011-12-26 14:23:59 +0000
+++ win32/DirectoryListingFrame.cpp	2011-12-26 16:12:50 +0000
@@ -437,8 +437,6 @@
 }
 
 void DirectoryListingFrame::postClosing() {
-	clearList();
-
 	SettingsManager::getInstance()->set(SettingsManager::DIRECTORYLISTINGFRAME_ORDER, WinUtil::toString(files->getColumnOrder()));
 	SettingsManager::getInstance()->set(SettingsManager::DIRECTORYLISTINGFRAME_WIDTHS, WinUtil::toString(files->getColumnWidths()));
 }
@@ -945,7 +943,7 @@
 void DirectoryListingFrame::changeDir(DirectoryListing::Directory* d) {
 
 	updating = true;
-	clearList();
+	files->clear();
 
 	for(auto i = d->directories.begin(); i != d->directories.end(); ++i) {
 		files->insert(files->size(), new ItemInfo(*i));
@@ -974,10 +972,6 @@
 	}
 }
 
-void DirectoryListingFrame::clearList() {
-	files->clear();
-}
-
 void DirectoryListingFrame::addHistory(const string& name) {
 	history.erase(history.begin() + historyIndex, history.end());
 	while(history.size() > 25)

=== modified file 'win32/DirectoryListingFrame.h'
--- win32/DirectoryListingFrame.h	2011-12-18 15:37:20 +0000
+++ win32/DirectoryListingFrame.h	2011-12-26 16:12:50 +0000
@@ -254,7 +254,6 @@
 	void updateTree(DirectoryListing::Directory* tree, HTREEITEM treeItem);
 	HTREEITEM findItem(HTREEITEM ht, const tstring& name);
 	void selectItem(const tstring& name);
-	void clearList();
 	void updateTitle();
 
 	void loadFile(const tstring& dir);

=== modified file 'win32/FinishedFrameBase.h'
--- win32/FinishedFrameBase.h	2011-12-26 14:23:59 +0000
+++ win32/FinishedFrameBase.h	2011-12-26 16:12:50 +0000
@@ -185,8 +185,6 @@
 	}
 
 	void postClosing() {
-		clearTables();
-
 		saveColumns(files,
 			in_UL ? SettingsManager::FINISHED_UL_FILES_ORDER : SettingsManager::FINISHED_DL_FILES_ORDER,
 			in_UL ? SettingsManager::FINISHED_UL_FILES_WIDTHS : SettingsManager::FINISHED_DL_FILES_WIDTHS);

=== modified file 'win32/QueueFrame.cpp'
--- win32/QueueFrame.cpp	2011-12-22 22:14:45 +0000
+++ win32/QueueFrame.cpp	2011-12-26 16:12:50 +0000
@@ -224,20 +224,14 @@
 }
 
 void QueueFrame::postClosing() {
-	HTREEITEM ht = dirs->getRoot();
-	while(ht != NULL) {
-		clearTree(ht);
-		ht = dirs->getNextSibling(ht);
-	}
-
-	SettingsManager::getInstance()->set(SettingsManager::QUEUE_PANED_POS, paned->getSplitterPos(0));
-
+	dirs->clear();
+	files->clear();
 	for(auto i = directories.begin(); i != directories.end(); ++i) {
 		delete i->second;
 	}
 	directories.clear();
-	files->clear();
 
+	SettingsManager::getInstance()->set(SettingsManager::QUEUE_PANED_POS, paned->getSplitterPos(0));
 	SettingsManager::getInstance()->set(SettingsManager::QUEUEFRAME_ORDER, WinUtil::toString(files->getColumnOrder()));
 	SettingsManager::getInstance()->set(SettingsManager::QUEUEFRAME_WIDTHS, WinUtil::toString(files->getColumnWidths()));
 }
@@ -479,8 +473,7 @@
 
 		// Ok, next now points to x:\... find how much is common
 
-		DirItemInfo* rootInfo = dirs->getData(next);
-		const string& rootStr = rootInfo->getDir();
+		const string& rootStr = dirs->getData(next)->getDir();
 
 		i = 0;
 
@@ -506,7 +499,6 @@
 				moveNode(next, parent);
 				next = dirs->getChild(oldRoot);
 			}
-			delete rootInfo;
 			dirs->erase(oldRoot);
 			parent = newRoot;
 		} else {
@@ -558,7 +550,6 @@
 
 	if(isFileList) {
 		dcassert(fileLists != NULL);
-		delete dirs->getData(fileLists);
 		dirs->erase(fileLists);
 		fileLists = NULL;
 		return;
@@ -585,9 +576,7 @@
 	next = parent;
 
 	while((dirs->getChild(next) == NULL) && (directories.find(getDir(next)) == directories.end())) {
-		delete dirs->getData(next);
 		parent = dirs->getParent(next);
-
 		dirs->erase(next);
 		if(parent == NULL)
 			break;
@@ -601,7 +590,6 @@
 		removeDirectories(next);
 		next = dirs->getNextSibling(ht);
 	}
-	delete dirs->getData(ht);
 	dirs->erase(ht);
 }
 
@@ -807,16 +795,6 @@
 	}
 }
 
-
-void QueueFrame::clearTree(HTREEITEM item) {
-	HTREEITEM next = dirs->getChild(item);
-	while(next != NULL) {
-		clearTree(next);
-		next = dirs->getNextSibling(next);
-	}
-	delete dirs->getData(item);
-}
-
 // Put it here to avoid a copy for each recursion...
 static TCHAR tmpBuf[1024];
 void QueueFrame::moveNode(HTREEITEM item, HTREEITEM parent) {

=== modified file 'win32/QueueFrame.h'
--- win32/QueueFrame.h	2011-10-17 19:39:46 +0000
+++ win32/QueueFrame.h	2011-12-26 16:12:50 +0000
@@ -229,8 +229,6 @@
 
 	QueueItemInfo* getItemInfo(const string& target);
 
-	void clearTree(HTREEITEM item);
-
 	void moveSelected();
 	void moveSelectedDir();
 	void moveDir(HTREEITEM ht, const string& target);

=== modified file 'win32/TransferView.cpp'
--- win32/TransferView.cpp	2011-12-25 16:46:20 +0000
+++ win32/TransferView.cpp	2011-12-26 16:12:50 +0000
@@ -141,7 +141,7 @@
 		downloads->onCustomDraw([this](NMLVCUSTOMDRAW& data) { return handleCustomDraw(data); });
 	}
 
-	onRaw([this](WPARAM, LPARAM) { return handleDestroy(); }, dwt::Message(WM_DESTROY));
+	onDestroy([this] { handleDestroy(); });
 	noEraseBackground();
 
 	layout();
@@ -167,14 +167,12 @@
 	UploadManager::getInstance()->removeListener(this);
 }
 
-LRESULT TransferView::handleDestroy() {
+void TransferView::handleDestroy() {
 	SettingsManager::getInstance()->set(SettingsManager::CONNECTIONS_ORDER, WinUtil::toString(connections->getColumnOrder()));
 	SettingsManager::getInstance()->set(SettingsManager::CONNECTIONS_WIDTHS, WinUtil::toString(connections->getColumnWidths()));
 
 	SettingsManager::getInstance()->set(SettingsManager::DOWNLOADS_ORDER, WinUtil::toString(downloads->getColumnOrder()));
 	SettingsManager::getInstance()->set(SettingsManager::DOWNLOADS_WIDTHS, WinUtil::toString(downloads->getColumnWidths()));
-
-	return 0;
 }
 
 bool TransferView::handleConnectionsMenu(dwt::ScreenCoordinate pt) {

=== modified file 'win32/TransferView.h'
--- win32/TransferView.h	2011-12-14 18:25:45 +0000
+++ win32/TransferView.h	2011-12-26 16:12:50 +0000
@@ -258,7 +258,7 @@
 
 	bool handleConnectionsMenu(dwt::ScreenCoordinate pt);
 	bool handleDownloadsMenu(dwt::ScreenCoordinate pt);
-	LRESULT handleDestroy();
+	void handleDestroy();
 	void handleForce();
 	void handleCopyNick();
 	void handleDisconnect();

=== modified file 'win32/TypedTable.h'
--- win32/TypedTable.h	2011-12-14 18:25:45 +0000
+++ win32/TypedTable.h	2011-12-26 16:12:50 +0000
@@ -72,8 +72,6 @@
 	};
 
 	virtual ~TypedTable() {
-		if(managed)
-			this->clear();
 	}
 
 	void create(const Seed& seed) {
@@ -87,6 +85,10 @@
 		addSortEvent<ContentType>();
 		addStyleEvent<ContentType>();
 		addTooltipEvent<ContentType>();
+
+		if(managed) {
+			onDestroy([this] { this->clear(); });
+		}
 	}
 
 	int insert(ContentType* item) {

=== modified file 'win32/TypedTree.h'
--- win32/TypedTree.h	2011-11-13 16:46:43 +0000
+++ win32/TypedTree.h	2011-12-26 16:12:50 +0000
@@ -19,13 +19,19 @@
 #ifndef DCPLUSPLUS_WIN32_TYPED_TREE_H
 #define DCPLUSPLUS_WIN32_TYPED_TREE_H
 
+#include "forward.h"
 #include "WinUtil.h"
 
-template<typename ContentType>
+/** Tree with an object associated to each item.
+
+@tparam ContentType Type of the objects associated to each item.
+
+@tparam managed Whether this class should handle deleting associated objects. */
+template<typename ContentType, bool managed>
 class TypedTree : public dwt::Tree
 {
 	typedef typename dwt::Tree BaseType;
-	typedef TypedTree<ContentType> ThisType;
+	typedef TypedTree<ContentType, managed> ThisType;
 
 public:
 	typedef ThisType* ObjectType;
@@ -39,6 +45,9 @@
 		}
 	};
 
+	virtual ~TypedTree() {
+	}
+
 	void create(const Seed& seed) {
 		BaseType::create(seed);
 
@@ -47,6 +56,10 @@
 			this->handleDisplay(data);
 			return 0;
 		}, dwt::Message(WM_NOTIFY, TVN_GETDISPINFO));
+
+		if(managed) {
+			onDestroy([this] { this->clear(); });
+		}
 	}
 
 	HTREEITEM insert(HTREEITEM parent, ContentType* data, bool expanded = false) {
@@ -84,6 +97,20 @@
 		TreeView_SetItemState(this->handle(), item, state, mask);
 	}
 
+	void clear() {
+		if(managed) {
+			auto item = this->getRoot();
+			while(item) {
+				this->clear(item);
+				item = this->getNextSibling(item);
+			}
+		}
+
+		this->BaseType::clear();
+	}
+
+	void erase(HTREEITEM item) { if(managed) delete getData(item); this->BaseType::erase(item); }
+
 private:
 	void handleDisplay(NMTVDISPINFO& data) {
 		if(data.item.mask & TVIF_TEXT) {
@@ -103,6 +130,15 @@
 			data.item.iSelectedImage = content->getSelectedImage();
 		}
 	}
+
+	void clear(HTREEITEM item) {
+		auto next = this->getChild(item);
+		while(next) {
+			clear(next);
+			next = this->getNextSibling(next);
+		}
+		delete this->getData(item);
+	}
 };
 
 #endif

=== modified file 'win32/forward.h'
--- win32/forward.h	2011-11-14 19:24:14 +0000
+++ win32/forward.h	2011-12-26 16:12:50 +0000
@@ -60,7 +60,7 @@
 template<typename ContentType, bool managed = true>
 class TypedTable;
 
-template<typename ContentType>
+template<typename ContentType, bool managed = true>
 class TypedTree;
 
 #endif /* FORWARD_H_ */