A very common issue with writing over an existing file, is that in case of
problem in the middle of the writing operation (disk full, application crash,
power failure...) the existing file will be lost. If it contained your thesis,
you might be a bit angry at the Qt application that did that.
The typical solution is to write into a temporary file in the same directory,
and then atomically renaming the temp file to overwrite the existing file.
This then works like a transaction, with the choice to end the writing
operation with a commit (= rename the temp file) or rollback (=erase the temp
file), in case of errors while writing.
Solution 1: using a separate class for handling the QTemporaryFile and the
rename call. KDE's KSaveFile and QtCreator's SaveFile do that. The code of
such a class is quite easy. The downside is that from an API point of view,
it's a bit weird. You have to use this "save file" wrapper class, which is hard
to picture in one's mind, and to name properly for what it does.
API-wise, such a class has methods like commit() and rollback(), to decide
what happens when we're done. If you forget to call either one, the destructor
will decide for you. Funny, in KDE it commits, in QtCreator it rolls back...
Solution 2: how about making this functionality part of QFile itself?
No need to "port to another class" anymore, just enable the safety feature by
calling file.setUseTemporaryFile(true).
This is what I've started doing in the attached patch, but I'd like to gather
feedback before polishing it up.
One issue is that after doing file.setUseTemporaryFile(true) and file.open(),
all the operations on the file object are no longer operating on the given
fileName, but on the "internal" temporary file. That's what we want for
writing,
but maybe it could confuse people that remove() or rename() leaves the
existing file untouched? I think it would simply have to be documented: when
enabling the feature, all that happens between open() and close(), happens on
the temp file.
The other question is, would one have to call commit/rollback explicitely, or
should QFile::close() (and the dtor) do the committing?
And how should one rollback? Explicit file.rollback(), or in the existing
file.remove()?
Oswald suggested doing that in close()/remove() directly, and getting rid of
commit()/rollback(). Opinions?
--
David Faure, fa...@kde.org, http://www.davidfaure.fr
Sponsored by Nokia to work on KDE, incl. KDE Frameworks 5
diff --git a/src/corelib/io/qfile.h b/src/corelib/io/qfile.h
index 554b295..c92bed5 100644
--- a/src/corelib/io/qfile.h
+++ b/src/corelib/io/qfile.h
@@ -117,6 +117,11 @@ public:
QString fileName() const;
void setFileName(const QString &name);
+ bool useTemporaryFile() const;
+ void setUseTemporaryFile(bool useTemp);
+ void rollback();
+ bool commit();
+
typedef QByteArray (*EncoderFn)(const QString &fileName);
typedef QString (*DecoderFn)(const QByteArray &localfileName);
static QByteArray encodeName(const QString &fileName);
diff --git a/src/corelib/io/qfile_p.h b/src/corelib/io/qfile_p.h
index 3072f5b..3b05697 100644
--- a/src/corelib/io/qfile_p.h
+++ b/src/corelib/io/qfile_p.h
@@ -76,12 +76,15 @@ protected:
QString fileName;
mutable QAbstractFileEngine *fileEngine;
+ bool useTemporaryFile;
+ bool finalized;
+
bool lastWasWrite;
QRingBuffer writeBuffer;
inline bool ensureFlushed() const;
bool putCharHelper(char c);
-
+
QFile::FileError error;
void setError(QFile::FileError err);
void setError(QFile::FileError err, const QString &errorString);
diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp
index d511852..007eeb7 100644
--- a/src/corelib/io/qfile.cpp
+++ b/src/corelib/io/qfile.cpp
@@ -90,7 +90,7 @@ QFile::EncoderFn QFilePrivate::encoder = locale_encode;
QFile::DecoderFn QFilePrivate::decoder = locale_decode;
QFilePrivate::QFilePrivate()
- : fileEngine(0), lastWasWrite(false),
+ : fileEngine(0), useTemporaryFile(false), finalized(true), lastWasWrite(false),
writeBuffer(QFILE_WRITEBUFFER_SIZE), error(QFile::NoError),
cachedSize(0)
{
@@ -434,6 +434,9 @@ QFile::QFile(QFilePrivate &dd, QObject *parent)
*/
QFile::~QFile()
{
+ Q_D(QFile);
+ if (!d->finalized)
+ commit();
close();
}
@@ -445,7 +448,8 @@ QFile::~QFile()
*/
QString QFile::fileName() const
{
- return fileEngine()->fileName(QAbstractFileEngine::DefaultName);
+ Q_D(const QFile);
+ return d->fileName;
}
/*!
@@ -482,6 +486,25 @@ QFile::setFileName(const QString &name)
d->fileName = name;
}
+// TODO DOCU
+bool QFile::useTemporaryFile() const
+{
+ Q_D(const QFile);
+ return d->useTemporaryFile;
+}
+
+// TODO (taken from setFileName)
+void QFile::setUseTemporaryFile(bool useTemp)
+{
+ Q_D(QFile);
+ if (isOpen()) {
+ qWarning("QFile::setUseTemporaryFile: File (%s) is already opened",
+ qPrintable(fileName()));
+ return;
+ }
+ d->useTemporaryFile = useTemp;
+}
+
/*!
\fn QString QFile::decodeName(const char *localFileName)
@@ -673,6 +696,7 @@ QFile::remove()
return false;
}
unsetError();
+ // TODO if open and using temp file, rollback()
close();
if(error() == QFile::NoError) {
if(fileEngine()->remove()) {
@@ -1019,6 +1043,27 @@ bool QFile::open(OpenMode mode)
return false;
}
+ if (d->useTemporaryFile && (mode & WriteOnly)) {
+ // TODO check if existing file is writable
+ QTemporaryFile tempFile;
+ tempFile.setAutoRemove(false);
+ tempFile.setFileTemplate(d->fileName);
+ if (!tempFile.open(mode)) {
+ d->setError(tempFile.error(), tempFile.errorString());
+ return false;
+ }
+ if (QFile::exists(d->fileName))
+ tempFile.setPermissions(permissions());
+ // Steal the file engine from the temp file
+ delete d->fileEngine;
+ QFilePrivate* tempPrivate = static_cast<QFilePrivate *>(tempFile.d_ptr.data());
+ d->fileEngine = tempPrivate->fileEngine;
+ tempPrivate->fileEngine = 0;
+ tempFile.setOpenMode(NotOpen);
+ d->finalized = false;
+ return true;
+ }
+
#ifdef Q_OS_SYMBIAN
// For symbian, the unbuffered flag is used to control write-behind cache behaviour
if (fileEngine()->open(mode))
@@ -1039,6 +1084,36 @@ bool QFile::open(OpenMode mode)
return false;
}
+// TODO DOCU
+void QFile::rollback()
+{
+ Q_D(QFile);
+ remove(); // remove the temp file
+ d->finalized = true;
+}
+
+// TODO DOCU
+bool QFile::commit()
+{
+ Q_D(QFile);
+ if (d->finalized)
+ return false;
+ d->finalized = true;
+ close();
+ if (error() != NoError) {
+ remove();
+ return false;
+ }
+
+ if (!fileEngine()->rename(d->fileName)) { // atomically replace old file with new file. TODO works on Windows?
+ remove();
+ return false;
+ }
+ fileEngine()->setFileName(d->fileName);
+
+ return true;
+}
+
/*!
\overload
diff --git a/tests/auto/corelib/io/qfile/tst_qfile.cpp b/tests/auto/corelib/io/qfile/tst_qfile.cpp
index fdb0a7e..ac32c74 100644
--- a/tests/auto/corelib/io/qfile/tst_qfile.cpp
+++ b/tests/auto/corelib/io/qfile/tst_qfile.cpp
@@ -216,6 +216,8 @@ private slots:
void openDirectory();
void writeNothing();
+ void useTemporary();
+
public:
// disabled this test for the moment... it hangs
void invalidFile_data();
@@ -1218,7 +1220,7 @@ void tst_QFile::copyFallback()
QVERIFY(QFile::exists("file-copy-destination.txt"));
QVERIFY(!file.isOpen());
- file.close();
+ file.close();
QFile::remove("file-copy-destination.txt");
}
@@ -2812,8 +2814,8 @@ void tst_QFile::map()
// exotic test to make sure that multiple maps work
// note: windows ce does not reference count mutliple maps
- // it's essentially just the same reference but it
- // cause a resource lock on the file which prevents it
+ // it's essentially just the same reference but it
+ // cause a resource lock on the file which prevents it
// from being removed uchar *memory1 = file.map(0, file.size());
uchar *memory1 = file.map(0, file.size());
QCOMPARE(file.error(), QFile::NoError);
@@ -3189,5 +3191,36 @@ void tst_QFile::autocloseHandle()
}
}
+void tst_QFile::useTemporary()
+{
+ // Test basic functionality
+
+ const QString targetFile = QString::fromLatin1("outfile");
+ QFile::remove(targetFile);
+ QFile file(targetFile);
+ file.setUseTemporaryFile(true);
+ QVERIFY(file.open(QIODevice::WriteOnly));
+ QCOMPARE(file.fileName(), targetFile);
+ QVERIFY(file.exists()); // the tempfile exists
+ QVERIFY(!QFile::exists(targetFile));
+
+ QTextStream ts(&file);
+ ts << "This is test data one.\n";
+ ts.flush();
+ QCOMPARE(file.error(), QFile::NoError);
+ QVERIFY(!QFile::exists(targetFile));
+
+ QVERIFY(file.commit());
+ QVERIFY(QFile::exists(targetFile));
+ QCOMPARE(file.fileName(), targetFile);
+
+ file.remove();
+ QVERIFY(!QFile::exists(targetFile));
+}
+
+// TODO test rollback
+// TODO test write error?
+
QTEST_MAIN(tst_QFile)
+
#include "tst_qfile.moc"_______________________________________________
Development mailing list
Development@qt-project.org
http://lists.qt-project.org/mailman/listinfo/development
