From e275db9d885ccccc3def4d52c2dae2f8c062df1a Mon Sep 17 00:00:00 2001 From: Ahmad Samir Date: Wed, 25 Sep 2024 21:07:46 +0300 Subject: [PATCH] QFileSystemEngine: split dir creation into mkdir/mkpath Split the logic of createDirectory() into mkdir and mkpath. This matches QDir::mkdir()/mkpath(). "mkdir()" won't confuse the compiler about which method to call, because libc's mkdir() is either used via QT_MKDIR which expands to "::mkdir" or directly as ::mdkir(), whereas the static QFileSystemEngine::mkdir() is always called with full scope. Change-Id: I31b67727cce23f1bc560432d40231a24dc560d5b Reviewed-by: Thiago Macieira --- src/corelib/io/qdir.cpp | 6 +- src/corelib/io/qfilesystemengine_p.h | 13 ++- src/corelib/io/qfilesystemengine_unix.cpp | 121 ++++++++++++---------- src/corelib/io/qfilesystemengine_win.cpp | 27 +++-- src/corelib/io/qtemporarydir.cpp | 5 +- 5 files changed, 100 insertions(+), 72 deletions(-) diff --git a/src/corelib/io/qdir.cpp b/src/corelib/io/qdir.cpp index cafd7ecea68..59687257243 100644 --- a/src/corelib/io/qdir.cpp +++ b/src/corelib/io/qdir.cpp @@ -1506,7 +1506,7 @@ bool QDir::mkdir(const QString &dirName, QFile::Permissions permissions) const QString fn = filePath(dirName); if (!d->fileEngine) - return QFileSystemEngine::createDirectory(QFileSystemEntry(fn), false, permissions); + return QFileSystemEngine::mkdir(QFileSystemEntry(fn), permissions); return d->fileEngine->mkdir(fn, false, permissions); } @@ -1528,7 +1528,7 @@ bool QDir::mkdir(const QString &dirName) const QString fn = filePath(dirName); if (!d->fileEngine) - return QFileSystemEngine::createDirectory(QFileSystemEntry(fn), false); + return QFileSystemEngine::mkdir(QFileSystemEntry(fn)); return d->fileEngine->mkdir(fn, false); } @@ -1580,7 +1580,7 @@ bool QDir::mkpath(const QString &dirPath) const QString fn = filePath(dirPath); if (!d->fileEngine) - return QFileSystemEngine::createDirectory(QFileSystemEntry(fn), true); + return QFileSystemEngine::mkpath(QFileSystemEntry(fn)); return d->fileEngine->mkdir(fn, true); } diff --git a/src/corelib/io/qfilesystemengine_p.h b/src/corelib/io/qfilesystemengine_p.h index 000d52805e8..65a4cadebec 100644 --- a/src/corelib/io/qfilesystemengine_p.h +++ b/src/corelib/io/qfilesystemengine_p.h @@ -116,7 +116,18 @@ public: static QString tempPath(); static bool createDirectory(const QFileSystemEntry &entry, bool createParents, - std::optional permissions = std::nullopt); + std::optional permissions = std::nullopt) + { + if (createParents) + return mkpath(entry, permissions); + return mkdir(entry, permissions); + } + + static bool mkdir(const QFileSystemEntry &entry, + std::optional permissions = std::nullopt); + static bool mkpath(const QFileSystemEntry &entry, + std::optional permissions = std::nullopt); + static bool removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents) { if (removeEmptyParents) diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp index 84010d5a909..548525661e9 100644 --- a/src/corelib/io/qfilesystemengine_unix.cpp +++ b/src/corelib/io/qfilesystemengine_unix.cpp @@ -85,6 +85,15 @@ QT_BEGIN_NAMESPACE using namespace Qt::StringLiterals; +static QByteArray &removeTrailingSlashes(QByteArray &path) +{ + // Darwin doesn't support trailing /'s, so remove for everyone + while (path.size() > 1 && path.endsWith('/')) + path.chop(1); + + return path; +} + enum { #ifdef Q_OS_ANDROID // On Android, the link(2) system call has been observed to always fail @@ -1114,79 +1123,77 @@ bool QFileSystemEngine::cloneFile(int srcfd, int dstfd, const QFileSystemMetaDat #endif } -// Note: if \a shouldMkdirFirst is false, we assume the caller did try to mkdir -// before calling this function. -static bool createDirectoryWithParents(const QByteArray &nativeName, mode_t mode, - bool shouldMkdirFirst = true) +static QSystemError createDirectoryWithParents(const QByteArray &path, mode_t mode) { #ifdef Q_OS_WASM - if (nativeName.length() == 1 && nativeName[0] == '/') - return true; + if (path == '/') + return {}; #endif - // helper function to check if a given path is a directory, since mkdir can - // fail if the dir already exists (it may have been created by another - // thread or another process) - const auto isDir = [](const QByteArray &nativeName) { - QT_STATBUF st; - return QT_STAT(nativeName.constData(), &st) == 0 && (st.st_mode & S_IFMT) == S_IFDIR; + auto tryMkDir = [&path, mode]() -> QSystemError { + if (QT_MKDIR(path, mode) == 0) { +#ifdef Q_OS_VXWORKS + forceRequestedPermissionsOnVxWorks(path, mode); +#endif + return {}; + } + // On macOS with APFS mkdir sets errno to EISDIR, QTBUG-97110 + if (errno == EISDIR) + return {}; + if (errno == EEXIST || errno == EROFS) { + // ::mkdir() can fail if the dir already exists (it may have been + // created by another thread or another process) + QT_STATBUF st; + if (QT_STAT(path.constData(), &st) != 0) + return QSystemError::stdError(errno); + const bool isDir = (st.st_mode & S_IFMT) == S_IFDIR; + return isDir ? QSystemError{} : QSystemError::stdError(EEXIST); + } + return QSystemError::stdError(errno); }; - if (shouldMkdirFirst && QT_MKDIR(nativeName, mode) == 0) { -#ifdef Q_OS_VXWORKS - forceRequestedPermissionsOnVxWorks(nativeName, mode); -#endif - return true; - } - if (errno == EISDIR) - return true; - if (errno == EEXIST || errno == EROFS) - return isDir(nativeName); - if (errno != ENOENT) - return false; + QSystemError result = tryMkDir(); + if (result.ok()) + return result; + + // Only handle non-existing dir components in the path + if (result.errorCode != ENOENT) + return result; + + qsizetype slash = path.lastIndexOf('/'); + while (slash > 0 && path[slash - 1] == '/') + --slash; + + if (slash < 1) + return result; // mkdir failed because the parent dir doesn't exist, so try to create it - qsizetype slash = nativeName.lastIndexOf('/'); - if (slash < 1) - return false; - - QByteArray parentNativeName = nativeName.left(slash); - if (!createDirectoryWithParents(parentNativeName, mode)) - return false; + QByteArray parentPath = path.first(slash); + if (result = createDirectoryWithParents(parentPath, mode); !result.ok()) + return result; // try again - if (QT_MKDIR(nativeName, mode) == 0) { -#ifdef Q_OS_VXWORKS - forceRequestedPermissionsOnVxWorks(nativeName, mode); -#endif - return true; - } - return errno == EEXIST && isDir(nativeName); + return tryMkDir(); } -//static -bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool createParents, - std::optional permissions) +bool QFileSystemEngine::mkpath(const QFileSystemEntry &entry, + std::optional permissions) { - QByteArray dirName = entry.nativeFilePath(); - Q_CHECK_FILE_NAME(dirName, false); + QByteArray path = entry.nativeFilePath(); + Q_CHECK_FILE_NAME(path, false); - // Darwin doesn't support trailing /'s, so remove for everyone - while (dirName.size() > 1 && dirName.endsWith(u'/')) - dirName.chop(1); - - // try to mkdir this directory mode_t mode = permissions ? QtPrivate::toMode_t(*permissions) : 0777; - if (QT_MKDIR(dirName, mode) == 0) { -#ifdef Q_OS_VXWORKS - forceRequestedPermissionsOnVxWorks(dirName, mode); -#endif - return true; - } - if (!createParents) - return false; + return createDirectoryWithParents(removeTrailingSlashes(path), mode).ok(); +} - return createDirectoryWithParents(dirName, mode, false); +bool QFileSystemEngine::mkdir(const QFileSystemEntry &entry, + std::optional permissions) +{ + QByteArray path = entry.nativeFilePath(); + Q_CHECK_FILE_NAME(path, false); + + mode_t mode = permissions ? QtPrivate::toMode_t(*permissions) : 0777; + return QT_MKDIR(removeTrailingSlashes(path), mode) == 0; } bool QFileSystemEngine::rmdir(const QFileSystemEntry &entry) diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp index 8d4e5d95f04..94df5092d3b 100644 --- a/src/corelib/io/qfilesystemengine_win.cpp +++ b/src/corelib/io/qfilesystemengine_win.cpp @@ -1526,34 +1526,45 @@ static bool createDirectoryWithParents(const QString &nativeName, return isDir(nativeName); } -//static -bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool createParents, - std::optional permissions) +bool QFileSystemEngine::mkpath(const QFileSystemEntry &entry, + std::optional permissions) { QString dirName = entry.filePath(); Q_CHECK_FILE_NAME(dirName, false); - dirName = QDir::toNativeSeparators(QDir::cleanPath(dirName)); - QNativeFilePermissions nativePermissions(permissions, true); if (!nativePermissions.isOk()) return false; auto securityAttributes = nativePermissions.securityAttributes(); + dirName = QDir::toNativeSeparators(QDir::cleanPath(dirName)); // try to mkdir this directory DWORD lastError; if (mkDir(dirName, securityAttributes, &lastError)) return true; - // mkpath should return true, if the directory already exists, mkdir false. - if (!createParents) - return false; + // mkpath should return true, if the directory already exists if (lastError == ERROR_ALREADY_EXISTS || lastError == ERROR_ACCESS_DENIED) return isDirPath(dirName, nullptr); return createDirectoryWithParents(dirName, securityAttributes, false); } +bool QFileSystemEngine::mkdir(const QFileSystemEntry &entry, + std::optional permissions) +{ + QString dirName = entry.filePath(); + Q_CHECK_FILE_NAME(dirName, false); + + + QNativeFilePermissions nativePermissions(permissions, true); + if (!nativePermissions.isOk()) + return false; + + dirName = QDir::toNativeSeparators(QDir::cleanPath(dirName)); + return mkDir(dirName, nativePermissions.securityAttributes()); +} + bool QFileSystemEngine::rmdir(const QFileSystemEntry &entry) { QString dirName = entry.filePath(); diff --git a/src/corelib/io/qtemporarydir.cpp b/src/corelib/io/qtemporarydir.cpp index 490ad595d43..751057f5fee 100644 --- a/src/corelib/io/qtemporarydir.cpp +++ b/src/corelib/io/qtemporarydir.cpp @@ -67,12 +67,11 @@ static QString defaultTemplateName() void QTemporaryDirPrivate::create(const QString &templateName) { QTemporaryFileName tfn(templateName); + constexpr auto perms = QFile::ReadOwner | QFile::WriteOwner | QFile::ExeOwner; for (int i = 0; i < 256; ++i) { tfn.generateNext(); QFileSystemEntry fileSystemEntry(tfn.path, QFileSystemEntry::FromNativePath()); - if (QFileSystemEngine::createDirectory(fileSystemEntry, false, - QFile::ReadOwner | QFile::WriteOwner - | QFile::ExeOwner)) { + if (QFileSystemEngine::mkdir(fileSystemEntry, perms)) { success = true; pathOrError = fileSystemEntry.filePath(); return;