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 <thiago.macieira@intel.com>

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);
 }
 

src/corelib/io/qfilesystemengine_p.h

@@ -116,7 +116,18 @@ public:
     static QString tempPath();
 
     static bool createDirectory(const QFileSystemEntry &entry, bool createParents,
+                                std::optional<QFile::Permissions> permissions = std::nullopt)
+    {
+        if (createParents)
+            return mkpath(entry, permissions);
+        return mkdir(entry, permissions);
+    }
+
+    static bool mkdir(const QFileSystemEntry &entry,
                       std::optional<QFile::Permissions> permissions = std::nullopt);
+    static bool mkpath(const QFileSystemEntry &entry,
+                       std::optional<QFile::Permissions> permissions = std::nullopt);
+
     static bool removeDirectory(const QFileSystemEntry &entry, bool removeEmptyParents)
     {
         if (removeEmptyParents)

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
+static QSystemError createDirectoryWithParents(const QByteArray &path, mode_t mode)
-// before calling this function.
-static bool createDirectoryWithParents(const QByteArray &nativeName, mode_t mode,
-                                       bool shouldMkdirFirst = true)
 {
 #ifdef Q_OS_WASM
-    if (nativeName.length() == 1 && nativeName[0] == '/')
+    if (path == '/')
-        return true;
+        return {};
 #endif
 
-    // helper function to check if a given path is a directory, since mkdir can
+    auto tryMkDir = [&path, mode]() -> QSystemError {
-    // fail if the dir already exists (it may have been created by another
+        if (QT_MKDIR(path, mode) == 0) {
-    // thread or another process)
+#ifdef Q_OS_VXWORKS
-    const auto isDir = [](const QByteArray &nativeName) {
+            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;
-        return QT_STAT(nativeName.constData(), &st) == 0 && (st.st_mode & S_IFMT) == S_IFDIR;
+            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) {
+    QSystemError result = tryMkDir();
-#ifdef Q_OS_VXWORKS
+    if (result.ok())
-        forceRequestedPermissionsOnVxWorks(nativeName, mode);
+        return result;
-#endif
+
-        return true;
+    // Only handle non-existing dir components in the path
-    }
+    if (result.errorCode != ENOENT)
-    if (errno == EISDIR)
+        return result;
-        return true;
+
-    if (errno == EEXIST || errno == EROFS)
+    qsizetype slash = path.lastIndexOf('/');
-        return isDir(nativeName);
+    while (slash > 0 && path[slash - 1] == '/')
-    if (errno != ENOENT)
+        --slash;
-        return false;
+
+    if (slash < 1)
+        return result;
 
     // mkdir failed because the parent dir doesn't exist, so try to create it
-    qsizetype slash = nativeName.lastIndexOf('/');
+    QByteArray parentPath = path.first(slash);
-    if (slash < 1)
+    if (result = createDirectoryWithParents(parentPath, mode); !result.ok())
-        return false;
+        return result;
-
-    QByteArray parentNativeName = nativeName.left(slash);
-    if (!createDirectoryWithParents(parentNativeName, mode))
-        return false;
 
     // try again
-    if (QT_MKDIR(nativeName, mode) == 0) {
+    return tryMkDir();
-#ifdef Q_OS_VXWORKS
-        forceRequestedPermissionsOnVxWorks(nativeName, mode);
-#endif
-        return true;
-    }
-    return errno == EEXIST && isDir(nativeName);
 }
 
-//static
+bool QFileSystemEngine::mkpath(const QFileSystemEntry &entry,
-bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool createParents,
                               std::optional<QFile::Permissions> permissions)
 {
-    QByteArray dirName = entry.nativeFilePath();
+    QByteArray path = entry.nativeFilePath();
-    Q_CHECK_FILE_NAME(dirName, false);
+    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) {
+    return createDirectoryWithParents(removeTrailingSlashes(path), mode).ok();
-#ifdef Q_OS_VXWORKS
-        forceRequestedPermissionsOnVxWorks(dirName, mode);
-#endif
-        return true;
 }
-    if (!createParents)
-        return false;
 
-    return createDirectoryWithParents(dirName, mode, false);
+bool QFileSystemEngine::mkdir(const QFileSystemEntry &entry,
+                              std::optional<QFile::Permissions> 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)

src/corelib/io/qfilesystemengine_win.cpp

@@ -1526,34 +1526,45 @@ static bool createDirectoryWithParents(const QString &nativeName,
     return isDir(nativeName);
 }
 
-//static
+bool QFileSystemEngine::mkpath(const QFileSystemEntry &entry,
-bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool createParents,
                                std::optional<QFile::Permissions> 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.
+    // mkpath should return true, if the directory already exists
-    if (!createParents)
-        return false;
     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<QFile::Permissions> 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();

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,
+        if (QFileSystemEngine::mkdir(fileSystemEntry, perms)) {
-                                               QFile::ReadOwner | QFile::WriteOwner
-                                                       | QFile::ExeOwner)) {
             success = true;
             pathOrError = fileSystemEntry.filePath();
             return;