From 1a4cc8d57b928509a64f9679e5c0e7afaa05cb54 Mon Sep 17 00:00:00 2001 From: d3fault Date: Fri, 24 Nov 2017 16:07:53 -0700 Subject: Add QIODevice::NewOnly and QIODevice::ExistingOnly OpenMode flags When QFile::open is called with the NewOnly flag, the call will fail if the file already exists. As usual, if the file does not exist, it will be created. Like QTemporaryFile, there is a guarantee from the operating system that you are not accidentally creating a new file on top of an older file. When QFile::open is called with the ExistingOnly flag, the call will fail if the file does not exist. The ExistingOnly flag only provides new functionality when used with the WriteOnly flag. For ReadOnly it provides no change in functionality, as ReadOnly by itself already never creates. Task-number: QTBUG-52244 Change-Id: I8e3206728f245f95172c225bf297023fb078fc6d Reviewed-by: Edward Welbourne --- mkspecs/android-clang/qplatformdefs.h | 1 + mkspecs/android-g++/qplatformdefs.h | 1 + mkspecs/common/android/qplatformdefs.h | 1 + mkspecs/common/posix/qplatformdefs.h | 1 + src/corelib/io/qfile.cpp | 10 +++--- src/corelib/io/qfsfileengine.cpp | 56 +++++++++++++++++++------------ src/corelib/io/qfsfileengine_p.h | 9 +++++ src/corelib/io/qfsfileengine_unix.cpp | 15 ++++++--- src/corelib/io/qfsfileengine_win.cpp | 7 ++-- src/corelib/io/qiodevice.cpp | 17 ++++++++++ src/corelib/io/qiodevice.h | 4 ++- src/corelib/io/qsavefile.cpp | 6 ++-- src/corelib/io/qtemporaryfile.cpp | 2 +- tests/auto/corelib/io/qfile/tst_qfile.cpp | 44 ++++++++++++++++++++++++ 14 files changed, 138 insertions(+), 36 deletions(-) diff --git a/mkspecs/android-clang/qplatformdefs.h b/mkspecs/android-clang/qplatformdefs.h index b9d987e608..f405c91ecb 100644 --- a/mkspecs/android-clang/qplatformdefs.h +++ b/mkspecs/android-clang/qplatformdefs.h @@ -139,6 +139,7 @@ #define QT_OPEN_CREAT O_CREAT #define QT_OPEN_TRUNC O_TRUNC #define QT_OPEN_APPEND O_APPEND +#define QT_OPEN_EXCL O_EXCL // Directory iteration #define QT_DIR DIR diff --git a/mkspecs/android-g++/qplatformdefs.h b/mkspecs/android-g++/qplatformdefs.h index a0e80629a4..0b92709dd5 100644 --- a/mkspecs/android-g++/qplatformdefs.h +++ b/mkspecs/android-g++/qplatformdefs.h @@ -139,6 +139,7 @@ #define QT_OPEN_CREAT O_CREAT #define QT_OPEN_TRUNC O_TRUNC #define QT_OPEN_APPEND O_APPEND +#define QT_OPEN_EXCL O_EXCL // Directory iteration #define QT_DIR DIR diff --git a/mkspecs/common/android/qplatformdefs.h b/mkspecs/common/android/qplatformdefs.h index 23180c5d3c..f75bc4093b 100644 --- a/mkspecs/common/android/qplatformdefs.h +++ b/mkspecs/common/android/qplatformdefs.h @@ -115,6 +115,7 @@ #define QT_OPEN_CREAT O_CREAT #define QT_OPEN_TRUNC O_TRUNC #define QT_OPEN_APPEND O_APPEND +#define QT_OPEN_EXCL O_EXCL // Directory iteration #define QT_DIR DIR diff --git a/mkspecs/common/posix/qplatformdefs.h b/mkspecs/common/posix/qplatformdefs.h index 90fb90a7e3..3ebc523338 100644 --- a/mkspecs/common/posix/qplatformdefs.h +++ b/mkspecs/common/posix/qplatformdefs.h @@ -128,6 +128,7 @@ #define QT_OPEN_CREAT O_CREAT #define QT_OPEN_TRUNC O_TRUNC #define QT_OPEN_APPEND O_APPEND +#define QT_OPEN_EXCL O_EXCL // Posix extensions to C89 #define QT_FILENO fileno diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index e4888e9523..33b0b2eb66 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -893,9 +893,9 @@ bool QFile::open(OpenMode mode) qWarning("QFile::open: File (%s) already open", qPrintable(fileName())); return false; } - if (mode & Append) + // Either Append or NewOnly implies WriteOnly + if (mode & (Append | NewOnly)) mode |= WriteOnly; - unsetError(); if ((mode & (ReadOnly | WriteOnly)) == 0) { qWarning("QIODevice::open: File access not specified"); @@ -965,7 +965,8 @@ bool QFile::open(FILE *fh, OpenMode mode, FileHandleFlags handleFlags) qWarning("QFile::open: File (%s) already open", qPrintable(fileName())); return false; } - if (mode & Append) + // Either Append or NewOnly implies WriteOnly + if (mode & (Append | NewOnly)) mode |= WriteOnly; unsetError(); if ((mode & (ReadOnly | WriteOnly)) == 0) { @@ -1023,7 +1024,8 @@ bool QFile::open(int fd, OpenMode mode, FileHandleFlags handleFlags) qWarning("QFile::open: File (%s) already open", qPrintable(fileName())); return false; } - if (mode & Append) + // Either Append or NewOnly implies WriteOnly + if (mode & (Append | NewOnly)) mode |= WriteOnly; unsetError(); if ((mode & (ReadOnly | WriteOnly)) == 0) { diff --git a/src/corelib/io/qfsfileengine.cpp b/src/corelib/io/qfsfileengine.cpp index 538fcb9a37..387990ed79 100644 --- a/src/corelib/io/qfsfileengine.cpp +++ b/src/corelib/io/qfsfileengine.cpp @@ -164,6 +164,35 @@ QFSFileEngine::QFSFileEngine(QFSFileEnginePrivate &dd) { } +/*! + \internal +*/ +bool QFSFileEngine::processOpenModeFlags(QIODevice::OpenMode *mode) +{ + QIODevice::OpenMode &openMode = *mode; + if ((openMode & QFile::NewOnly) && (openMode & QFile::ExistingOnly)) { + qWarning("NewOnly and ExistingOnly are mutually exclusive"); + setError(QFile::OpenError, QLatin1String("NewOnly and ExistingOnly are mutually exclusive")); + return false; + } + + if ((openMode & QFile::ExistingOnly) && !(openMode & (QFile::ReadOnly | QFile::WriteOnly))) { + qWarning("ExistingOnly must be specified alongside ReadOnly, WriteOnly, or ReadWrite"); + setError(QFile::OpenError, QLatin1String("ExistingOnly must be specified alongside ReadOnly, WriteOnly, or ReadWrite")); + return false; + } + + // Either Append or NewOnly implies WriteOnly + if (openMode & (QFile::Append | QFile::NewOnly)) + openMode |= QFile::WriteOnly; + + // WriteOnly implies Truncate when ReadOnly, Append, and NewOnly are not set. + if ((openMode & QFile::WriteOnly) && !(openMode & (QFile::ReadOnly | QFile::Append | QFile::NewOnly))) + openMode |= QFile::Truncate; + + return true; +} + /*! Destructs the QFSFileEngine. */ @@ -205,13 +234,8 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode) return false; } - // Append implies WriteOnly. - if (openMode & QFile::Append) - openMode |= QFile::WriteOnly; - - // WriteOnly implies Truncate if neither ReadOnly nor Append are sent. - if ((openMode & QFile::WriteOnly) && !(openMode & (QFile::ReadOnly | QFile::Append))) - openMode |= QFile::Truncate; + if (!processOpenModeFlags(&openMode)) + return false; d->openMode = openMode; d->lastFlushFailed = false; @@ -238,13 +262,8 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode, FILE *fh, QFile::FileHand Q_D(QFSFileEngine); - // Append implies WriteOnly. - if (openMode & QFile::Append) - openMode |= QFile::WriteOnly; - - // WriteOnly implies Truncate if neither ReadOnly nor Append are sent. - if ((openMode & QFile::WriteOnly) && !(openMode & (QFile::ReadOnly | QFile::Append))) - openMode |= QFile::Truncate; + if (!processOpenModeFlags(&openMode)) + return false; d->openMode = openMode; d->lastFlushFailed = false; @@ -302,13 +321,8 @@ bool QFSFileEngine::open(QIODevice::OpenMode openMode, int fd, QFile::FileHandle { Q_D(QFSFileEngine); - // Append implies WriteOnly. - if (openMode & QFile::Append) - openMode |= QFile::WriteOnly; - - // WriteOnly implies Truncate if neither ReadOnly nor Append are sent. - if ((openMode & QFile::WriteOnly) && !(openMode & (QFile::ReadOnly | QFile::Append))) - openMode |= QFile::Truncate; + if (!processOpenModeFlags(&openMode)) + return false; d->openMode = openMode; d->lastFlushFailed = false; diff --git a/src/corelib/io/qfsfileengine_p.h b/src/corelib/io/qfsfileengine_p.h index 16ba161b75..6b091a8eef 100644 --- a/src/corelib/io/qfsfileengine_p.h +++ b/src/corelib/io/qfsfileengine_p.h @@ -131,6 +131,9 @@ public: protected: QFSFileEngine(QFSFileEnginePrivate &dd); + +private: + inline bool processOpenModeFlags(QIODevice::OpenMode *mode); }; class Q_AUTOTEST_EXPORT QFSFileEnginePrivate : public QAbstractFileEnginePrivate @@ -219,6 +222,12 @@ public: int sysOpen(const QString &, int flags); #endif + static bool openModeCanCreate(QIODevice::OpenMode openMode) + { + // WriteOnly can create, but only when ExistingOnly isn't specified. + // ReadOnly by itself never creates. + return (openMode & QFile::WriteOnly) && !(openMode & QFile::ExistingOnly); + } protected: QFSFileEnginePrivate(); diff --git a/src/corelib/io/qfsfileengine_unix.cpp b/src/corelib/io/qfsfileengine_unix.cpp index c040d67862..7dd4f6556d 100644 --- a/src/corelib/io/qfsfileengine_unix.cpp +++ b/src/corelib/io/qfsfileengine_unix.cpp @@ -74,11 +74,13 @@ static inline int openModeToOpenFlags(QIODevice::OpenMode mode) oflags |= QT_OPEN_LARGEFILE; #endif - if ((mode & QFile::ReadWrite) == QFile::ReadWrite) { - oflags = QT_OPEN_RDWR | QT_OPEN_CREAT; - } else if (mode & QFile::WriteOnly) { - oflags = QT_OPEN_WRONLY | QT_OPEN_CREAT; - } + if ((mode & QFile::ReadWrite) == QFile::ReadWrite) + oflags = QT_OPEN_RDWR; + else if (mode & QFile::WriteOnly) + oflags = QT_OPEN_WRONLY; + + if (QFSFileEnginePrivate::openModeCanCreate(mode)) + oflags |= QT_OPEN_CREAT; if (mode & QFile::Append) { oflags |= QT_OPEN_APPEND; @@ -87,6 +89,9 @@ static inline int openModeToOpenFlags(QIODevice::OpenMode mode) oflags |= QT_OPEN_TRUNC; } + if (mode & QFile::NewOnly) + oflags |= QT_OPEN_EXCL; + return oflags; } diff --git a/src/corelib/io/qfsfileengine_win.cpp b/src/corelib/io/qfsfileengine_win.cpp index 759effe632..8199f6a846 100644 --- a/src/corelib/io/qfsfileengine_win.cpp +++ b/src/corelib/io/qfsfileengine_win.cpp @@ -117,9 +117,12 @@ bool QFSFileEnginePrivate::nativeOpen(QIODevice::OpenMode openMode) if (openMode & QIODevice::WriteOnly) accessRights |= GENERIC_WRITE; - // WriteOnly can create files, ReadOnly cannot. - DWORD creationDisp = (openMode & QIODevice::WriteOnly) ? OPEN_ALWAYS : OPEN_EXISTING; + DWORD creationDisp = (openMode & QIODevice::NewOnly) + ? CREATE_NEW + : openModeCanCreate(openMode) + ? OPEN_ALWAYS + : OPEN_EXISTING; // Create the file handle. #ifndef Q_OS_WINRT SECURITY_ATTRIBUTES securityAtts = { sizeof(SECURITY_ATTRIBUTES), NULL, FALSE }; diff --git a/src/corelib/io/qiodevice.cpp b/src/corelib/io/qiodevice.cpp index 7d46898911..95a5fb27cf 100644 --- a/src/corelib/io/qiodevice.cpp +++ b/src/corelib/io/qiodevice.cpp @@ -324,6 +324,23 @@ QIODevicePrivate::~QIODevicePrivate() terminators are translated to the local encoding, for example '\\r\\n' for Win32. \value Unbuffered Any buffer in the device is bypassed. + \value NewOnly Fail if the file to be opened already exists. Create and + open the file only if it does not exist. There is a + guarantee from the operating system that you are the only + one creating and opening the file. Note that this mode + implies WriteOnly, and combining it with ReadWrite is + allowed. This flag currently only affects QFile. Other + classes might use this flag in the future, but until then + using this flag with any classes other than QFile may + result in undefined behavior. + \value ExistingOnly Fail if the file to be opened does not exist. This flag + must be specified alongside ReadOnly, WriteOnly, or + ReadWrite. Note that using this flag with ReadOnly alone + is redundant, as ReadOnly already fails when the file does + not exist. This flag currently only affects QFile. Other + classes might use this flag in the future, but until then + using this flag with any classes other than QFile may + result in undefined behavior. Certain flags, such as \c Unbuffered and \c Truncate, are meaningless when used with some subclasses. Some of these diff --git a/src/corelib/io/qiodevice.h b/src/corelib/io/qiodevice.h index af37b3fd53..2e4debe339 100644 --- a/src/corelib/io/qiodevice.h +++ b/src/corelib/io/qiodevice.h @@ -76,7 +76,9 @@ public: Append = 0x0004, Truncate = 0x0008, Text = 0x0010, - Unbuffered = 0x0020 + Unbuffered = 0x0020, + NewOnly = 0x0040, + ExistingOnly = 0x0080 }; Q_DECLARE_FLAGS(OpenMode, OpenModeFlag) diff --git a/src/corelib/io/qsavefile.cpp b/src/corelib/io/qsavefile.cpp index 63f2284ef5..56934a9a0f 100644 --- a/src/corelib/io/qsavefile.cpp +++ b/src/corelib/io/qsavefile.cpp @@ -184,7 +184,8 @@ void QSaveFile::setFileName(const QString &name) Important: the \a mode must include QIODevice::WriteOnly. It may also have additional flags, such as QIODevice::Text and QIODevice::Unbuffered. - QIODevice::ReadWrite and QIODevice::Append are not supported at the moment. + QIODevice::ReadWrite, QIODevice::Append, QIODevice::NewOnly and + QIODevice::ExistingOnly are not supported at the moment. \sa QIODevice::OpenMode, setFileName() */ @@ -201,7 +202,8 @@ bool QSaveFile::open(OpenMode mode) return false; } // In the future we could implement ReadWrite by copying from the existing file to the temp file... - if ((mode & ReadOnly) || (mode & Append)) { + // The implications of NewOnly and ExistingOnly when used with QSaveFile need to be considered carefully... + if (mode & (ReadOnly | Append | NewOnly | ExistingOnly)) { qWarning("QSaveFile::open: Unsupported open mode 0x%x", int(mode)); return false; } diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp index 35699d52df..73249d7df8 100644 --- a/src/corelib/io/qtemporaryfile.cpp +++ b/src/corelib/io/qtemporaryfile.cpp @@ -248,7 +248,7 @@ static bool createFileFromTemplate(NativeFileHandle &file, QTemporaryFileName &t } #else // POSIX file = QT_OPEN(path.constData(), - QT_OPEN_CREAT | O_EXCL | QT_OPEN_RDWR | QT_OPEN_LARGEFILE, + QT_OPEN_CREAT | QT_OPEN_EXCL | QT_OPEN_RDWR | QT_OPEN_LARGEFILE, static_cast(mode)); if (file != -1) diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp index 06c3a9578f..bfb14da8b8 100644 --- a/tests/auto/corelib/io/qfile/tst_qfile.cpp +++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp @@ -168,6 +168,8 @@ private slots: void getch(); void ungetChar(); void createFile(); + void createFileNewOnly(); + void openFileExistingOnly(); void append(); void permissions_data(); void permissions(); @@ -1211,6 +1213,48 @@ void tst_QFile::createFile() QVERIFY( QFile::exists( "createme.txt" ) ); } +void tst_QFile::createFileNewOnly() +{ + QFile::remove("createme.txt"); + QVERIFY(!QFile::exists("createme.txt")); + + QFile f("createme.txt"); + QVERIFY2(f.open(QIODevice::NewOnly), msgOpenFailed(f).constData()); + f.close(); + QVERIFY(QFile::exists("createme.txt")); + + QVERIFY(!f.open(QIODevice::NewOnly)); + QVERIFY(QFile::exists("createme.txt")); + QFile::remove("createme.txt"); +} + +void tst_QFile::openFileExistingOnly() +{ + QFile::remove("dontcreateme.txt"); + QVERIFY(!QFile::exists("dontcreateme.txt")); + + QFile f("dontcreateme.txt"); + QVERIFY(!f.open(QIODevice::ExistingOnly | QIODevice::ReadOnly)); + QVERIFY(!f.open(QIODevice::ExistingOnly | QIODevice::WriteOnly)); + QVERIFY(!f.open(QIODevice::ExistingOnly | QIODevice::ReadWrite)); + QVERIFY(!f.open(QIODevice::ExistingOnly)); + QVERIFY(!QFile::exists("dontcreateme.txt")); + + QVERIFY2(f.open(QIODevice::NewOnly), msgOpenFailed(f).constData()); + f.close(); + QVERIFY(QFile::exists("dontcreateme.txt")); + + QVERIFY2(f.open(QIODevice::ExistingOnly | QIODevice::ReadOnly), msgOpenFailed(f).constData()); + f.close(); + QVERIFY2(f.open(QIODevice::ExistingOnly | QIODevice::WriteOnly), msgOpenFailed(f).constData()); + f.close(); + QVERIFY2(f.open(QIODevice::ExistingOnly | QIODevice::ReadWrite), msgOpenFailed(f).constData()); + f.close(); + QVERIFY(!f.open(QIODevice::ExistingOnly)); + QVERIFY(QFile::exists("dontcreateme.txt")); + QFile::remove("dontcreateme.txt"); +} + void tst_QFile::append() { const QString name("appendme.txt"); -- cgit v1.2.3