From cffe6800d1db6ef810715d884ee20c07e388fef9 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 7 Feb 2023 09:37:10 -0800 Subject: QFileSystemWatcher/Win: remove the pre-QFileInfo path normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit It's completely unnecessary, since QFileInfo will query the file system anyway and that has a much better view of what is normalized and what isn't. More importantly, this fixes the mistake in failing to normalize properly in removePaths(), which removed the ending slash of a root directory such as "C:\\". That caused the path to become "C:", which QFileInfo interprets as "current path on drive C:". [ChangeLog][QtCore][QFileSystemWatcher] Fixed a bug that prevented removePaths() from removing the root of a drive on Windows. Fixes: QTBUG-110986 Change-Id: I9671dee8ceb64aa9b9cafffd17419b6d69670876 Reviewed-by: Mårten Nordheim (cherry picked from commit 6d1769791348e9387e0c5f29d970131895888814) Reviewed-by: Qt Cherry-pick Bot --- src/corelib/io/qfilesystemwatcher_win.cpp | 16 ++++------------ .../io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp | 18 ++++++++++++++++-- 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/src/corelib/io/qfilesystemwatcher_win.cpp b/src/corelib/io/qfilesystemwatcher_win.cpp index 7586054c27..a0b2b006d6 100644 --- a/src/corelib/io/qfilesystemwatcher_win.cpp +++ b/src/corelib/io/qfilesystemwatcher_win.cpp @@ -332,12 +332,7 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths, QStringList unhandled; for (const QString &path : paths) { auto sg = qScopeGuard([&] { unhandled.push_back(path); }); - QString normalPath = path; - if ((normalPath.endsWith(u'/') && !normalPath.endsWith(":/"_L1)) - || (normalPath.endsWith(u'\\') && !normalPath.endsWith(":\\"_L1))) { - normalPath.chop(1); - } - QFileInfo fileInfo(normalPath); + QFileInfo fileInfo(path); fileInfo.stat(); if (!fileInfo.exists()) continue; @@ -351,7 +346,7 @@ QStringList QWindowsFileSystemWatcherEngine::addPaths(const QStringList &paths, continue; } - DEBUG() << "Looking for a thread/handle for" << normalPath; + DEBUG() << "Looking for a thread/handle for" << fileInfo.path(); const QString absolutePath = isDir ? fileInfo.absoluteFilePath() : fileInfo.absolutePath(); const uint flags = isDir @@ -495,11 +490,8 @@ QStringList QWindowsFileSystemWatcherEngine::removePaths(const QStringList &path QStringList unhandled; for (const QString &path : paths) { auto sg = qScopeGuard([&] { unhandled.push_back(path); }); - QString normalPath = path; - if (normalPath.endsWith(u'/') || normalPath.endsWith(u'\\')) - normalPath.chop(1); - QFileInfo fileInfo(normalPath); - DEBUG() << "removing" << normalPath; + QFileInfo fileInfo(path); + DEBUG() << "removing" << fileInfo.path(); QString absolutePath = fileInfo.absoluteFilePath(); QList::iterator jt, end; end = threads.end(); diff --git a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp index 5acab6e473..dfdf4e05c1 100644 --- a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp +++ b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp @@ -396,8 +396,14 @@ void tst_QFileSystemWatcher::addPaths() QFileSystemWatcher watcher; QStringList paths; paths << QDir::homePath() << QDir::tempPath(); +#ifndef Q_OS_QNX + // Adding this makes QNX fail and we haven't investigated why + for (const QFileInfo &fi : QDir::drives()) + paths << fi.absoluteFilePath(); // on Unix, this will be just "/" +#endif + QCOMPARE(watcher.addPaths(paths), QStringList()); - QCOMPARE(watcher.directories().size(), 2); + QCOMPARE(watcher.directories().size(), paths.size()); // With empty list paths.clear(); @@ -464,8 +470,14 @@ void tst_QFileSystemWatcher::removePaths() QFileSystemWatcher watcher; QStringList paths; paths << QDir::homePath() << QDir::tempPath(); +#ifndef Q_OS_QNX + // Adding this makes QNX fail and we haven't investigated why + for (const QFileInfo &fi : QDir::drives()) + paths << fi.absoluteFilePath(); // on Unix, this will be just "/" +#endif + QCOMPARE(watcher.addPaths(paths), QStringList()); - QCOMPARE(watcher.directories().size(), 2); + QCOMPARE(watcher.directories().size(), paths.size()); QCOMPARE(watcher.removePaths(paths), QStringList()); QCOMPARE(watcher.directories().size(), 0); @@ -613,9 +625,11 @@ void tst_QFileSystemWatcher::nonExistingFile() QStringList() << "../..//./does-not-exist"); // empty path is not actually a failure + QTest::ignoreMessage(QtWarningMsg, "QFileSystemWatcher::addPaths: list is empty"); QCOMPARE(watcher.addPaths(QStringList() << QString()), QStringList()); // empty path is not actually a failure + QTest::ignoreMessage(QtWarningMsg, "QFileSystemWatcher::removePaths: list is empty"); QCOMPARE(watcher.removePaths(QStringList() << QString()), QStringList()); } -- cgit v1.2.1