------------------------------------------------------------
revno: 3151
committer: poy <p...@123gen.com>
branch nick: trunk
timestamp: Wed 2012-12-12 23:44:47 +0100
message:
Share file name duplicates due to directory merges (not tested)
modified:
changelog.txt
dcpp/ShareManager.cpp
dcpp/ShareManager.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 2012-11-08 19:11:36 +0000
+++ changelog.txt 2012-12-12 22:44:47 +0000
@@ -9,6 +9,7 @@
* Update Boost to version 1.52
* Restore "Requesting" messages in the transfer list
* [L#1071363] Apply link & plugin formatting to status messages (crise, poy)
+* [L#311818] Share file name duplicates due to directory merges (poy)
-- 0.802 2012-10-20 --
* Perf improvements using lock-free queues, requires P6 CPUs (poy)
=== modified file 'dcpp/ShareManager.cpp'
--- dcpp/ShareManager.cpp 2012-06-21 18:52:47 +0000
+++ dcpp/ShareManager.cpp 2012-12-12 22:44:47 +0000
@@ -486,17 +486,17 @@
Lock l(cs);
shares.emplace(realPath, vName);
- updateIndices(*merge(dp));
+ updateIndices(*merge(dp, realPath));
setDirty();
}
}
-ShareManager::Directory::Ptr ShareManager::merge(const Directory::Ptr& directory) {
+ShareManager::Directory::Ptr ShareManager::merge(const Directory::Ptr& directory, const string& realPath) {
for(auto& i: directories) {
if(Util::stricmp(i->getName(), directory->getName()) == 0) {
- dcdebug("Merging directory %s\n", directory->getName().c_str());
- i->merge(directory);
+ dcdebug("Merging directory <%s> into %s\n", realPath.c_str(), directory->getName().c_str());
+ i->merge(directory, realPath);
return i;
}
}
@@ -507,38 +507,63 @@
return directory;
}
-void ShareManager::Directory::merge(const Directory::Ptr& source) {
+void ShareManager::Directory::merge(const Directory::Ptr& source, const string& realPath) {
+ // merge directories
for(auto& i: source->directories) {
auto subSource = i.second;
auto ti = directories.find(subSource->getName());
if(ti == directories.end()) {
- if(findFile(subSource->getName()) != files.end()) {
- dcdebug("File named the same as directory");
- } else {
- directories.emplace(subSource->getName(), subSource);
- subSource->parent = this;
+ // the directory doesn't exist; create it.
+ directories.emplace(subSource->getName(), subSource);
+ subSource->parent = this;
+
+ auto f = findFile(subSource->getName());
+ if(f != files.end()) {
+ // we have a file that has the same name as the dir being merged; rename it.
+ const_cast<File&>(*f).validateName(Util::getFilePath(f->getRealPath()));
}
+
} else {
+ // the directory was already existing; merge into it.
auto subTarget = ti->second;
- subTarget->merge(subSource);
+ subTarget->merge(subSource, realPath + subSource->getName() + PATH_SEPARATOR);
}
}
- // All subdirs either deleted or moved to target...
- source->directories.clear();
-
+ // merge files
for(auto& i: source->files) {
- if(findFile(i.getName()) == files.end()) {
- if(directories.find(i.getName()) != directories.end()) {
- dcdebug("Directory named the same as file");
- } else {
- auto added = files.insert(i);
- if(added.second) {
- const_cast<File&>(*added.first).setParent(this);
- }
- }
+ auto& file = const_cast<File&>(i);
+
+ file.setParent(this);
+ file.validateName(realPath);
+
+ files.insert(move(file));
+ }
+}
+
+bool ShareManager::Directory::nameInUse(const string& name) const {
+ return findFile(name) != files.end() || directories.find(name) != directories.end();
+}
+
+void ShareManager::Directory::File::validateName(const string& sourcePath) {
+ if(parent->nameInUse(name)) {
+ uint32_t num = 0;
+ string base = name, ext, vname;
+ auto dot = base.rfind('.');
+ if(dot != string::npos) {
+ ext = base.substr(dot);
+ base.erase(dot);
}
+ do {
+ ++num;
+ vname = base + " (" + Util::toString(num) + ")" + ext;
+ } while(parent->nameInUse(vname));
+ dcdebug("Renaming duplicate <%s> to <%s>\n", name.c_str(), vname.c_str());
+ realPath = sourcePath + name;
+ name = move(vname);
+ } else {
+ realPath.reset();
}
}
@@ -573,7 +598,7 @@
if(Util::stricmp(i->second, vName) == 0 && checkHidden(i->first)) {
Directory::Ptr dp = buildTree(i->first, 0);
dp->setName(i->second);
- merge(dp);
+ merge(dp, i->first);
}
}
@@ -721,7 +746,7 @@
if(!SETTING(LIST_DUPES)) {
try {
LogManager::getInstance()->message(str(F_("Duplicate file will not be shared: %1% (Size: %2% B) Dupe matched against: %3%")
- % Util::addBrackets(dir.getRealPath(f.getName())) % Util::toString(f.getSize()) % Util::addBrackets(j->second->getParent()->getRealPath(j->second->getName()))));
+ % Util::addBrackets(f.getRealPath()) % Util::toString(f.getSize()) % Util::addBrackets(j->second->getRealPath())));
dir.files.erase(i);
} catch (const ShareException&) {
}
@@ -784,12 +809,12 @@
lastFullUpdate = GET_TICK();
- DirList newDirs;
+ vector<pair<Directory::Ptr, string>> newDirs;
for(auto& i: dirs) {
if (checkHidden(i.second)) {
Directory::Ptr dp = buildTree(i.second, Directory::Ptr());
dp->setName(i.first);
- newDirs.push_back(dp);
+ newDirs.emplace_back(dp, i.second);
}
}
@@ -798,7 +823,7 @@
directories.clear();
for(auto& i: newDirs) {
- merge(i);
+ merge(i.first, i.second);
}
rebuildIndices();
=== modified file 'dcpp/ShareManager.h'
--- dcpp/ShareManager.h 2012-05-27 14:02:55 +0000
+++ dcpp/ShareManager.h 2012-12-12 22:44:47 +0000
@@ -24,6 +24,8 @@
#include <set>
#include <unordered_map>
+#include <boost/optional.hpp>
+
#include "TimerManager.h"
#include "SearchManager.h"
#include "SettingsManager.h"
@@ -143,28 +145,28 @@
File() : size(0), parent(0) { }
File(const string& aName, int64_t aSize, const Directory::Ptr& aParent, const TTHValue& aRoot) :
name(aName), tth(aRoot), size(aSize), parent(aParent.get()) { }
- File(const File& rhs) :
- name(rhs.getName()), tth(rhs.getTTH()), size(rhs.getSize()), parent(rhs.getParent()) { }
-
- ~File() { }
-
- File& operator=(const File& rhs) {
- name = rhs.name; size = rhs.size; parent = rhs.parent; tth = rhs.tth;
- return *this;
- }
bool operator==(const File& rhs) const {
return getParent() == rhs.getParent() && (Util::stricmp(getName(), rhs.getName()) == 0);
}
+ /** Ensure this file's name doesn't clash with the names of the parent directory's sub-
+ directories or files; rename to "file (N).ext" otherwise (and set realPath to the
+ actual path on disk).
+ @param sourcePath Real path (on the disk) of the directory this file came from. */
+ void validateName(const string& sourcePath);
+
string getADCPath() const { return parent->getADCPath() + name; }
string getFullName() const { return parent->getFullName() + name; }
- string getRealPath() const { return parent->getRealPath(name); }
+ string getRealPath() const { return realPath ? realPath.get() : parent->getRealPath(name); }
GETSET(string, name, Name);
GETSET(TTHValue, tth, TTH);
GETSET(int64_t, size, Size);
GETSET(Directory*, parent, Parent);
+
+ private:
+ boost::optional<string> realPath;
};
int64_t size;
@@ -182,6 +184,10 @@
string getFullName() const noexcept;
string getRealPath(const std::string& path) const;
+ /** Check whether the given name would clash with this directory's sub-directories or
+ files. */
+ bool nameInUse(const string& name) const;
+
int64_t getSize() const noexcept;
void search(SearchResultList& aResults, StringSearch::List& aStrings, int aSearchType, int64_t aSize, int aFileType, Client* aClient, StringList::size_type maxResults) const noexcept;
@@ -193,7 +199,7 @@
File::Set::const_iterator findFile(const string& aFile) const { return find_if(files.begin(), files.end(), Directory::File::StringComp(aFile)); }
- void merge(const Ptr& source);
+ void merge(const Ptr& source, const string& realPath);
GETSET(string, name, Name);
GETSET(Directory*, parent, Parent);
@@ -262,8 +268,9 @@
typedef std::list<Directory::Ptr> DirList;
DirList directories;
- /** Map real name to virtual name - multiple real names may be mapped to a single virtual one */
- StringMap shares;
+ /** Map real name to virtual name - multiple real names may be mapped to a single virtual one.
+ The map is sorted to make sure conflicts are always resolved in the same order when merging. */
+ map<string, string> shares;
typedef unordered_map<TTHValue, Directory::File::Set::const_iterator> HashFileMap;
typedef HashFileMap::iterator HashFileIter;
@@ -282,7 +289,7 @@
void updateIndices(Directory& aDirectory);
void updateIndices(Directory& dir, const Directory::File::Set::iterator& i);
- Directory::Ptr merge(const Directory::Ptr& directory);
+ Directory::Ptr merge(const Directory::Ptr& directory, const string& realPath);
void generateXmlList();
bool loadCache() noexcept;
_______________________________________________
Mailing list: https://launchpad.net/~linuxdcpp-team
Post to : linuxdcpp-team@lists.launchpad.net
Unsubscribe : https://launchpad.net/~linuxdcpp-team
More help : https://help.launchpad.net/ListHelp
