Use case insensitive matching for hashes QFileSystemWatcher/Win.

Do not lower case file names to generate hash keys since
QString::toLower() converts some characters with context
which the Windows file system will not.

Task-number: QTBUG-31341

Change-Id: I285bfedef3c1ca9d59083229e61974dd378c72ae
Reviewed-by: Jędrzej Nowacki <jedrzej.nowacki@digia.com>
This commit is contained in:
Friedemann Kleint 2013-06-13 13:42:01 +02:00 committed by The Qt Project
parent 1cd8ae2115
commit 6e8dddadd8
3 changed files with 56 additions and 28 deletions

View File

@ -95,7 +95,7 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths,
) )
#endif #endif
normalPath.chop(1); normalPath.chop(1);
QFileInfo fileInfo(normalPath.toLower()); QFileInfo fileInfo(normalPath);
if (!fileInfo.exists()) if (!fileInfo.exists())
continue; continue;
@ -136,15 +136,16 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths,
thread = *jt; thread = *jt;
QMutexLocker locker(&(thread->mutex)); QMutexLocker locker(&(thread->mutex));
handle = thread->handleForDir.value(absolutePath); handle = thread->handleForDir.value(QFileSystemWatcherPathKey(absolutePath));
if (handle.handle != INVALID_HANDLE_VALUE && handle.flags == flags) { if (handle.handle != INVALID_HANDLE_VALUE && handle.flags == flags) {
// found a thread now insert... // found a thread now insert...
DEBUG() << "Found a thread" << thread; DEBUG() << "Found a thread" << thread;
QHash<QString, QWindowsFileSystemWatcherEngine::PathInfo> &h QWindowsFileSystemWatcherEngineThread::PathInfoHash &h =
= thread->pathInfoForHandle[handle.handle]; thread->pathInfoForHandle[handle.handle];
if (!h.contains(fileInfo.absoluteFilePath())) { const QFileSystemWatcherPathKey key(fileInfo.absoluteFilePath());
thread->pathInfoForHandle[handle.handle].insert(fileInfo.absoluteFilePath(), pathInfo); if (!h.contains(key)) {
thread->pathInfoForHandle[handle.handle].insert(key, pathInfo);
if (isDir) if (isDir)
directories->append(path); directories->append(path);
else else
@ -177,9 +178,9 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths,
DEBUG() << "Added handle" << handle.handle << "for" << absolutePath << "to watch" << fileInfo.absoluteFilePath() DEBUG() << "Added handle" << handle.handle << "for" << absolutePath << "to watch" << fileInfo.absoluteFilePath()
<< "to existing thread " << thread; << "to existing thread " << thread;
thread->handles.append(handle.handle); thread->handles.append(handle.handle);
thread->handleForDir.insert(absolutePath, handle); thread->handleForDir.insert(QFileSystemWatcherPathKey(absolutePath), handle);
thread->pathInfoForHandle[handle.handle].insert(fileInfo.absoluteFilePath(), pathInfo); thread->pathInfoForHandle[handle.handle].insert(QFileSystemWatcherPathKey(fileInfo.absoluteFilePath()), pathInfo);
if (isDir) if (isDir)
directories->append(path); directories->append(path);
else else
@ -195,9 +196,9 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths,
QWindowsFileSystemWatcherEngineThread *thread = new QWindowsFileSystemWatcherEngineThread(); QWindowsFileSystemWatcherEngineThread *thread = new QWindowsFileSystemWatcherEngineThread();
DEBUG() << " ###Creating new thread" << thread << "(" << (threads.count()+1) << "threads)"; DEBUG() << " ###Creating new thread" << thread << "(" << (threads.count()+1) << "threads)";
thread->handles.append(handle.handle); thread->handles.append(handle.handle);
thread->handleForDir.insert(absolutePath, handle); thread->handleForDir.insert(QFileSystemWatcherPathKey(absolutePath), handle);
thread->pathInfoForHandle[handle.handle].insert(fileInfo.absoluteFilePath(), pathInfo); thread->pathInfoForHandle[handle.handle].insert(QFileSystemWatcherPathKey(fileInfo.absoluteFilePath()), pathInfo);
if (isDir) if (isDir)
directories->append(path); directories->append(path);
else else
@ -230,7 +231,7 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path
QString normalPath = path; QString normalPath = path;
if (normalPath.endsWith(QLatin1Char('/')) || normalPath.endsWith(QLatin1Char('\\'))) if (normalPath.endsWith(QLatin1Char('/')) || normalPath.endsWith(QLatin1Char('\\')))
normalPath.chop(1); normalPath.chop(1);
QFileInfo fileInfo(normalPath.toLower()); QFileInfo fileInfo(normalPath);
DEBUG() << "removing" << normalPath; DEBUG() << "removing" << normalPath;
QString absolutePath = fileInfo.absoluteFilePath(); QString absolutePath = fileInfo.absoluteFilePath();
QList<QWindowsFileSystemWatcherEngineThread *>::iterator jt, end; QList<QWindowsFileSystemWatcherEngineThread *>::iterator jt, end;
@ -242,16 +243,16 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path
QMutexLocker locker(&(thread->mutex)); QMutexLocker locker(&(thread->mutex));
QWindowsFileSystemWatcherEngine::Handle handle = thread->handleForDir.value(absolutePath); QWindowsFileSystemWatcherEngine::Handle handle = thread->handleForDir.value(QFileSystemWatcherPathKey(absolutePath));
if (handle.handle == INVALID_HANDLE_VALUE) { if (handle.handle == INVALID_HANDLE_VALUE) {
// perhaps path is a file? // perhaps path is a file?
absolutePath = fileInfo.absolutePath(); absolutePath = fileInfo.absolutePath();
handle = thread->handleForDir.value(absolutePath); handle = thread->handleForDir.value(QFileSystemWatcherPathKey(absolutePath));
} }
if (handle.handle != INVALID_HANDLE_VALUE) { if (handle.handle != INVALID_HANDLE_VALUE) {
QHash<QString, QWindowsFileSystemWatcherEngine::PathInfo> &h = QWindowsFileSystemWatcherEngineThread::PathInfoHash &h =
thread->pathInfoForHandle[handle.handle]; thread->pathInfoForHandle[handle.handle];
if (h.remove(fileInfo.absoluteFilePath())) { if (h.remove(QFileSystemWatcherPathKey(fileInfo.absoluteFilePath()))) {
// ### // ###
files->removeAll(path); files->removeAll(path);
directories->removeAll(path); directories->removeAll(path);
@ -264,7 +265,7 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path
Q_ASSERT(indexOfHandle != -1); Q_ASSERT(indexOfHandle != -1);
thread->handles.remove(indexOfHandle); thread->handles.remove(indexOfHandle);
thread->handleForDir.remove(absolutePath); thread->handleForDir.remove(QFileSystemWatcherPathKey(absolutePath));
// h is now invalid // h is now invalid
it.remove(); it.remove();
@ -326,7 +327,7 @@ QWindowsFileSystemWatcherEngineThread::~QWindowsFileSystemWatcherEngineThread()
} }
} }
static inline QString msgFindNextFailed(const QHash<QString, QWindowsFileSystemWatcherEngine::PathInfo> &pathInfos) static inline QString msgFindNextFailed(const QWindowsFileSystemWatcherEngineThread::PathInfoHash &pathInfos)
{ {
QString result; QString result;
QTextStream str(&result); QTextStream str(&result);
@ -366,7 +367,7 @@ void QWindowsFileSystemWatcherEngineThread::run()
// for some reason, so we must check if the handle exist in the handles vector // for some reason, so we must check if the handle exist in the handles vector
if (handles.contains(handle)) { if (handles.contains(handle)) {
DEBUG() << "thread" << this << "Acknowledged handle:" << at << handle; DEBUG() << "thread" << this << "Acknowledged handle:" << at << handle;
QHash<QString, QWindowsFileSystemWatcherEngine::PathInfo> &h = pathInfoForHandle[handle]; QWindowsFileSystemWatcherEngineThread::PathInfoHash &h = pathInfoForHandle[handle];
bool fakeRemove = false; bool fakeRemove = false;
if (!FindNextChangeNotification(handle)) { if (!FindNextChangeNotification(handle)) {
@ -381,9 +382,9 @@ void QWindowsFileSystemWatcherEngineThread::run()
qErrnoWarning(error, "%s", qPrintable(msgFindNextFailed(h))); qErrnoWarning(error, "%s", qPrintable(msgFindNextFailed(h)));
} }
QMutableHashIterator<QString, QWindowsFileSystemWatcherEngine::PathInfo> it(h); QMutableHashIterator<QFileSystemWatcherPathKey, QWindowsFileSystemWatcherEngine::PathInfo> it(h);
while (it.hasNext()) { while (it.hasNext()) {
QHash<QString, QWindowsFileSystemWatcherEngine::PathInfo>::iterator x = it.next(); QWindowsFileSystemWatcherEngineThread::PathInfoHash::iterator x = it.next();
QString absolutePath = x.value().absolutePath; QString absolutePath = x.value().absolutePath;
QFileInfo fileInfo(x.value().path); QFileInfo fileInfo(x.value().path);
DEBUG() << "checking" << x.key(); DEBUG() << "checking" << x.key();
@ -407,7 +408,7 @@ void QWindowsFileSystemWatcherEngineThread::run()
Q_ASSERT(indexOfHandle != -1); Q_ASSERT(indexOfHandle != -1);
handles.remove(indexOfHandle); handles.remove(indexOfHandle);
handleForDir.remove(absolutePath); handleForDir.remove(QFileSystemWatcherPathKey(absolutePath));
// h is now invalid // h is now invalid
} }
} else if (x.value().isDir) { } else if (x.value().isDir) {

View File

@ -128,11 +128,27 @@ private:
}; };
class QFileSystemWatcherPathKey : public QString
{
public:
QFileSystemWatcherPathKey() {}
explicit QFileSystemWatcherPathKey(const QString &other) : QString(other) {}
QFileSystemWatcherPathKey(const QFileSystemWatcherPathKey &other) : QString(other) {}
bool operator==(const QFileSystemWatcherPathKey &other) const { return !compare(other, Qt::CaseInsensitive); }
};
Q_DECLARE_TYPEINFO(QFileSystemWatcherPathKey, Q_MOVABLE_TYPE);
inline uint qHash(const QFileSystemWatcherPathKey &key) { return qHash(key.toCaseFolded()); }
class QWindowsFileSystemWatcherEngineThread : public QThread class QWindowsFileSystemWatcherEngineThread : public QThread
{ {
Q_OBJECT Q_OBJECT
public: public:
typedef QHash<QFileSystemWatcherPathKey, QWindowsFileSystemWatcherEngine::Handle> HandleForDirHash;
typedef QHash<QFileSystemWatcherPathKey, QWindowsFileSystemWatcherEngine::PathInfo> PathInfoHash;
QWindowsFileSystemWatcherEngineThread(); QWindowsFileSystemWatcherEngineThread();
~QWindowsFileSystemWatcherEngineThread(); ~QWindowsFileSystemWatcherEngineThread();
void run(); void run();
@ -143,9 +159,9 @@ public:
QVector<Qt::HANDLE> handles; QVector<Qt::HANDLE> handles;
int msg; int msg;
QHash<QString, QWindowsFileSystemWatcherEngine::Handle> handleForDir; HandleForDirHash handleForDir;
QHash<Qt::HANDLE, QHash<QString, QWindowsFileSystemWatcherEngine::PathInfo> > pathInfoForHandle; QHash<Qt::HANDLE, PathInfoHash> pathInfoForHandle;
Q_SIGNALS: Q_SIGNALS:
void fileChanged(const QString &path, bool removed); void fileChanged(const QString &path, bool removed);

View File

@ -94,20 +94,31 @@ tst_QFileSystemWatcher::tst_QFileSystemWatcher()
void tst_QFileSystemWatcher::basicTest_data() void tst_QFileSystemWatcher::basicTest_data()
{ {
QTest::addColumn<QString>("backend"); QTest::addColumn<QString>("backend");
QTest::newRow("native backend") << "native"; QTest::addColumn<QString>("testFileName");
QTest::newRow("poller backend") << "poller"; const QString testFile = QStringLiteral("testfile.txt");
// QTBUG-31341: Test the UNICODE capabilities; ensure no QString::toLower()
// is in the code path since that will lower case for example
// LATIN_CAPITAL_LETTER_I_WITH_DOT_ABOVE with context, whereas the Windows file
// system will not.
const QString specialCharacterFile =
QString(QChar(ushort(0x130))) // LATIN_CAPITAL_LETTER_I_WITH_DOT_ABOVE
+ QChar(ushort(0x00DC)) // LATIN_CAPITAL_LETTER_U_WITH_DIAERESIS
+ QStringLiteral(".txt");
QTest::newRow("native backend-testfile") << "native" << testFile;
QTest::newRow("poller backend-testfile") << "poller" << testFile;
QTest::newRow("native backend-specialchars") << "native" << specialCharacterFile;
} }
void tst_QFileSystemWatcher::basicTest() void tst_QFileSystemWatcher::basicTest()
{ {
QFETCH(QString, backend); QFETCH(QString, backend);
QFETCH(QString, testFileName);
// create test file // create test file
QTemporaryDir temporaryDirectory(m_tempDirPattern); QTemporaryDir temporaryDirectory(m_tempDirPattern);
QVERIFY(temporaryDirectory.isValid()); QVERIFY(temporaryDirectory.isValid());
QFile testFile(temporaryDirectory.path() + QStringLiteral("/testfile.txt")); QFile testFile(temporaryDirectory.path() + QLatin1Char('/') + testFileName);
testFile.setPermissions(QFile::ReadOwner | QFile::WriteOwner);
testFile.remove();
QVERIFY(testFile.open(QIODevice::WriteOnly | QIODevice::Truncate)); QVERIFY(testFile.open(QIODevice::WriteOnly | QIODevice::Truncate));
testFile.write(QByteArray("hello")); testFile.write(QByteArray("hello"));
testFile.close(); testFile.close();