linuxdcpp-team team mailing list archive

[noreply@xxxxxxxxxxxxx]

------------------------------------------------------------
revno: 3159
committer: poy <poy@xxxxxxxxxx>
branch nick: trunk
timestamp: Thu 2012-12-20 20:38:26 +0100
message:
  Reject file lists that contain duplicate items
modified:
  changelog.txt
  dcpp/ADLSearch.cpp
  dcpp/DirectoryListing.cpp
  dcpp/DirectoryListing.h
  win32/DirectoryListingFrame.cpp


--
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	2012-12-17 18:49:40 +0000
+++ changelog.txt	2012-12-20 19:38:26 +0000
@@ -11,8 +11,11 @@
 * [L#1071363] Apply link & plugin formatting to status messages (crise, poy)
 * [L#311818] Share file name duplicates due to directory merges (poy)
 * [L#311818] Share file name duplicates due to case differences (poy)
+* [L#311818] Reject file lists that contain duplicate items (poy)
 
-Note for XP users: shared files will have to be re-hashed.
+Note: The hash registry will be upgraded when running this version for the
+first time. Make sure all your drives are connected to avoid re-hashing.
+That upgrade only works on Win >= Vista; re-hashing is compulsory on XP.
 
 -- 0.802 2012-10-20 --
 * Perf improvements using lock-free queues, requires P6 CPUs (poy)

=== modified file 'dcpp/ADLSearch.cpp'
--- dcpp/ADLSearch.cpp	2012-06-21 18:52:47 +0000
+++ dcpp/ADLSearch.cpp	2012-12-20 19:38:26 +0000
@@ -287,7 +287,7 @@
 			DirectoryListing::File *copyFile = new DirectoryListing::File(*currentFile, true);
 			dcassert(id.subdir->getAdls());
 
-			id.subdir->files.push_back(copyFile);
+			id.subdir->files.insert(copyFile);
 		}
 		id.fileAdded = false;	// Prepare for next stage
 	}
@@ -305,7 +305,7 @@
 		}
 		if(is.matchesFile(currentFile->getName(), filePath, currentFile->getSize())) {
 			DirectoryListing::File *copyFile = new DirectoryListing::File(*currentFile, true);
-			destDirVector[is.ddIndex].dir->files.push_back(copyFile);
+			destDirVector[is.ddIndex].dir->files.insert(copyFile);
 			destDirVector[is.ddIndex].fileAdded = true;
 
 			if(is.isAutoQueue){
@@ -329,7 +329,7 @@
 		if(id.subdir != NULL) {
 			DirectoryListing::Directory* newDir =
 				new DirectoryListing::AdlDirectory(fullPath, id.subdir, currentDir->getName());
-			id.subdir->directories.push_back(newDir);
+			id.subdir->directories.insert(newDir);
 			id.subdir = newDir;
 		}
 	}
@@ -347,7 +347,7 @@
 		if(is.matchesDirectory(currentDir->getName())) {
 			destDirVector[is.ddIndex].subdir =
 				new DirectoryListing::AdlDirectory(fullPath, destDirVector[is.ddIndex].dir, currentDir->getName());
-			destDirVector[is.ddIndex].dir->directories.push_back(destDirVector[is.ddIndex].subdir);
+			destDirVector[is.ddIndex].dir->directories.insert(destDirVector[is.ddIndex].subdir);
 			if(breakOnFirst) {
 				// Found a match, search no more
 				break;
@@ -417,7 +417,7 @@
 		} else if(Util::stricmp(i.dir->getName(), szDiscard) == 0) {
 			delete i.dir;
 		} else {
-			root->directories.push_back(i.dir);
+			root->directories.insert(i.dir);
 		}
 	}
 }

=== modified file 'dcpp/DirectoryListing.cpp'
--- dcpp/DirectoryListing.cpp	2012-12-13 17:50:01 +0000
+++ dcpp/DirectoryListing.cpp	2012-12-20 19:38:26 +0000
@@ -179,45 +179,49 @@
 				return;
 			TTHValue tth(h); /// @todo verify validity?
 
-			if(updating) {
-				// just update the current file if it is already there.
-				for(auto& i: cur->files) {
-					auto& file = *i;
-					/// @todo comparisons should be case-insensitive but it takes too long - add a cache
-					if(file.getTTH() == tth || file.getName() == n) {
-						file.setName(n);
-						file.setSize(size);
-						file.setTTH(tth);
-						return;
-					}
+			auto f = new DirectoryListing::File(cur, n, size, tth);
+			auto insert = cur->files.insert(f);
+
+			if(!insert.second) {
+				// the file was already there
+				delete f;
+				if(updating) {
+					// partial file list
+					f = *insert.first;
+					f->setName(n); // the casing might have changed
+					f->setSize(size);
+					f->setTTH(tth);
+				} else {
+					// duplicates are forbidden in complete file lists
+					throw Exception(_("Duplicate item in the file list"));
 				}
 			}
 
-			DirectoryListing::File* f = new DirectoryListing::File(cur, n, size, tth);
-			cur->files.push_back(f);
-
 		} else if(name == sDirectory) {
 			const string& n = getAttrib(attribs, sName, 0);
 			if(n.empty()) {
 				throw SimpleXMLException(_("Directory missing name attribute"));
 			}
+
 			bool incomp = getAttrib(attribs, sIncomplete, 1) == "1";
-			DirectoryListing::Directory* d = nullptr;
-			if(updating) {
-				for(auto i: cur->directories) {
-					/// @todo comparisons should be case-insensitive but it takes too long - add a cache
-					if(i->getName() == n) {
-						d = i;
-						if(!d->getComplete())
-							d->setComplete(!incomp);
-						break;
-					}
+
+			auto d = new DirectoryListing::Directory(cur, n, false, !incomp);
+			auto insert = cur->directories.insert(d);
+
+			if(!insert.second) {
+				// the dir was already there
+				delete d;
+				if(updating) {
+					// partial file list
+					d = *insert.first;
+					if(!d->getComplete())
+						d->setComplete(!incomp);
+				} else {
+					// duplicates are forbidden in complete file lists
+					throw Exception(_("Duplicate item in the file list"));
 				}
 			}
-			if(!d) {
-				d = new DirectoryListing::Directory(cur, n, false, !incomp);
-				cur->directories.push_back(d);
-			}
+
 			cur = d;
 
 			if(simple) {
@@ -225,6 +229,7 @@
 				endTag(name);
 			}
 		}
+
 	} else if(name == sFileListing) {
 		const string& b = getAttrib(attribs, sBase, 2);
 		if(b.size() >= 1 && b[0] == '/' && *(b.end() - 1) == '/') {
@@ -245,7 +250,7 @@
 			}
 			if(!d) {
 				d = new DirectoryListing::Directory(cur, i, false, false);
-				cur->directories.push_back(d);
+				cur->directories.insert(d);
 			}
 			cur = d;
 		}
@@ -339,26 +344,6 @@
 	stream.write(getTTH().toBase32(tmp));
 	stream.write(LIT("\"/>\r\n"));
 }
-
-void DirectoryListing::sortDirs() {
-	root->sortDirs();
-}
-
-void DirectoryListing::Directory::sortDirs() {
-	for(auto d: directories) {
-		d->sortDirs();
-	}
-	sort(directories.begin(), directories.end(), Directory::Sort());
-}
-
-bool DirectoryListing::Directory::Sort::operator()(const Ptr& a, const Ptr& b) const {
-	return compare(a->getName(), b->getName()) < 0;
-}
-
-bool DirectoryListing::File::Sort::operator()(const Ptr& a, const Ptr& b) const {
-	return compare(a->getName(), b->getName()) < 0;
-}
-
 void DirectoryListing::setComplete(bool complete) {
 	root->setAllComplete(complete);
 }
@@ -404,17 +389,12 @@
 }
 
 void DirectoryListing::download(Directory* aDir, const string& aTarget, bool highPrio) {
-	string tmp;
 	string target = (aDir == getRoot()) ? aTarget : aTarget + aDir->getName() + PATH_SEPARATOR;
 	// First, recurse over the directories
-	Directory::List& lst = aDir->directories;
-	sort(lst.begin(), lst.end(), Directory::Sort());
-	for(auto& j: lst) {
+	for(auto& j: aDir->directories) {
 		download(j, target, highPrio);
 	}
 	// Then add the files
-	File::List& l = aDir->files;
-	sort(l.begin(), l.end(), File::Sort());
 	for(auto file: aDir->files) {
 		try {
 			download(file, target + file->getName(), false, highPrio);
@@ -458,25 +438,9 @@
 	return nullptr;
 }
 
-struct HashContained {
-	HashContained(const DirectoryListing::Directory::TTHSet& l) : tl(l) { }
-	const DirectoryListing::Directory::TTHSet& tl;
-	bool operator()(const DirectoryListing::File::Ptr i) const {
-		return tl.count((i->getTTH())) && (DeleteFunction()(i), true);
-	}
-};
-
-struct DirectoryEmpty {
-	bool operator()(const DirectoryListing::Directory::Ptr i) const {
-		bool r = i->getFileCount() + i->directories.size() == 0;
-		if (r) DeleteFunction()(i);
-		return r;
-	}
-};
-
 DirectoryListing::Directory::~Directory() {
-	for_each(directories.begin(), directories.end(), DeleteFunction());
-	for_each(files.begin(), files.end(), DeleteFunction());
+	std::for_each(directories.begin(), directories.end(), DeleteFunction());
+	std::for_each(files.begin(), files.end(), DeleteFunction());
 }
 
 void DirectoryListing::Directory::filterList(DirectoryListing& dirList) {
@@ -488,9 +452,28 @@
 }
 
 void DirectoryListing::Directory::filterList(DirectoryListing::Directory::TTHSet& l) {
-	for(auto i: directories) i->filterList(l);
-	directories.erase(std::remove_if(directories.begin(),directories.end(),DirectoryEmpty()),directories.end());
-	files.erase(std::remove_if(files.begin(),files.end(),HashContained(l)),files.end());
+	for(auto i = directories.begin(); i != directories.end();) {
+		auto d = *i;
+
+		d->filterList(l);
+
+		if(d->directories.empty() || d->files.empty()) {
+			i = directories.erase(i);
+			delete d;
+		} else {
+			++i;
+		}
+	}
+
+	for(auto i = files.begin(); i != files.end();) {
+		auto f = *i;
+		if(l.find(f->getTTH()) != l.end()) {
+			i = files.erase(i);
+			delete f;
+		} else {
+			++i;
+		}
+	}
 }
 
 void DirectoryListing::Directory::getHashList(DirectoryListing::Directory::TTHSet& l) {

=== modified file 'dcpp/DirectoryListing.h'
--- dcpp/DirectoryListing.h	2012-12-13 17:50:01 +0000
+++ dcpp/DirectoryListing.h	2012-12-20 19:38:26 +0000
@@ -19,6 +19,10 @@
 #ifndef DCPLUSPLUS_DCPP_DIRECTORY_LISTING_H
 #define DCPLUSPLUS_DCPP_DIRECTORY_LISTING_H
 
+#include <set>
+
+#include <boost/noncopyable.hpp>
+
 #include "forward.h"
 #include "noexcept.h"
 
@@ -30,6 +34,8 @@
 
 namespace dcpp {
 
+using std::set;
+
 class ListLoader;
 
 class DirectoryListing : boost::noncopyable
@@ -37,11 +43,9 @@
 public:
 	class Directory;
 
-	class File : public FastAlloc<File> {
+	class File : public FastAlloc<File>, boost::noncopyable {
 	public:
 		typedef File* Ptr;
-		typedef vector<Ptr> List;
-		typedef List::iterator Iter;
 
 		File(Directory* aDir, const string& aName, int64_t aSize, const TTHValue& aTTH) noexcept :
 			name(aName), size(aSize), parent(aDir), tthRoot(aTTH), adls(false)
@@ -54,8 +58,6 @@
 
 		void save(OutputStream& stream, string& indent, string& tmp) const;
 
-		struct Sort { bool operator()(const Ptr& a, const Ptr& b) const; };
-
 		GETSET(string, name, Name);
 		GETSET(int64_t, size, Size);
 		GETSET(Directory*, parent, Parent);
@@ -66,13 +68,15 @@
 	class Directory : public FastAlloc<Directory>, boost::noncopyable {
 	public:
 		typedef Directory* Ptr;
-		typedef vector<Ptr> List;
-		typedef List::iterator Iter;
 
 		typedef unordered_set<TTHValue> TTHSet;
 
-		List directories;
-		File::List files;
+		template<typename T> struct Less {
+			bool operator()(typename T::Ptr a, typename T::Ptr b) const { return compare(a->getName(), b->getName()) < 0; }
+		};
+
+		set<Ptr, Less<Directory>> directories;
+		set<File::Ptr, Less<File>> files;
 
 		Directory(Directory* aParent, const string& aName, bool _adls, bool aComplete)
 			: name(aName), parent(aParent), adls(_adls), complete(aComplete) { }
@@ -85,7 +89,6 @@
 		void filterList(TTHSet& l);
 		void getHashList(TTHSet& l);
 		void save(OutputStream& stream, string& indent, string& tmp) const;
-		void sortDirs();
 		void setAllComplete(bool complete);
 
 		size_t getFileCount() const { return files.size(); }
@@ -98,8 +101,6 @@
 			return x;
 		}
 
-		struct Sort { bool operator()(const Ptr& a, const Ptr& b) const; };
-
 		GETSET(string, name, Name);
 		GETSET(Directory*, parent, Parent);
 		GETSET(bool, adls, Adls);
@@ -123,8 +124,6 @@
 
 	/** write an XML representation of this file list to the specified file. */
 	void save(const string& path) const;
-	/** sort directories and sub-directories recursively (case-insensitive). */
-	void sortDirs();
 	/** recursively mark directories and sub-directories as complete or incomplete. */
 	void setComplete(bool complete);
 

=== modified file 'win32/DirectoryListingFrame.cpp'
--- win32/DirectoryListingFrame.cpp	2012-11-05 20:39:11 +0000
+++ win32/DirectoryListingFrame.cpp	2012-12-20 19:38:26 +0000
@@ -469,9 +469,6 @@
 			step("ADLS");
 			ADLSearchManager::getInstance()->matchListing(*parent.dl);
 
-			step("sorting dirs");
-			parent.dl->sortDirs();
-
 			step("caching dirs");
 			cacheDirs(parent.dl->getRoot());
 
@@ -659,8 +656,7 @@
 				}
 				dirs->erase(dir);
 				dir = dirs->getChild(treeRoot);
-				auto& pdirs = d->getParent()->directories;
-				pdirs.erase(std::remove(pdirs.begin(), pdirs.end(), d), pdirs.end());
+				d->getParent()->directories.erase(d);
 				delete d;
 			} else {
 				dir = dirs->getNextSibling(dir);
@@ -668,8 +664,6 @@
 		}
 		ADLSearchManager::getInstance()->matchListing(*dl);
 
-		dl->sortDirs();
-
 		loaded = true;
 		addRecent();