From ead72a1155d6986e53b6f3025a262e04e822979e Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Mon, 11 Mar 2024 20:42:51 +0200 Subject: [PATCH] QFileSystemEngine: add rmpath() removeDirectory() acts in two mode, rmdir (one entry) and rmpath (the entry and all empty parent directories). This is irregular behavior, and API with a very specific use-case (in unittests, where you create a dir tree and want to cleanup after the test finishes). So, split the code into rmdir() and rmpath(), which matches QDir::rmdir() and QDir::rmpath(). On Unix, a further optimization pointed out by Thiago in review, remove the stat() call, ::rmdir() will fail with ENOTDIR if we try to remove anything that isn't a dir. Change-Id: I1bbb6e6c1ce49ba6d73d3c510b449223498612fb Reviewed-by: Thiago Macieira --- src/corelib/io/qdir.cpp | 4 +-- src/corelib/io/qfilesystemengine_p.h | 10 +++++- src/corelib/io/qfilesystemengine_unix.cpp | 38 ++++++++++----------- src/corelib/io/qfilesystemengine_win.cpp | 41 +++++++++++++---------- 4 files changed, 52 insertions(+), 41 deletions(-) diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp index efad270cf82..cafd7ecea68 100644 --- a/src/corelib/io/qdir.cpp +++ b/src/corelib/io/qdir.cpp @@ -1552,7 +1552,7 @@ bool QDir::rmdir(const QString &dirName) const QString fn = filePath(dirName); if (!d->fileEngine) - return QFileSystemEngine::removeDirectory(QFileSystemEntry(fn), false); + return QFileSystemEngine::rmdir(QFileSystemEntry(fn)); return d->fileEngine->rmdir(fn, false); } @@ -1606,7 +1606,7 @@ bool QDir::rmpath(const QString &dirPath) const QString fn = filePath(dirPath); if (!d->fileEngine) - return QFileSystemEngine::removeDirectory(QFileSystemEntry(fn), true); + return QFileSystemEngine::rmpath(QFileSystemEntry(fn)); return d->fileEngine->rmdir(fn, true); } diff --git a/src/corelib/io/qfilesystemengine_p.h b/src/corelib/io/qfilesystemengine_p.h index d6eb064dde0..000d52805e8 100644 --- a/src/corelib/io/qfilesystemengine_p.h +++ b/src/corelib/io/qfilesystemengine_p.h @@ -117,7 +117,15 @@ public: static bool createDirectory(const QFileSystemEntry &entry, bool createParents, std::optional permissions = std::nullopt); - static bool removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents); + static bool removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents) + { + if (removeEmptyParents) + return rmpath(entry); + return rmdir(entry); + } + + static bool rmdir(const QFileSystemEntry &entry); + static bool rmpath(const QFileSystemEntry &entry); static bool createLink(const QFileSystemEntry &source, const QFileSystemEntry &target, QSystemError &error); diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp index c3042b34e22..627447ec381 100644 --- a/src/corelib/io/qfilesystemengine_unix.cpp +++ b/src/corelib/io/qfilesystemengine_unix.cpp @@ -1189,29 +1189,27 @@ bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool crea return createDirectoryWithParents(dirName, mode, false); } -//static -bool QFileSystemEngine::removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents) +bool QFileSystemEngine::rmdir(const QFileSystemEntry &entry) { - Q_CHECK_FILE_NAME(entry, false); + const QByteArray path = entry.nativeFilePath(); + Q_CHECK_FILE_NAME(path, false); + return ::rmdir(path.constData()) == 0; +} - if (removeEmptyParents) { - QString dirName = QDir::cleanPath(entry.filePath()); - for (qsizetype oldslash = 0, slash=dirName.size(); slash > 0; oldslash = slash) { - const QByteArray chunk = QFile::encodeName(dirName.left(slash)); - QT_STATBUF st; - if (QT_STAT(chunk.constData(), &st) != -1) { - if ((st.st_mode & S_IFMT) != S_IFDIR) - return false; - if (::rmdir(chunk.constData()) != 0) - return oldslash != 0; - } else { - return false; - } - slash = dirName.lastIndexOf(QDir::separator(), oldslash-1); - } - return true; +bool QFileSystemEngine::rmpath(const QFileSystemEntry &entry) +{ + const QString path = QDir::cleanPath(entry.filePath()); + Q_CHECK_FILE_NAME(path, false); + + for (qsizetype oldslash = 0, slash = path.size(); slash > 0; oldslash = slash) { + const QByteArray chunk = QFile::encodeName(path.left(slash)); + if (::rmdir(chunk.constData()) != 0) + return oldslash != 0; + + slash = path.lastIndexOf(QDir::separator(), oldslash - 1); } - return rmdir(QFile::encodeName(entry.filePath()).constData()) == 0; + + return true; } //static diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp index 07384cd7598..8d4e5d95f04 100644 --- a/src/corelib/io/qfilesystemengine_win.cpp +++ b/src/corelib/io/qfilesystemengine_win.cpp @@ -1554,30 +1554,35 @@ bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool crea return createDirectoryWithParents(dirName, securityAttributes, false); } -//static -bool QFileSystemEngine::removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents) +bool QFileSystemEngine::rmdir(const QFileSystemEntry &entry) { QString dirName = entry.filePath(); Q_CHECK_FILE_NAME(dirName, false); - if (removeEmptyParents) { - dirName = QDir::toNativeSeparators(QDir::cleanPath(dirName)); - for (int oldslash = 0, slash=dirName.length(); slash > 0; oldslash = slash) { - const auto chunkRef = QStringView{dirName}.left(slash); - if (chunkRef.length() == 2 && chunkRef.at(0).isLetter() - && chunkRef.at(1) == u':') { - break; - } - const QString chunk = chunkRef.toString(); - if (!isDirPath(chunk, nullptr)) - return false; - if (!rmDir(chunk)) - return oldslash != 0; - slash = dirName.lastIndexOf(QDir::separator(), oldslash-1); + return rmDir(dirName); +} + +bool QFileSystemEngine::rmpath(const QFileSystemEntry &entry) +{ + const QString dirName = QDir::toNativeSeparators(QDir::cleanPath(entry.filePath())); + Q_CHECK_FILE_NAME(dirName, false); + + for (int oldslash = 0, slash = dirName.size(); slash > 0; oldslash = slash) { + const auto chunkRef = QStringView{dirName}.left(slash); + if (chunkRef.length() == 2 && chunkRef.at(0).isLetter() + && chunkRef.at(1) == u':') { + break; } - return true; + const QString chunk = chunkRef.toString(); + // TODO: get isDirPath() and rmDir() to accept QStringView + if (!isDirPath(chunk, nullptr)) + return false; + if (!rmDir(chunk)) + return oldslash != 0; + slash = dirName.lastIndexOf(QDir::separator(), oldslash - 1); } - return rmDir(entry.filePath()); + + return true; } //static