From ed19fc05313d338059a34304522d8e3a1932dc03 Mon Sep 17 00:00:00 2001 From: Marc Mutz Date: Sun, 19 May 2019 09:59:24 +0200 Subject: [PATCH] QFileSystemWatcher: lock autotest code away into a cold section MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The code contained a sizeable chunk of string parsing along with qDebug()s in the normal path of execution. That code, however, was only used for Qt's own autotests. The idea of this patch is, then, to not only move the autotest case into the cold text section (using Q_UNLIKELY), but also to completely exclude it, when QT_BUILD_INTERNAL is not set. Unfortunately, the structure of the function did not really lend itself to #ifdefing that part of the code out (production code was in the middle of non-production code), so I transformed the engine selection code into a lambda, replacing assignment with returns, and swapping the branches of the central if around to yield a single block of code that can be excluded from compilation with just one #ifdef. As a consequence, the runtime code is almost unaffected, and the function is much easier to read now. Since the test-specific code is only compiled into Qt now in developer builds, guard the tests that rely on this behavior with the same macro. Change-Id: I9fd1c57020a13cef4cd1b1674ed2d3ab9424d7cd Reviewed-by: Mårten Nordheim --- src/corelib/io/qfilesystemwatcher.cpp | 41 ++++++++++--------- .../tst_qfilesystemwatcher.cpp | 12 ++++++ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/corelib/io/qfilesystemwatcher.cpp b/src/corelib/io/qfilesystemwatcher.cpp index f40e166d9f..9b47b27f5c 100644 --- a/src/corelib/io/qfilesystemwatcher.cpp +++ b/src/corelib/io/qfilesystemwatcher.cpp @@ -352,33 +352,34 @@ QStringList QFileSystemWatcher::addPaths(const QStringList &paths) return QStringList(); } - QFileSystemWatcherEngine *engine = 0; + const auto selectEngine = [this, d]() -> QFileSystemWatcherEngine* { +#ifdef QT_BUILD_INTERNAL + const QString on = objectName(); - const QString on = objectName(); - - if (!on.startsWith(QLatin1String("_qt_autotest_force_engine_"))) { + if (Q_UNLIKELY(on.startsWith(QLatin1String("_qt_autotest_force_engine_")))) { + // Autotest override case - use the explicitly selected engine only + const QStringRef forceName = on.midRef(26); + if (forceName == QLatin1String("poller")) { + qDebug("QFileSystemWatcher: skipping native engine, using only polling engine"); + d_func()->initPollerEngine(); + return d->poller; + } else if (forceName == QLatin1String("native")) { + qDebug("QFileSystemWatcher: skipping polling engine, using only native engine"); + return d->native; + } + return nullptr; + } +#endif // Normal runtime case - search intelligently for best engine if(d->native) { - engine = d->native; + return d->native; } else { d_func()->initPollerEngine(); - engine = d->poller; + return d->poller; } + }; - } else { - // Autotest override case - use the explicitly selected engine only - const QStringRef forceName = on.midRef(26); - if(forceName == QLatin1String("poller")) { - qDebug("QFileSystemWatcher: skipping native engine, using only polling engine"); - d_func()->initPollerEngine(); - engine = d->poller; - } else if(forceName == QLatin1String("native")) { - qDebug("QFileSystemWatcher: skipping polling engine, using only native engine"); - engine = d->native; - } - } - - if(engine) + if (auto engine = selectEngine()) p = engine->addPaths(p, &d->files, &d->directories); return p; diff --git a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp index 67ffa91e57..cdd1f6361e 100644 --- a/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp +++ b/tests/auto/corelib/io/qfilesystemwatcher/tst_qfilesystemwatcher.cpp @@ -46,11 +46,13 @@ public: tst_QFileSystemWatcher(); private slots: +#ifdef QT_BUILD_INTERNAL void basicTest_data(); void basicTest(); void watchDirectory_data(); void watchDirectory(); +#endif void addPath(); void removePath(); @@ -58,8 +60,10 @@ private slots: void removePaths(); void removePathsFilesInSameDirectory(); +#ifdef QT_BUILD_INTERNAL void watchFileAndItsDirectory_data() { basicTest_data(); } void watchFileAndItsDirectory(); +#endif void nonExistingFile(); @@ -67,8 +71,10 @@ private slots: void destroyAfterQCoreApplication(); +#ifdef QT_BUILD_INTERNAL void QTBUG2331(); void QTBUG2331_data() { basicTest_data(); } +#endif void signalsEmittedAfterFileMoved(); @@ -90,6 +96,7 @@ tst_QFileSystemWatcher::tst_QFileSystemWatcher() #endif } +#ifdef QT_BUILD_INTERNAL void tst_QFileSystemWatcher::basicTest_data() { QTest::addColumn("backend"); @@ -360,6 +367,7 @@ void tst_QFileSystemWatcher::watchDirectory() for (const auto &testDirName : testDirs) QVERIFY(temporaryDir.rmdir(testDirName)); } +#endif // QT_BUILD_INTERNAL void tst_QFileSystemWatcher::addPath() { @@ -502,6 +510,7 @@ void tst_QFileSystemWatcher::removePathsFilesInSameDirectory() QCOMPARE(watcher.files().size(), 0); } +#ifdef QT_BUILD_INTERNAL static QByteArray msgFileOperationFailed(const char *what, const QFile &f) { return what + QByteArrayLiteral(" failed on \"") @@ -601,6 +610,7 @@ void tst_QFileSystemWatcher::watchFileAndItsDirectory() QVERIFY(temporaryDir.rmdir(testDirName)); } +#endif // QT_BUILD_INTERNAL void tst_QFileSystemWatcher::nonExistingFile() { @@ -673,6 +683,7 @@ void tst_QFileSystemWatcher::destroyAfterQCoreApplication() QTest::qWait(30); } +#ifdef QT_BUILD_INTERNAL // regression test for QTBUG2331. // essentially, on windows, directories were not unwatched after being deleted // from the disk, causing all sorts of interesting problems. @@ -696,6 +707,7 @@ void tst_QFileSystemWatcher::QTBUG2331() QTRY_COMPARE(changedSpy.count(), 1); QCOMPARE(watcher.directories(), QStringList()); } +#endif // QT_BUILD_INTERNAL class SignalReceiver : public QObject {