summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorShawn Rutledge <shawn.rutledge@digia.com>2013-06-11 16:58:49 +0200
committerThe Qt Project <gerrit-noreply@qt-project.org>2013-06-14 16:57:57 +0200
commit3e2cd8ef6f1ce4f46719890134c8bba9dbdb19ba (patch)
tree88e08a6714ab06f01def25e13683bfb139b879cb
parent0bc96d1d6ba29c6e0a4c39cc526d9515036b7453 (diff)
fix QFileSystemEngine::createDirectory race conditionv5.1.0-rc1
During a call to QDir::mkpath(), the same path could be created by another process, in which case the OS mkdir will fail with EEXIST. But the docs for mkpath() state that it's not an error if it already exists, whereas for mkdir() it is an error. So QFileSystemEngine::createDirectory should accept the EEXIST error silently if it occurs while creating the sequence of parent directories and the final leaf directory, but should fail if EEXIST happens when it was called from QDir::mkdir(), which is when the createParents parameter is false. We assume the operating system mkdir() and CreateDirectory() are atomic, so there should be no race condition in QDir::mkdir(). It's not necessary for mkpath() to call stat() at each level, only to check whether an existing entry is a directory or a file. Also added to the autotest to verify that if the path is an existing file, creating a dir with the same name will fail in either mkdir or mkpath. Task-number: QTBUG-30046 Change-Id: I926352f10654fdf3b322c8685bb85ad8b8844874 Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@digia.com>
-rw-r--r--src/corelib/io/qfilesystemengine_unix.cpp11
-rw-r--r--src/corelib/io/qfilesystemengine_win.cpp13
-rw-r--r--tests/auto/corelib/io/qdir/tst_qdir.cpp18
3 files changed, 29 insertions, 13 deletions
diff --git a/src/corelib/io/qfilesystemengine_unix.cpp b/src/corelib/io/qfilesystemengine_unix.cpp
index b18e32b29a..a7517b4c7f 100644
--- a/src/corelib/io/qfilesystemengine_unix.cpp
+++ b/src/corelib/io/qfilesystemengine_unix.cpp
@@ -510,11 +510,12 @@ bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool crea
}
if (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;
- } else if (QT_MKDIR(chunk.constData(), 0777) != 0) {
+ if (QT_MKDIR(chunk.constData(), 0777) != 0) {
+ if (errno == EEXIST) {
+ QT_STATBUF st;
+ if (QT_STAT(chunk.constData(), &st) == 0 && (st.st_mode & S_IFMT) == S_IFDIR)
+ continue;
+ }
return false;
}
}
diff --git a/src/corelib/io/qfilesystemengine_win.cpp b/src/corelib/io/qfilesystemengine_win.cpp
index bee7689535..fdbd6e01e4 100644
--- a/src/corelib/io/qfilesystemengine_win.cpp
+++ b/src/corelib/io/qfilesystemengine_win.cpp
@@ -1044,14 +1044,13 @@ bool QFileSystemEngine::createDirectory(const QFileSystemEntry &entry, bool crea
}
if (slash) {
QString chunk = dirName.left(slash);
- bool existed = false;
- if (!isDirPath(chunk, &existed)) {
- if (!existed) {
- if (!mkDir(chunk))
- return false;
- } else {
- return false;
+ if (!mkDir(chunk)) {
+ if (GetLastError() == ERROR_ALREADY_EXISTS) {
+ bool existed = false;
+ if (isDirPath(chunk, &existed) && existed)
+ continue;
}
+ return false;
}
}
}
diff --git a/tests/auto/corelib/io/qdir/tst_qdir.cpp b/tests/auto/corelib/io/qdir/tst_qdir.cpp
index d44ef167b5..fbb21e4e9a 100644
--- a/tests/auto/corelib/io/qdir/tst_qdir.cpp
+++ b/tests/auto/corelib/io/qdir/tst_qdir.cpp
@@ -311,12 +311,28 @@ void tst_QDir::mkdir()
void tst_QDir::makedirReturnCode()
{
QString dirName = QString::fromLatin1("makedirReturnCode");
- QDir::current().rmdir(dirName); // cleanup a previous run.
+ QFile f(QDir::current().filePath(dirName));
+
+ // cleanup a previous run.
+ f.remove();
+ QDir::current().rmdir(dirName);
+
QDir dir(dirName);
QVERIFY(!dir.exists());
QVERIFY(QDir::current().mkdir(dirName));
QVERIFY(!QDir::current().mkdir(dirName)); // calling mkdir on an existing dir will fail.
QVERIFY(QDir::current().mkpath(dirName)); // calling mkpath on an existing dir will pass
+
+ // Remove the directory and create a file with the same path
+ QDir::current().rmdir(dirName);
+ QVERIFY(!f.exists());
+ f.open(QIODevice::WriteOnly);
+ f.write("test");
+ f.close();
+ QVERIFY(f.exists());
+ QVERIFY(!QDir::current().mkdir(dirName)); // calling mkdir on an existing file will fail.
+ QVERIFY(!QDir::current().mkpath(dirName)); // calling mkpath on an existing file will fail.
+ f.remove();
}
void tst_QDir::rmdir_data()