From 7c06af4528d7aa7186b12546c261d5d2a0a3641d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Thu, 27 Aug 2009 18:56:47 +0200 Subject: Reset QTemporaryFile's state after failed open() on Windows This fixes a regression introduced in 4.5.2 where QTemporaryFile would no longer attempt to generate a random name after a failed open. Under certain situations, this led to a non-random file being left behind in QDir::tempPath when using the fallback implementation of QFile::copy. Avoid calling QFSFileEngine::setFileName() on a template, so as not to process it as file name. By consistently not calling setFileTemplate in the constructor, we also delay allocation of the fileEngine. Changes made to that function also keep it from unnecessarily allocating the fileEngine. Task-number: 260165 Reviewed-by: Thiago Macieira --- src/corelib/io/qtemporaryfile.cpp | 38 ++++++++--- tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp | 82 ++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 8 deletions(-) diff --git a/src/corelib/io/qtemporaryfile.cpp b/src/corelib/io/qtemporaryfile.cpp index bd039c8..47fa257 100644 --- a/src/corelib/io/qtemporaryfile.cpp +++ b/src/corelib/io/qtemporaryfile.cpp @@ -292,14 +292,20 @@ class QTemporaryFileEngine : public QFSFileEngine Q_DECLARE_PRIVATE(QFSFileEngine) public: QTemporaryFileEngine(const QString &file, bool fileIsTemplate = true) - : QFSFileEngine(file), filePathIsTemplate(fileIsTemplate) + : QFSFileEngine(), filePathIsTemplate(fileIsTemplate) { + Q_D(QFSFileEngine); + d->filePath = file; + + if (!filePathIsTemplate) + QFSFileEngine::setFileName(file); } ~QTemporaryFileEngine(); bool isReallyOpen(); void setFileName(const QString &file); + void setFileTemplate(const QString &fileTemplate); bool open(QIODevice::OpenMode flags); bool remove(); @@ -336,6 +342,13 @@ void QTemporaryFileEngine::setFileName(const QString &file) QFSFileEngine::setFileName(file); } +void QTemporaryFileEngine::setFileTemplate(const QString &fileTemplate) +{ + Q_D(QFSFileEngine); + if (filePathIsTemplate) + d->filePath = fileTemplate; +} + bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode) { Q_D(QFSFileEngine); @@ -382,12 +395,19 @@ bool QTemporaryFileEngine::open(QIODevice::OpenMode openMode) return false; } + QString template_ = d->filePath; d->filePath = QString::fromLocal8Bit(filename); - filePathIsTemplate = false; d->nativeInitFileName(); - d->closeFileHandle = true; delete [] filename; - return QFSFileEngine::open(openMode); + + if (QFSFileEngine::open(openMode)) { + filePathIsTemplate = false; + return true; + } + + d->filePath = template_; + d->nativeFilePath.clear(); + return false; #endif } @@ -533,7 +553,8 @@ QTemporaryFile::QTemporaryFile() QTemporaryFile::QTemporaryFile(const QString &templateName) : QFile(*new QTemporaryFilePrivate, 0) { - setFileTemplate(templateName); + Q_D(QTemporaryFile); + d->templateName = templateName; } /*! @@ -567,7 +588,8 @@ QTemporaryFile::QTemporaryFile(QObject *parent) QTemporaryFile::QTemporaryFile(const QString &templateName, QObject *parent) : QFile(*new QTemporaryFilePrivate, parent) { - setFileTemplate(templateName); + Q_D(QTemporaryFile); + d->templateName = templateName; } #endif @@ -671,10 +693,10 @@ QString QTemporaryFile::fileTemplate() const */ void QTemporaryFile::setFileTemplate(const QString &name) { - Q_ASSERT(!isOpen()); Q_D(QTemporaryFile); - fileEngine()->setFileName(name); d->templateName = name; + if (d->fileEngine) + static_cast(d->fileEngine)->setFileTemplate(name); } /*! diff --git a/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp b/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp index 2541b48..73486e7 100644 --- a/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp +++ b/tests/auto/qtemporaryfile/tst_qtemporaryfile.cpp @@ -89,6 +89,8 @@ private slots: void renameFdLeak(); void reOpenThroughQFile(); void keepOpenMode(); + void resetTemplateAfterError(); + void setTemplateAfterOpen(); public: }; @@ -472,5 +474,85 @@ void tst_QTemporaryFile::keepOpenMode() } } +void tst_QTemporaryFile::resetTemplateAfterError() +{ + // calling setFileTemplate on a failed open + + QString tempPath = QDir::tempPath(); + + QString const fileTemplate("destination/qt_temp_file_test.XXXXXX"); + QString const fileTemplate2(tempPath + "/qt_temp_file_test.XXXXXX"); + + QVERIFY2( QDir(tempPath).exists() || QDir().mkpath(tempPath), "Test precondition" ); + QVERIFY2( !QFile::exists("destination"), "Test precondition" ); + QVERIFY2( !QFile::exists(fileTemplate2) || QFile::remove(fileTemplate2), "Test precondition" ); + + QFile file(fileTemplate2); + QByteArray fileContent("This file is intentionally NOT left empty."); + + QVERIFY( file.open(QIODevice::ReadWrite | QIODevice::Truncate) ); + QCOMPARE( file.write(fileContent), (qint64)fileContent.size() ); + QVERIFY( file.flush() ); + + QString fileName; + { + QTemporaryFile temp; + + QVERIFY( temp.fileName().isEmpty() ); + QVERIFY( !temp.fileTemplate().isEmpty() ); + + temp.setFileTemplate( fileTemplate ); + + QVERIFY( temp.fileName().isEmpty() ); + QCOMPARE( temp.fileTemplate(), fileTemplate ); + + QVERIFY( !temp.open() ); + + QVERIFY( temp.fileName().isEmpty() ); + QCOMPARE( temp.fileTemplate(), fileTemplate ); + + temp.setFileTemplate( fileTemplate2 ); + QVERIFY( temp.open() ); + + fileName = temp.fileName(); + QVERIFY( QFile::exists(fileName) ); + QVERIFY( !fileName.isEmpty() ); + QVERIFY2( fileName != fileTemplate2, + ("Generated name shouldn't be same as template: " + fileTemplate2).toLocal8Bit().constData() ); + } + + QVERIFY( !QFile::exists(fileName) ); + + file.seek(0); + QCOMPARE( QString(file.readAll()), QString(fileContent) ); + QVERIFY( file.remove() ); +} + +void tst_QTemporaryFile::setTemplateAfterOpen() +{ + QTemporaryFile temp; + + QVERIFY( temp.fileName().isEmpty() ); + QVERIFY( !temp.fileTemplate().isEmpty() ); + + QVERIFY( temp.open() ); + + QString const fileName = temp.fileName(); + QString const newTemplate("funny-path/funny-name-XXXXXX.tmp"); + + QVERIFY( !fileName.isEmpty() ); + QVERIFY( QFile::exists(fileName) ); + QVERIFY( !temp.fileTemplate().isEmpty() ); + QVERIFY( temp.fileTemplate() != newTemplate ); + + temp.close(); // QTemporaryFile::setFileTemplate will assert on isOpen() up to 4.5.2 + temp.setFileTemplate(newTemplate); + QCOMPARE( temp.fileTemplate(), newTemplate ); + + QVERIFY( temp.open() ); + QCOMPARE( temp.fileName(), fileName ); + QCOMPARE( temp.fileTemplate(), newTemplate ); +} + QTEST_MAIN(tst_QTemporaryFile) #include "tst_qtemporaryfile.moc" -- cgit v0.12