From ba2f7945965d9d84d802ab0bda04664229209d3f Mon Sep 17 00:00:00 2001 From: David Faure Date: Mon, 22 Apr 2024 18:32:25 +0200 Subject: [PATCH] QSaveFile: don't reset fileEngine after commit() [ChangeLog][QtCore][QSaveFile] Member functions such as fileTime() and size() now continue to work after commit(). Fixes: QTBUG-66605 Fixes: QTBUG-77039 Change-Id: I0b17eae9493911d1a0828b589d762bd8eb344bce Reviewed-by: Ahmad Samir Reviewed-by: Thiago Macieira --- src/corelib/io/qsavefile.cpp | 8 +++---- .../corelib/io/qsavefile/tst_qsavefile.cpp | 24 +++++++++++++++++++ 2 files changed, 28 insertions(+), 4 deletions(-) diff --git a/src/corelib/io/qsavefile.cpp b/src/corelib/io/qsavefile.cpp index 6d8918c29c..cc59bb3725 100644 --- a/src/corelib/io/qsavefile.cpp +++ b/src/corelib/io/qsavefile.cpp @@ -113,10 +113,10 @@ QSaveFile::QSaveFile(const QString &name, QObject *parent) QSaveFile::~QSaveFile() { Q_D(QSaveFile); - QFileDevice::close(); - if (d->fileEngine) { + if (isOpen()) { + QFileDevice::close(); + Q_ASSERT(d->fileEngine); d->fileEngine->remove(); - d->fileEngine.reset(); } } @@ -298,7 +298,7 @@ bool QSaveFile::commit() } QFileDevice::close(); // calls flush() - const auto fe = std::move(d->fileEngine); + const auto &fe = d->fileEngine; // Sync to disk if possible. Ignore errors (e.g. not supported). fe->syncToDisk(); diff --git a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp index d1f6916233..c027f8a3c1 100644 --- a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp +++ b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp @@ -5,6 +5,7 @@ #include #include #include +#include #include #include #include @@ -93,14 +94,30 @@ void tst_QSaveFile::transactionalWrite() QCOMPARE(file.write(data), qint64(strlen(data))); QCOMPARE(file.error(), QFile::NoError); QVERIFY(!QFile::exists(targetFile)); + QVERIFY(file.fileTime(QFile::FileModificationTime).isValid()); QVERIFY(file.commit()); QVERIFY(QFile::exists(targetFile)); QCOMPARE(file.fileName(), targetFile); +#if defined(Q_OS_WIN) + // Without this delay, file.fileTime() and file.size() tests fail to + // pass on Windows in the CI. It passes locally in a VM, so it looks like + // it depends on how often different filesystems on different OSes, update + // their metadata. + // Interestingly, this delay is enough to fix similar tests in the rest + // of tst_QSaveFile's functions. + QTRY_VERIFY(file.fileTime(QFile::FileModificationTime).isValid()); +#else + QVERIFY(file.fileTime(QFile::FileModificationTime).isValid()); +#endif + + QCOMPARE(file.size(), qint64(strlen(data))); QFile reader(targetFile); QVERIFY(reader.open(QIODevice::ReadOnly)); QCOMPARE(QString::fromLatin1(reader.readAll()), QString::fromLatin1(data)); + QCOMPARE(file.fileTime(QFile::FileModificationTime), + reader.fileTime(QFile::FileModificationTime)); // check that permissions are the same as for QFile const QString otherFile = dir.path() + QString::fromLatin1("/otherfile"); @@ -147,6 +164,7 @@ void tst_QSaveFile::retryTransactionalWrite() QCOMPARE(file.write(data), qint64(strlen(data))); QCOMPARE(file.error(), QFile::NoError); QVERIFY(file.commit()); + QCOMPARE(file.size(), qint64(strlen(data))); } void tst_QSaveFile::saveTwice() @@ -161,11 +179,13 @@ void tst_QSaveFile::saveTwice() QVERIFY2(file.open(QIODevice::WriteOnly), msgCannotOpen(file).constData()); QCOMPARE(file.write(hello), qint64(strlen(hello))); QVERIFY2(file.commit(), qPrintable(file.errorString())); + QCOMPARE(file.size(), qint64(strlen(hello))); const char *world = "World"; QVERIFY2(file.open(QIODevice::WriteOnly), msgCannotOpen(file).constData()); QCOMPARE(file.write(world), qint64(strlen(world))); QVERIFY2(file.commit(), qPrintable(file.errorString())); + QCOMPARE(file.size(), qint64(strlen(world))); QFile reader(targetFile); QVERIFY2(reader.open(QIODevice::ReadOnly), msgCannotOpen(reader).constData()); @@ -188,6 +208,7 @@ void tst_QSaveFile::textStreamManualFlush() QVERIFY(!QFile::exists(targetFile)); QVERIFY(file.commit()); + QCOMPARE(file.size(), qint64(strlen(data))); QFile reader(targetFile); QVERIFY(reader.open(QIODevice::ReadOnly)); QCOMPARE(QString::fromLatin1(reader.readAll().constData()), QString::fromLatin1(data)); @@ -437,6 +458,7 @@ void tst_QSaveFile::symlink() QVERIFY(saveFile.open(QIODevice::WriteOnly)); QCOMPARE(saveFile.write(someData), someData.size()); saveFile.commit(); + QCOMPARE(saveFile.size(), someData.size()); QFile file(targetFile); QVERIFY2(file.open(QIODevice::ReadOnly), msgCannotOpen(file).constData()); @@ -466,6 +488,7 @@ void tst_QSaveFile::symlink() QVERIFY(saveFile.open(QIODevice::WriteOnly)); QCOMPARE(saveFile.write(someData), someData.size()); saveFile.commit(); + QCOMPARE(saveFile.size(), someData.size()); // the explicit file becomes a file instead of a link QVERIFY(!QFileInfo(cyclicLink + QLatin1Char('1')).isSymLink()); @@ -545,6 +568,7 @@ void tst_QSaveFile::alternateDataStream() QVERIFY2(file.open(QIODevice::WriteOnly), qPrintable(file.errorString())); file.write(newContent); QVERIFY2(file.commit(), qPrintable(file.errorString())); + QCOMPARE(file.size(), qint64(strlen(newContent))); // check the contents QFile targetFile(adsName);