From 6504bc6bbc9276fe1a26c1af53c202371f334416 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Tue, 4 Jul 2017 11:39:07 -0700 Subject: [PATCH] QFileInfo: harmonize QFileInfo() and QFileInfo("") If a QFileInfo was constructed with an empty path, which could happen with QFileInfo(QFile()) or via QDir, etc., then it would issue system calls to empty paths and could even produce warnings. This commit makes am empty path name be the same as a default-constructed QFileInfo and corrects the use if 0 for ownerId and groupId to match the documentation. [ChangeLog][Important Behavior Changes] QFileInfo on empty strings now behaves like the default-constructed QFileInfo. Notably, path() will now be the empty string too, instead of ".", which means absoluteFilePath() is no longer the current working directory. Change-Id: I8d96dea9955d4c749b99fffd14ce34968b1d9bbf Reviewed-by: Simon Hausmann --- src/corelib/io/qfileinfo.cpp | 193 +++++++----------- src/corelib/io/qfileinfo_p.h | 23 ++- src/corelib/io/qfilesystemengine.cpp | 4 + .../corelib/io/qfileinfo/tst_qfileinfo.cpp | 139 ++++++++----- 4 files changed, 182 insertions(+), 177 deletions(-) diff --git a/src/corelib/io/qfileinfo.cpp b/src/corelib/io/qfileinfo.cpp index 104a171e999..235e391aa0e 100644 --- a/src/corelib/io/qfileinfo.cpp +++ b/src/corelib/io/qfileinfo.cpp @@ -591,9 +591,6 @@ QString QFileInfo::absolutePath() const if (d->isDefaultConstructed) { return QLatin1String(""); - } else if (d->fileEntry.isEmpty()) { - qWarning("QFileInfo::absolutePath: Constructed with empty filename"); - return QLatin1String(""); } return d->getFileName(QAbstractFileEngine::AbsolutePathName); } @@ -913,14 +910,10 @@ QDir QFileInfo::absoluteDir() const bool QFileInfo::isReadable() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::UserReadPermission)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::UserReadPermission); - return (d->metaData.permissions() & QFile::ReadUser) != 0; - } - return d->getFileFlags(QAbstractFileEngine::ReadUserPerm); + return d->checkAttribute( + QFileSystemMetaData::UserReadPermission, + [d]() { return (d->metaData.permissions() & QFile::ReadUser) != 0; }, + [d]() { return d->getFileFlags(QAbstractFileEngine::ReadUserPerm); }); } /*! @@ -934,14 +927,10 @@ bool QFileInfo::isReadable() const bool QFileInfo::isWritable() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::UserWritePermission)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::UserWritePermission); - return (d->metaData.permissions() & QFile::WriteUser) != 0; - } - return d->getFileFlags(QAbstractFileEngine::WriteUserPerm); + return d->checkAttribute( + QFileSystemMetaData::UserWritePermission, + [d]() { return (d->metaData.permissions() & QFile::WriteUser) != 0; }, + [d]() { return d->getFileFlags(QAbstractFileEngine::WriteUserPerm); }); } /*! @@ -952,14 +941,10 @@ bool QFileInfo::isWritable() const bool QFileInfo::isExecutable() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::UserExecutePermission)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::UserExecutePermission); - return (d->metaData.permissions() & QFile::ExeUser) != 0; - } - return d->getFileFlags(QAbstractFileEngine::ExeUserPerm); + return d->checkAttribute( + QFileSystemMetaData::UserExecutePermission, + [d]() { return (d->metaData.permissions() & QFile::ExeUser) != 0; }, + [d]() { return d->getFileFlags(QAbstractFileEngine::ExeUserPerm); }); } /*! @@ -971,14 +956,10 @@ bool QFileInfo::isExecutable() const bool QFileInfo::isHidden() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::HiddenAttribute)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::HiddenAttribute); - return d->metaData.isHidden(); - } - return d->getFileFlags(QAbstractFileEngine::HiddenFlag); + return d->checkAttribute( + QFileSystemMetaData::HiddenAttribute, + [d]() { return d->metaData.isHidden(); }, + [d]() { return d->getFileFlags(QAbstractFileEngine::HiddenFlag); }); } /*! @@ -1014,14 +995,10 @@ bool QFileInfo::isNativePath() const bool QFileInfo::isFile() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::FileType)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::FileType); - return d->metaData.isFile(); - } - return d->getFileFlags(QAbstractFileEngine::FileType); + return d->checkAttribute( + QFileSystemMetaData::FileType, + [d]() { return d->metaData.isFile(); }, + [d]() { return d->getFileFlags(QAbstractFileEngine::FileType); }); } /*! @@ -1033,14 +1010,10 @@ bool QFileInfo::isFile() const bool QFileInfo::isDir() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::DirectoryType)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::DirectoryType); - return d->metaData.isDirectory(); - } - return d->getFileFlags(QAbstractFileEngine::DirectoryType); + return d->checkAttribute( + QFileSystemMetaData::DirectoryType, + [d]() { return d->metaData.isDirectory(); }, + [d]() { return d->getFileFlags(QAbstractFileEngine::DirectoryType); }); } @@ -1054,14 +1027,10 @@ bool QFileInfo::isDir() const bool QFileInfo::isBundle() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::BundleType)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::BundleType); - return d->metaData.isBundle(); - } - return d->getFileFlags(QAbstractFileEngine::BundleType); + return d->checkAttribute( + QFileSystemMetaData::BundleType, + [d]() { return d->metaData.isBundle(); }, + [d]() { return d->getFileFlags(QAbstractFileEngine::BundleType); }); } /*! @@ -1084,14 +1053,10 @@ bool QFileInfo::isBundle() const bool QFileInfo::isSymLink() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::LegacyLinkType)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::LegacyLinkType); - return d->metaData.isLegacyLink(); - } - return d->getFileFlags(QAbstractFileEngine::LinkType); + return d->checkAttribute( + QFileSystemMetaData::LegacyLinkType, + [d]() { return d->metaData.isLegacyLink(); }, + [d]() { return d->getFileFlags(QAbstractFileEngine::LinkType); }); } /*! @@ -1103,7 +1068,7 @@ bool QFileInfo::isRoot() const { Q_D(const QFileInfo); if (d->isDefaultConstructed) - return true; + return false; if (d->fileEngine == 0) { if (d->fileEntry.isRoot()) { #if defined(Q_OS_WIN) && !defined(Q_OS_WINRT) @@ -1179,14 +1144,10 @@ QString QFileInfo::owner() const uint QFileInfo::ownerId() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return 0; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::UserId)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::UserId); - return d->metaData.userId(); - } - return d->fileEngine->ownerId(QAbstractFileEngine::OwnerUser); + return d->checkAttribute(uint(-2), + QFileSystemMetaData::UserId, + [d]() { return d->metaData.userId(); }, + [d]() { return d->fileEngine->ownerId(QAbstractFileEngine::OwnerUser); }); } /*! @@ -1218,14 +1179,10 @@ QString QFileInfo::group() const uint QFileInfo::groupId() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return 0; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::GroupId)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::GroupId); - return d->metaData.groupId(); - } - return d->fileEngine->ownerId(QAbstractFileEngine::OwnerGroup); + return d->checkAttribute(uint(-2), + QFileSystemMetaData::GroupId, + [d]() { return d->metaData.groupId(); }, + [d]() { return d->fileEngine->ownerId(QAbstractFileEngine::OwnerGroup); }); } /*! @@ -1247,16 +1204,15 @@ uint QFileInfo::groupId() const bool QFileInfo::permission(QFile::Permissions permissions) const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return false; - if (d->fileEngine == 0) { - // the QFileSystemMetaData::MetaDataFlag and QFile::Permissions overlap, so just static cast. - QFileSystemMetaData::MetaDataFlag permissionFlags = static_cast((int)permissions); - if (!d->cache_enabled || !d->metaData.hasFlags(permissionFlags)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, permissionFlags); - return (d->metaData.permissions() & permissions) == permissions; - } - return d->getFileFlags(QAbstractFileEngine::FileFlags((int)permissions)) == (uint)permissions; + // the QFileSystemMetaData::MetaDataFlag and QFile::Permissions overlap, so just cast. + auto fseFlags = QFileSystemMetaData::MetaDataFlag(int(permissions)); + auto feFlags = QAbstractFileEngine::FileFlags(int(permissions)); + return d->checkAttribute( + fseFlags, + [=]() { return (d->metaData.permissions() & permissions) == permissions; }, + [=]() { + return d->getFileFlags(feFlags) == uint(permissions); + }); } /*! @@ -1269,14 +1225,12 @@ bool QFileInfo::permission(QFile::Permissions permissions) const QFile::Permissions QFileInfo::permissions() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return 0; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::Permissions)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::Permissions); - return d->metaData.permissions(); - } - return QFile::Permissions(d->getFileFlags(QAbstractFileEngine::PermsMask) & QAbstractFileEngine::PermsMask); + return d->checkAttribute( + QFileSystemMetaData::Permissions, + [d]() { return d->metaData.permissions(); }, + [d]() { + return QFile::Permissions(d->getFileFlags(QAbstractFileEngine::PermsMask) & QAbstractFileEngine::PermsMask); + }); } @@ -1289,18 +1243,16 @@ QFile::Permissions QFileInfo::permissions() const qint64 QFileInfo::size() const { Q_D(const QFileInfo); - if (d->isDefaultConstructed) - return 0; - if (d->fileEngine == 0) { - if (!d->cache_enabled || !d->metaData.hasFlags(QFileSystemMetaData::SizeAttribute)) - QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, QFileSystemMetaData::SizeAttribute); - return d->metaData.size(); - } - if (!d->getCachedFlag(QFileInfoPrivate::CachedSize)) { - d->setCachedFlag(QFileInfoPrivate::CachedSize); - d->fileSize = d->fileEngine->size(); - } - return d->fileSize; + return d->checkAttribute( + QFileSystemMetaData::SizeAttribute, + [d]() { return d->metaData.size(); }, + [d]() { + if (!d->getCachedFlag(QFileInfoPrivate::CachedSize)) { + d->setCachedFlag(QFileInfoPrivate::CachedSize); + d->fileSize = d->fileEngine->size(); + } + return d->fileSize; + }); } #if QT_DEPRECATED_SINCE(5, 10) @@ -1394,11 +1346,6 @@ QDateTime QFileInfo::fileTime(QFile::FileTime time) const Q_D(const QFileInfo); auto fetime = QAbstractFileEngine::FileTime(time); - if (d->isDefaultConstructed) - return QDateTime(); - if (d->fileEngine) - return d->getFileTime(fetime).toLocalTime(); - QFileSystemMetaData::MetaDataFlags flag; switch (time) { case QFile::FileAccessTime: @@ -1415,10 +1362,10 @@ QDateTime QFileInfo::fileTime(QFile::FileTime time) const break; } - if (!d->cache_enabled || !d->metaData.hasFlags(flag)) - if (!QFileSystemEngine::fillMetaData(d->fileEntry, d->metaData, flag)) - return QDateTime(); - return d->metaData.fileTime(fetime).toLocalTime(); + return d->checkAttribute( + flag, + [=]() { return d->metaData.fileTime(fetime).toLocalTime(); }, + [=]() { return d->getFileTime(fetime).toLocalTime(); }); } /*! diff --git a/src/corelib/io/qfileinfo_p.h b/src/corelib/io/qfileinfo_p.h index 923247c62c4..e4b28f4519d 100644 --- a/src/corelib/io/qfileinfo_p.h +++ b/src/corelib/io/qfileinfo_p.h @@ -103,7 +103,7 @@ public: fileEngine(QFileSystemEngine::resolveEntryAndCreateLegacyEngine(fileEntry, metaData)), cachedFlags(0), #ifndef QT_NO_FSFILEENGINE - isDefaultConstructed(false), + isDefaultConstructed(file.isEmpty()), #else isDefaultConstructed(!fileEngine), #endif @@ -179,6 +179,27 @@ public: inline void setCachedFlag(uint c) const { if (cache_enabled) cachedFlags |= c; } + template + Ret checkAttribute(Ret defaultValue, QFileSystemMetaData::MetaDataFlags fsFlags, const FSLambda &fsLambda, + const EngineLambda &engineLambda) const + { + if (isDefaultConstructed) + return defaultValue; + if (fileEngine) + return engineLambda(); + if (!cache_enabled || !metaData.hasFlags(fsFlags)) { + QFileSystemEngine::fillMetaData(fileEntry, metaData, fsFlags); + // ignore errors, fillMetaData will have cleared the flags + } + return fsLambda(); + } + + template + Ret checkAttribute(QFileSystemMetaData::MetaDataFlags fsFlags, const FSLambda &fsLambda, + const EngineLambda &engineLambda) const + { + return checkAttribute(Ret(), fsFlags, fsLambda, engineLambda); + } }; QT_END_NAMESPACE diff --git a/src/corelib/io/qfilesystemengine.cpp b/src/corelib/io/qfilesystemengine.cpp index 955b459bba0..73a2e37a384 100644 --- a/src/corelib/io/qfilesystemengine.cpp +++ b/src/corelib/io/qfilesystemengine.cpp @@ -213,6 +213,8 @@ QString QFileSystemEngine::resolveUserName(const QFileSystemEntry &entry, QFileS #else //(Q_OS_UNIX) if (!metaData.hasFlags(QFileSystemMetaData::UserId)) QFileSystemEngine::fillMetaData(entry, metaData, QFileSystemMetaData::UserId); + if (!metaData.exists()) + return QString(); return resolveUserName(metaData.userId()); #endif } @@ -226,6 +228,8 @@ QString QFileSystemEngine::resolveGroupName(const QFileSystemEntry &entry, QFile #else //(Q_OS_UNIX) if (!metaData.hasFlags(QFileSystemMetaData::GroupId)) QFileSystemEngine::fillMetaData(entry, metaData, QFileSystemMetaData::GroupId); + if (!metaData.exists()) + return QString(); return resolveGroupName(metaData.groupId()); #endif } diff --git a/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp b/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp index 94273e2394b..3fa765e2faa 100644 --- a/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp +++ b/tests/auto/corelib/io/qfileinfo/tst_qfileinfo.cpp @@ -232,8 +232,9 @@ private slots: #endif void group(); + void invalidState_data(); void invalidState(); - void nonExistingFileDates(); + void nonExistingFile(); private: const QString m_currentDir; @@ -1366,7 +1367,7 @@ void tst_QFileInfo::isNativePath_data() QTest::addColumn("isNativePath"); QTest::newRow("default-constructed") << QString() << false; - QTest::newRow("empty") << QString("") << true; + QTest::newRow("empty") << QString("") << false; QTest::newRow("local root") << QString::fromLatin1("/") << true; QTest::newRow("local non-existent file") << QString::fromLatin1("/abrakadabra.boo") << true; @@ -1896,64 +1897,96 @@ void tst_QFileInfo::group() QCOMPARE(fi.group(), expected); } -void tst_QFileInfo::invalidState() +static void stateCheck(const QFileInfo &info, const QString &dirname, const QString &filename) { - // Shouldn't crash; - - { - QFileInfo info; - QCOMPARE(info.size(), qint64(0)); - QVERIFY(!info.exists()); - - info.setCaching(false); - - info.created(); - info.birthTime(); - info.metadataChangeTime(); - info.lastRead(); - info.lastModified(); - } - - { - QFileInfo info(""); - QCOMPARE(info.size(), qint64(0)); - QVERIFY(!info.exists()); - - info.setCaching(false); - - info.created(); - info.birthTime(); - info.metadataChangeTime(); - info.lastRead(); - info.lastModified(); - } - - { - QFileInfo info("file-doesn't-really-exist.txt"); - QCOMPARE(info.size(), qint64(0)); - QVERIFY(!info.exists()); - - info.setCaching(false); - - info.created(); - info.birthTime(); - info.metadataChangeTime(); - info.lastRead(); - info.lastModified(); - } - - QVERIFY(true); -} - -void tst_QFileInfo::nonExistingFileDates() -{ - QFileInfo info("non-existing-file.foobar"); + QCOMPARE(info.size(), qint64(0)); QVERIFY(!info.exists()); + + QString path; + QString abspath; + if (!dirname.isEmpty()) { + path = "."; + abspath = dirname + '/' + filename; + } + + QCOMPARE(info.filePath(), filename); + QCOMPARE(info.absoluteFilePath(), abspath); + QCOMPARE(info.canonicalFilePath(), QString()); + QCOMPARE(info.fileName(), filename); + QCOMPARE(info.baseName(), filename); + QCOMPARE(info.completeBaseName(), filename); + QCOMPARE(info.suffix(), QString()); + QCOMPARE(info.bundleName(), QString()); + QCOMPARE(info.completeSuffix(), QString()); + + QVERIFY(info.isRelative()); + QCOMPARE(info.path(), path); + QCOMPARE(info.absolutePath(), dirname); + QCOMPARE(info.dir().path(), "."); + + // these don't look right + QCOMPARE(info.canonicalPath(), path); + QCOMPARE(info.absoluteDir().path(), dirname.isEmpty() ? "." : dirname); + + QVERIFY(!info.isReadable()); + QVERIFY(!info.isWritable()); + QVERIFY(!info.isExecutable()); + QVERIFY(!info.isHidden()); + QVERIFY(!info.isFile()); + QVERIFY(!info.isDir()); + QVERIFY(!info.isSymLink()); + QVERIFY(!info.isBundle()); + QVERIFY(!info.isRoot()); + QCOMPARE(info.isNativePath(), !filename.isEmpty()); + + QCOMPARE(info.readLink(), QString()); + QCOMPARE(info.ownerId(), uint(-2)); + QCOMPARE(info.groupId(), uint(-2)); + QCOMPARE(info.owner(), QString()); + QCOMPARE(info.group(), QString()); + + QCOMPARE(info.permissions(), QFile::Permissions()); + QVERIFY(!info.created().isValid()); QVERIFY(!info.birthTime().isValid()); QVERIFY(!info.metadataChangeTime().isValid()); QVERIFY(!info.lastRead().isValid()); QVERIFY(!info.lastModified().isValid()); +}; + +void tst_QFileInfo::invalidState_data() +{ + QTest::addColumn("mode"); + QTest::newRow("default") << 0; + QTest::newRow("empty") << 1; + QTest::newRow("copy-of-default") << 2; + QTest::newRow("copy-of-empty") << 3; +} + +void tst_QFileInfo::invalidState() +{ + // Shouldn't crash or produce warnings + QFETCH(int, mode); + const QFileInfo &info = (mode & 1 ? QFileInfo("") : QFileInfo()); + + if (mode & 2) { + QFileInfo copy(info); + stateCheck(copy, QString(), QString()); + } else { + stateCheck(info, QString(), QString()); + } +} + +void tst_QFileInfo::nonExistingFile() +{ + QString dirname = QDir::currentPath(); + QString cdirname = QFileInfo(dirname).canonicalFilePath(); + if (dirname != cdirname) + QDir::setCurrent(cdirname); // chdir() to our canonical path + + QString filename = "non-existing-file-foobar"; + QFileInfo info(filename); + stateCheck(info, dirname, filename); } QTEST_MAIN(tst_QFileInfo)