From 6e8dddadd877ec8e9a8f6006e7beaa183d5da452 Mon Sep 17 00:00:00 2001 From: Friedemann Kleint Date: Thu, 13 Jun 2013 13:42:01 +0200 Subject: [PATCH] Use case insensitive matching for hashes QFileSystemWatcher/Win. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/corelib/io/qfilesystemwatcher_win.cpp | 43 ++++++++++--------- src/corelib/io/qfilesystemwatcher_win_p.h | 20 ++++++++- .../tst_qfilesystemwatcher.cpp | 21 ++++++--- 3 files changed, 56 insertions(+), 28 deletions(-) diff --git a/src/corelib/io/qfilesystemwatcher_win.cpp b/src/corelib/io/qfilesystemwatcher_win.cpp index 6ac6a3fbd9d..bb16df04ad8 100644 --- a/src/corelib/io/qfilesystemwatcher_win.cpp +++ b/src/corelib/io/qfilesystemwatcher_win.cpp @@ -95,7 +95,7 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths, ) #endif normalPath.chop(1); - QFileInfo fileInfo(normalPath.toLower()); + QFileInfo fileInfo(normalPath); if (!fileInfo.exists()) continue; @@ -136,15 +136,16 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths, thread = *jt; QMutexLocker locker(&(thread->mutex)); - handle = thread->handleForDir.value(absolutePath); + handle = thread->handleForDir.value(QFileSystemWatcherPathKey(absolutePath)); if (handle.handle != INVALID_HANDLE_VALUE && handle.flags == flags) { // found a thread now insert... DEBUG() << "Found a thread" << thread; - QHash &h - = thread->pathInfoForHandle[handle.handle]; - if (!h.contains(fileInfo.absoluteFilePath())) { - thread->pathInfoForHandle[handle.handle].insert(fileInfo.absoluteFilePath(), pathInfo); + QWindowsFileSystemWatcherEngineThread::PathInfoHash &h = + thread->pathInfoForHandle[handle.handle]; + const QFileSystemWatcherPathKey key(fileInfo.absoluteFilePath()); + if (!h.contains(key)) { + thread->pathInfoForHandle[handle.handle].insert(key, pathInfo); if (isDir) directories->append(path); else @@ -177,9 +178,9 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths, DEBUG() << "Added handle" << handle.handle << "for" << absolutePath << "to watch" << fileInfo.absoluteFilePath() << "to existing thread " << thread; 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) directories->append(path); else @@ -195,9 +196,9 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths, QWindowsFileSystemWatcherEngineThread *thread = new QWindowsFileSystemWatcherEngineThread(); DEBUG() << " ###Creating new thread" << thread << "(" << (threads.count()+1) << "threads)"; 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) directories->append(path); else @@ -230,7 +231,7 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path QString normalPath = path; if (normalPath.endsWith(QLatin1Char('/')) || normalPath.endsWith(QLatin1Char('\\'))) normalPath.chop(1); - QFileInfo fileInfo(normalPath.toLower()); + QFileInfo fileInfo(normalPath); DEBUG() << "removing" << normalPath; QString absolutePath = fileInfo.absoluteFilePath(); QList::iterator jt, end; @@ -242,16 +243,16 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path 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) { // perhaps path is a file? absolutePath = fileInfo.absolutePath(); - handle = thread->handleForDir.value(absolutePath); + handle = thread->handleForDir.value(QFileSystemWatcherPathKey(absolutePath)); } if (handle.handle != INVALID_HANDLE_VALUE) { - QHash &h = + QWindowsFileSystemWatcherEngineThread::PathInfoHash &h = thread->pathInfoForHandle[handle.handle]; - if (h.remove(fileInfo.absoluteFilePath())) { + if (h.remove(QFileSystemWatcherPathKey(fileInfo.absoluteFilePath()))) { // ### files->removeAll(path); directories->removeAll(path); @@ -264,7 +265,7 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path Q_ASSERT(indexOfHandle != -1); thread->handles.remove(indexOfHandle); - thread->handleForDir.remove(absolutePath); + thread->handleForDir.remove(QFileSystemWatcherPathKey(absolutePath)); // h is now invalid it.remove(); @@ -326,7 +327,7 @@ QWindowsFileSystemWatcherEngineThread::~QWindowsFileSystemWatcherEngineThread() } } -static inline QString msgFindNextFailed(const QHash &pathInfos) +static inline QString msgFindNextFailed(const QWindowsFileSystemWatcherEngineThread::PathInfoHash &pathInfos) { QString 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 if (handles.contains(handle)) { DEBUG() << "thread" << this << "Acknowledged handle:" << at << handle; - QHash &h = pathInfoForHandle[handle]; + QWindowsFileSystemWatcherEngineThread::PathInfoHash &h = pathInfoForHandle[handle]; bool fakeRemove = false; if (!FindNextChangeNotification(handle)) { @@ -381,9 +382,9 @@ void QWindowsFileSystemWatcherEngineThread::run() qErrnoWarning(error, "%s", qPrintable(msgFindNextFailed(h))); } - QMutableHashIterator it(h); + QMutableHashIterator it(h); while (it.hasNext()) { - QHash::iterator x = it.next(); + QWindowsFileSystemWatcherEngineThread::PathInfoHash::iterator x = it.next(); QString absolutePath = x.value().absolutePath; QFileInfo fileInfo(x.value().path); DEBUG() << "checking" << x.key(); @@ -407,7 +408,7 @@ void QWindowsFileSystemWatcherEngineThread::run() Q_ASSERT(indexOfHandle != -1); handles.remove(indexOfHandle); - handleForDir.remove(absolutePath); + handleForDir.remove(QFileSystemWatcherPathKey(absolutePath)); // h is now invalid } } else if (x.value().isDir) { diff --git a/src/corelib/io/qfilesystemwatcher_win_p.h b/src/corelib/io/qfilesystemwatcher_win_p.h index 8937910d10a..790fb954d93 100644 --- a/src/corelib/io/qfilesystemwatcher_win_p.h +++ b/src/corelib/io/qfilesystemwatcher_win_p.h @@ -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 { Q_OBJECT public: + typedef QHash HandleForDirHash; + typedef QHash PathInfoHash; + QWindowsFileSystemWatcherEngineThread(); ~QWindowsFileSystemWatcherEngineThread(); void run(); @@ -143,9 +159,9 @@ public: QVector handles; int msg; - QHash handleForDir; + HandleForDirHash handleForDir; - QHash > pathInfoForHandle; + QHash pathInfoForHandle; Q_SIGNALS: void fileChanged(const QString &path, bool removed); diff --git a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp index a96d8706eee..2756fcce50a 100644 --- a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp +++ b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp @@ -94,20 +94,31 @@ tst_QFileSystemWatcher::tst_QFileSystemWatcher() void tst_QFileSystemWatcher::basicTest_data() { QTest::addColumn("backend"); - QTest::newRow("native backend") << "native"; - QTest::newRow("poller backend") << "poller"; + QTest::addColumn("testFileName"); + 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() { QFETCH(QString, backend); + QFETCH(QString, testFileName); // create test file QTemporaryDir temporaryDirectory(m_tempDirPattern); QVERIFY(temporaryDirectory.isValid()); - QFile testFile(temporaryDirectory.path() + QStringLiteral("/testfile.txt")); - testFile.setPermissions(QFile::ReadOwner | QFile::WriteOwner); - testFile.remove(); + QFile testFile(temporaryDirectory.path() + QLatin1Char('/') + testFileName); QVERIFY(testFile.open(QIODevice::WriteOnly | QIODevice::Truncate)); testFile.write(QByteArray("hello")); testFile.close();