QDir: refactor PathNormalization enum

AllowUncPaths (and the whole enum) is private API that is always enabled
only on Windows (build-time decision). So, remove it and change the
unittests to match reality. Now DefaultNormalization on Windows implies
handling UNC paths.

This was suggested in code review by Thiago; since this is a hot code
path, the goal is letting the compiler keep the flags parameter to
qt_normalizePathSegments() in a register, by keeping the
PathNormalization enum as small as possible.

Drive-by change: since those lines in the unittest are changed anyway,
take the chance and use u""_s syntax.

Change-Id: I3dcf30d888a0ea9f8898e260e65c5f85655296d5
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This commit is contained in:
Ahmad Samir 2024-09-20 00:22:06 +03:00
parent ec894d6945
commit 4f47ee4de4
3 changed files with 84 additions and 94 deletions

View File

@ -51,33 +51,24 @@ static QString driveSpec(const QString &path)
} }
#endif #endif
enum {
#if defined(Q_OS_WIN)
OSSupportsUncPaths = true
#else
OSSupportsUncPaths = false
#endif
};
// Return the length of the root part of an absolute path, for use by cleanPath(), cd(). // Return the length of the root part of an absolute path, for use by cleanPath(), cd().
static qsizetype rootLength(QStringView name, bool allowUncPaths) static qsizetype rootLength(QStringView name)
{ {
#if defined(Q_OS_WIN)
const qsizetype len = name.size(); const qsizetype len = name.size();
// starts with double slash // Handle possible UNC paths which start with double slash
if (allowUncPaths && name.startsWith("//"_L1)) { if (name.startsWith("//"_L1)) {
// Server name '//server/path' is part of the prefix. // Server name '//server/path' is part of the prefix.
const qsizetype nextSlash = name.indexOf(u'/', 2); const qsizetype nextSlash = name.indexOf(u'/', 2);
return nextSlash >= 0 ? nextSlash + 1 : len; return nextSlash >= 0 ? nextSlash + 1 : len;
} }
#if defined(Q_OS_WIN)
if (len >= 2 && name.at(1) == u':') { if (len >= 2 && name.at(1) == u':') {
// Handle a possible drive letter // Handle a possible drive letter
return len > 2 && name.at(2) == u'/' ? 3 : 2; return len > 2 && name.at(2) == u'/' ? 3 : 2;
} }
#endif #endif
if (len && name.at(0) == u'/')
return 1; return name.startsWith(u'/') ? 1 : 0;
return 0;
} }
//************* QDirPrivate //************* QDirPrivate
@ -2227,9 +2218,8 @@ bool QDir::match(const QString &filter, const QString &fileName)
*/ */
bool qt_normalizePathSegments(QString *path, QDirPrivate::PathNormalizations flags) bool qt_normalizePathSegments(QString *path, QDirPrivate::PathNormalizations flags)
{ {
const bool allowUncPaths = flags.testAnyFlag(QDirPrivate::AllowUncPaths);
const bool isRemote = flags.testAnyFlag(QDirPrivate::RemotePath); const bool isRemote = flags.testAnyFlag(QDirPrivate::RemotePath);
const qsizetype prefixLength = rootLength(*path, allowUncPaths); const qsizetype prefixLength = rootLength(*path);
// RFC 3986 says: "The input buffer is initialized with the now-appended // RFC 3986 says: "The input buffer is initialized with the now-appended
// path components and the output buffer is initialized to the empty // path components and the output buffer is initialized to the empty
@ -2372,8 +2362,7 @@ static bool qt_cleanPath(QString *path)
QString &ret = *path; QString &ret = *path;
ret = QDir::fromNativeSeparators(ret); ret = QDir::fromNativeSeparators(ret);
auto normalization = OSSupportsUncPaths ? QDirPrivate::AllowUncPaths : QDirPrivate::DefaultNormalization; bool ok = qt_normalizePathSegments(&ret, QDirPrivate::DefaultNormalization);
bool ok = qt_normalizePathSegments(&ret, normalization);
// Strip away last slash except for root directories // Strip away last slash except for root directories
if (ret.size() > 1 && ret.endsWith(u'/')) { if (ret.size() > 1 && ret.endsWith(u'/')) {

View File

@ -29,7 +29,6 @@ class QDirPrivate : public QSharedData
public: public:
enum PathNormalization { enum PathNormalization {
DefaultNormalization = 0x00, DefaultNormalization = 0x00,
AllowUncPaths = 0x01,
RemotePath = 0x02, RemotePath = 0x02,
}; };
Q_DECLARE_FLAGS(PathNormalizations, PathNormalization) Q_DECLARE_FLAGS(PathNormalizations, PathNormalization)

View File

@ -62,7 +62,6 @@ class tst_QDir : public QObject
Q_OBJECT Q_OBJECT
public: public:
enum UncHandling { HandleUnc, IgnoreUnc };
tst_QDir(); tst_QDir();
private slots: private slots:
@ -222,8 +221,6 @@ private:
}; };
}; };
Q_DECLARE_METATYPE(tst_QDir::UncHandling)
tst_QDir::tst_QDir() tst_QDir::tst_QDir()
#ifdef Q_OS_ANDROID #ifdef Q_OS_ANDROID
: m_dataPath(QStandardPaths::writableLocation(QStandardPaths::CacheLocation)) : m_dataPath(QStandardPaths::writableLocation(QStandardPaths::CacheLocation))
@ -1381,92 +1378,97 @@ tst_QDir::cleanPath()
void tst_QDir::normalizePathSegments_data() void tst_QDir::normalizePathSegments_data()
{ {
QTest::addColumn<QString>("path"); QTest::addColumn<QString>("path");
QTest::addColumn<UncHandling>("uncHandling");
QTest::addColumn<QString>("expected"); QTest::addColumn<QString>("expected");
QTest::newRow("data0") << "/Users/sam/troll/qt4.0//.." << HandleUnc << "/Users/sam/troll/"; QTest::newRow("data0") << u"/Users/sam/troll/qt4.0//.."_s << u"/Users/sam/troll/"_s;
QTest::newRow("data1") << "/Users/sam////troll/qt4.0//.." << HandleUnc << "/Users/sam/troll/"; QTest::newRow("data1") << u"/Users/sam////troll/qt4.0//.."_s << u"/Users/sam/troll/"_s;
QTest::newRow("data53") <<"/b//." << HandleUnc << "/b/"; QTest::newRow("data53") <<u"/b//."_s << u"/b/"_s;
QTest::newRow("data54") <<"/b//./" << HandleUnc << "/b/"; QTest::newRow("data54") <<u"/b//./"_s << u"/b/"_s;
QTest::newRow("data55") <<"/b/." << HandleUnc << "/b/"; QTest::newRow("data55") <<u"/b/."_s << u"/b/"_s;
QTest::newRow("data56") <<"/b/./" << HandleUnc << "/b/"; QTest::newRow("data56") <<u"/b/./"_s << u"/b/"_s;
QTest::newRow("data57") <<"/b" << HandleUnc << "/b"; QTest::newRow("data57") <<u"/b"_s << u"/b"_s;
QTest::newRow("data2") << "/" << HandleUnc << "/"; QTest::newRow("data2") << u"/"_s << u"/"_s;
QTest::newRow("data3") << "//" << HandleUnc << "//"; #if defined(Q_OS_WIN)
QTest::newRow("data4") << "//" << IgnoreUnc << "/"; QTest::newRow("data3") << u"//"_s << u"//"_s;
QTest::newRow("data5") << "/." << HandleUnc << "/";
QTest::newRow("data6") << "/./" << HandleUnc << "/";
QTest::newRow("data7") << "/.." << HandleUnc << "/..";
QTest::newRow("data8") << "/../" << HandleUnc << "/../";
QTest::newRow("/../.") << "/../." << HandleUnc << "/../";
QTest::newRow("/.././") << "/.././" << HandleUnc << "/../";
QTest::newRow("/../..") << "/../.." << HandleUnc << "/../..";
QTest::newRow("data9") << "." << HandleUnc << ".";
QTest::newRow("data10") << "./" << HandleUnc << ".";
QTest::newRow("data11") << "./." << HandleUnc << ".";
QTest::newRow("data12") << "././" << HandleUnc << ".";
QTest::newRow("data13") << ".." << HandleUnc << "..";
QTest::newRow("data14") << "../" << HandleUnc << "../";
QTest::newRow("data15") << "../." << HandleUnc << "../";
QTest::newRow("data16") << ".././" << HandleUnc << "../";
QTest::newRow("data17") << "../.." << HandleUnc << "../..";
QTest::newRow("data18") << "../../" << HandleUnc << "../../";
QTest::newRow("./file1.txt") << "./file1.txt" << HandleUnc << "file1.txt";
QTest::newRow("data19") << ".//file1.txt" << HandleUnc << "file1.txt";
QTest::newRow("/foo/bar//file1.txt") << "/foo/bar//file1.txt" << HandleUnc << "/foo/bar/file1.txt";
QTest::newRow("data20") << "/foo/bar/..//file1.txt" << HandleUnc << "/foo/file1.txt";
QTest::newRow("data21") << "foo/.." << HandleUnc << ".";
QTest::newRow("data22") << "./foo/.." << HandleUnc << ".";
QTest::newRow("data23") << ".foo/.." << HandleUnc << ".";
QTest::newRow("data24") << "foo/bar/../.." << HandleUnc << ".";
QTest::newRow("data25") << "./foo/bar/../.." << HandleUnc << ".";
QTest::newRow("data26") << "../foo/bar" << HandleUnc << "../foo/bar";
QTest::newRow("data27") << "./../foo/bar" << HandleUnc << "../foo/bar";
QTest::newRow("data28") << "../../foo/../bar" << HandleUnc << "../../bar";
QTest::newRow("data29") << "./foo/bar/.././.." << HandleUnc << ".";
QTest::newRow("data30") << "/./foo" << HandleUnc << "/foo";
QTest::newRow("data31") << "/../foo/" << HandleUnc << "/../foo/";
QTest::newRow("data32") << "c:/" << HandleUnc << "c:/";
QTest::newRow("data33") << "c://" << HandleUnc << "c:/";
QTest::newRow("data34") << "c://foo" << HandleUnc << "c:/foo";
QTest::newRow("data35") << "c:" << HandleUnc << "c:";
QTest::newRow("data36") << "c:foo/bar" << IgnoreUnc << "c:foo/bar";
QTest::newRow("data37") << "c:/." << HandleUnc << "c:/";
#if defined Q_OS_WIN
QTest::newRow("data38") << "c:/.." << HandleUnc << "c:/..";
QTest::newRow("data39") << "c:/../" << HandleUnc << "c:/../";
#else #else
QTest::newRow("data38") << "c:/.." << HandleUnc << "."; QTest::newRow("data3") << u"//"_s << u"/"_s;
QTest::newRow("data39") << "c:/../" << HandleUnc << ".";
#endif #endif
QTest::newRow("data40") << "c:/./" << HandleUnc << "c:/"; QTest::newRow("data5") << u"/."_s << u"/"_s;
QTest::newRow("data41") << "foo/../foo/.." << HandleUnc << "."; QTest::newRow("data6") << u"/./"_s << u"/"_s;
QTest::newRow("data42") << "foo/../foo/../.." << HandleUnc << ".."; QTest::newRow("data7") << u"/.."_s << u"/.."_s;
QTest::newRow("data43") << "..foo.bar/foo" << HandleUnc << "..foo.bar/foo"; QTest::newRow("data8") << u"/../"_s << u"/../"_s;
QTest::newRow("data44") << ".foo./bar/.." << HandleUnc << ".foo./"; QTest::newRow("/../.") << u"/../."_s << u"/../"_s;
QTest::newRow("data45") << "foo/..bar.." << HandleUnc << "foo/..bar.."; QTest::newRow("/.././") << u"/.././"_s << u"/../"_s;
QTest::newRow("data46") << "foo/.bar./.." << HandleUnc << "foo/"; QTest::newRow("/../..") << u"/../.."_s << u"/../.."_s;
QTest::newRow("data47") << "//foo//bar" << HandleUnc << "//foo/bar"; QTest::newRow("data9") << u"."_s << u"."_s;
QTest::newRow("data48") << "..." << HandleUnc << "..."; QTest::newRow("data10") << u"./"_s << u"."_s;
QTest::newRow("data49") << "foo/.../bar" << HandleUnc << "foo/.../bar"; QTest::newRow("data11") << u"./."_s << u"."_s;
QTest::newRow("data50") << "ab/a/" << HandleUnc << "ab/a/"; // Path item with length of 2 QTest::newRow("data12") << u"././"_s << u"."_s;
QTest::newRow("data13") << u".."_s << u".."_s;
QTest::newRow("data14") << u"../"_s << u"../"_s;
QTest::newRow("data15") << u"../."_s << u"../"_s;
QTest::newRow("data16") << u".././"_s << u"../"_s;
QTest::newRow("data17") << u"../.."_s << u"../.."_s;
QTest::newRow("data18") << u"../../"_s << u"../../"_s;
QTest::newRow("./file1.txt") << u"./file1.txt"_s << u"file1.txt"_s;
QTest::newRow("data19") << u".//file1.txt"_s << u"file1.txt"_s;
QTest::newRow("/foo/bar//file1.txt") << u"/foo/bar//file1.txt"_s << u"/foo/bar/file1.txt"_s;
QTest::newRow("data20") << u"/foo/bar/..//file1.txt"_s << u"/foo/file1.txt"_s;
QTest::newRow("data21") << u"foo/.."_s << u"."_s;
QTest::newRow("data22") << u"./foo/.."_s << u"."_s;
QTest::newRow("data23") << u".foo/.."_s << u"."_s;
QTest::newRow("data24") << u"foo/bar/../.."_s << u"."_s;
QTest::newRow("data25") << u"./foo/bar/../.."_s << u"."_s;
QTest::newRow("data26") << u"../foo/bar"_s << u"../foo/bar"_s;
QTest::newRow("data27") << u"./../foo/bar"_s << u"../foo/bar"_s;
QTest::newRow("data28") << u"../../foo/../bar"_s << u"../../bar"_s;
QTest::newRow("data29") << u"./foo/bar/.././.."_s << u"."_s;
QTest::newRow("data30") << u"/./foo"_s << u"/foo"_s;
QTest::newRow("data31") << u"/../foo/"_s << u"/../foo/"_s;
QTest::newRow("data32") << u"c:/"_s << u"c:/"_s;
QTest::newRow("data33") << u"c://"_s << u"c:/"_s;
QTest::newRow("data34") << u"c://foo"_s << u"c:/foo"_s;
QTest::newRow("data35") << u"c:"_s << u"c:"_s;
QTest::newRow("data37") << u"c:/."_s << u"c:/"_s;
#if defined Q_OS_WIN
QTest::newRow("data38") << u"c:/.."_s << u"c:/.."_s;
QTest::newRow("data39") << u"c:/../"_s << u"c:/../"_s;
#else
QTest::newRow("data38") << u"c:/.."_s << u"."_s;
QTest::newRow("data39") << u"c:/../"_s << u"."_s;
#endif
QTest::newRow("data40") << u"c:/./"_s << u"c:/"_s;
QTest::newRow("data41") << u"foo/../foo/.."_s << u"."_s;
QTest::newRow("data42") << u"foo/../foo/../.."_s << u".."_s;
QTest::newRow("data43") << u"..foo.bar/foo"_s << u"..foo.bar/foo"_s;
QTest::newRow("data44") << u".foo./bar/.."_s << u".foo./"_s;
QTest::newRow("data45") << u"foo/..bar.."_s << u"foo/..bar.."_s;
QTest::newRow("data46") << u"foo/.bar./.."_s << u"foo/"_s;
#if defined(Q_OS_WIN)
QTest::newRow("data47") << u"//foo//bar"_s << u"//foo/bar"_s;
#else
QTest::newRow("data47") << u"//foo//bar"_s << u"/foo/bar"_s;
#endif
QTest::newRow("data48") << u"..."_s << u"..."_s;
QTest::newRow("data49") << u"foo/.../bar"_s << u"foo/.../bar"_s;
QTest::newRow("data50") << u"ab/a/"_s << u"ab/a/"_s; // Path item with length of 2
#if defined(Q_OS_WIN)
// Drive letters and unc path in one string. The drive letter isn't handled as a drive letter // Drive letters and unc path in one string. The drive letter isn't handled as a drive letter
// but as a host name in this case (even though Windows host names can't contain a ':') // but as a host name in this case (even though Windows host names can't contain a ':')
QTest::newRow("data51") << "//c:/foo" << HandleUnc << "//c:/foo"; QTest::newRow("data51") << u"//c:/foo"_s << u"//c:/foo"_s;
QTest::newRow("data52") << "//c:/foo" << IgnoreUnc << "/c:/foo"; #endif
QTest::newRow("resource0") << ":/prefix/foo.bar" << HandleUnc << ":/prefix/foo.bar"; QTest::newRow("resource0") << u":/prefix/foo.bar"_s << u":/prefix/foo.bar"_s;
QTest::newRow("resource1") << "://prefix/..//prefix/foo.bar" << HandleUnc << ":/prefix/foo.bar"; QTest::newRow("resource1") << u"://prefix/..//prefix/foo.bar"_s << u":/prefix/foo.bar"_s;
} }
void tst_QDir::normalizePathSegments() void tst_QDir::normalizePathSegments()
{ {
QFETCH(QString, path); QFETCH(QString, path);
QFETCH(UncHandling, uncHandling);
QFETCH(QString, expected); QFETCH(QString, expected);
// for QDirPrivate::RemotePath, see tst_QUrl::resolving // for QDirPrivate::RemotePath, see tst_QUrl::resolving
qt_normalizePathSegments(&path, uncHandling == HandleUnc ? QDirPrivate::AllowUncPaths : QDirPrivate::DefaultNormalization); qt_normalizePathSegments(&path, QDirPrivate::DefaultNormalization);
QCOMPARE(path, expected); QCOMPARE(path, expected);
} }
# endif //QT_BUILD_INTERNAL # endif //QT_BUILD_INTERNAL