diff --git a/src/corelib/io/qfiledevice.cpp b/src/corelib/io/qfiledevice.cpp index 431dc65f5b8..3ce75bd3237 100644 --- a/src/corelib/io/qfiledevice.cpp +++ b/src/corelib/io/qfiledevice.cpp @@ -338,7 +338,8 @@ void QFileDevice::close() // reset cached size d->cachedSize = 0; - // keep earlier error from flush + // If flush() succeeded but close() failed, copy its error condition; + // otherwise, keep the earlier flush() error. if (d->fileEngine->close() && flushed) unsetError(); else if (flushed) diff --git a/src/corelib/io/qsavefile.cpp b/src/corelib/io/qsavefile.cpp index 073992264af..b571c06c1a6 100644 --- a/src/corelib/io/qsavefile.cpp +++ b/src/corelib/io/qsavefile.cpp @@ -302,10 +302,15 @@ bool QSaveFile::commit() // Sync to disk if possible. Ignore errors (e.g. not supported). fe->syncToDisk(); + // ensure we act on either a close()/flush() failure or a previous write() + // problem + if (d->error == QFileDevice::NoError) + d->error = d->writeError; + d->writeError = QFileDevice::NoError; + if (d->useTemporaryFile) { - if (d->writeError != QFileDevice::NoError) { + if (d->error != QFileDevice::NoError) { fe->remove(); - d->writeError = QFileDevice::NoError; return false; } // atomically replace old file with new file @@ -317,7 +322,10 @@ bool QSaveFile::commit() return false; } } - return true; + + // return true if all previous write() calls succeeded and if close() and + // flush() succeeded. + return d->error == QFileDevice::NoError; } /*! diff --git a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp index c027f8a3c11..cf9a9834bff 100644 --- a/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp +++ b/tests/auto/corelib/io/qsavefile/tst_qsavefile.cpp @@ -12,6 +12,9 @@ #include #if defined(Q_OS_UNIX) +#include +#include +#include #include // for geteuid #endif @@ -65,6 +68,15 @@ private slots: void symlink(); void directory(); +#ifdef Q_OS_UNIX + void writeFailToDevFull_data(); + void writeFailToDevFull(); +#endif +#if defined(RLIMIT_FSIZE) && (defined(Q_OS_LINUX) || defined(Q_OS_MACOS)) + void writeFailResourceLimit_data(); + void writeFailResourceLimit(); +#endif + #ifdef Q_OS_WIN void alternateDataStream_data(); void alternateDataStream(); @@ -288,8 +300,8 @@ void tst_QSaveFile::transactionalWriteNoPermissionsOnDir() QVERIFY2(file.open(QIODevice::WriteOnly), msgCannotOpen(file).constData()); QCOMPARE((int)file.error(), (int)QFile::NoError); QCOMPARE(file.write("W"), Q_INT64_C(1)); - file.cancelWriting(); // no effect, as per the documentation - QVERIFY(file.commit()); + file.cancelWriting(); + QVERIFY(!file.commit()); QVERIFY(reader.open(QIODevice::ReadOnly)); QCOMPARE(QString::fromLatin1(reader.readAll()), QString::fromLatin1("W")); @@ -525,6 +537,123 @@ void tst_QSaveFile::directory() #endif } +[[maybe_unused]] static void bufferedAndUnbuffered() +{ + QTest::addColumn("mode"); + QTest::newRow("unbuffered") << QIODevice::OpenMode(QIODevice::Unbuffered); + QTest::newRow("buffered") << QIODevice::OpenMode(); +} + +#ifdef Q_OS_UNIX +void tst_QSaveFile::writeFailToDevFull_data() +{ + // check if /dev/full exists and is writable + if (access("/dev/full", W_OK) != 0) + QSKIP("/dev/full either does not exist or is not writable"); + if (access("/dev", W_OK) == 0) + QSKIP("/dev is writable (running as root?): this test would replace /dev/full"); + + bufferedAndUnbuffered(); +} + +void tst_QSaveFile::writeFailToDevFull() +{ + QFETCH(QIODevice::OpenMode, mode); + mode |= QIODevice::WriteOnly; + + QSaveFile saveFile("/dev/full"); + saveFile.setDirectWriteFallback(true); + + QVERIFY2(saveFile.open(mode), msgCannotOpen(saveFile).constData()); + + QByteArray data("abc"); + qint64 written = saveFile.write(data); + if (mode & QIODevice::Unbuffered) { + // error reported immediately + QCOMPARE(written, -1); + QCOMPARE(saveFile.error(), QFile::ResourceError); + QCOMPARE(saveFile.errorString(), qt_error_string(ENOSPC)); + } else { + // error reported only on .commit() + QCOMPARE(written, data.size()); + QCOMPARE(saveFile.error(), QFile::NoError); + } + QVERIFY(!saveFile.commit()); + QCOMPARE(saveFile.error(), QFile::ResourceError); +} +#endif // Q_OS_UNIX +#if defined(RLIMIT_FSIZE) && (defined(Q_OS_LINUX) || defined(Q_OS_MACOS)) +// This test is only enabled on Linux and on macOS because we can verify that +// those OSes do respect RLIMIT_FSIZE. We can also verify that some other Unix +// OSes do not. + +void tst_QSaveFile::writeFailResourceLimit_data() +{ + bufferedAndUnbuffered(); +} + +void tst_QSaveFile::writeFailResourceLimit() +{ + // don't make it too small because stdout may be a log file! + static constexpr qint64 FileSizeLimit = 1024 * 1024; + struct RlimitChanger { + struct rlimit old; + RlimitChanger() + { + getrlimit(RLIMIT_FSIZE, &old); + struct rlimit newLimit = {}; + newLimit.rlim_cur = FileSizeLimit; + newLimit.rlim_max = old.rlim_max; + if (setrlimit(RLIMIT_FSIZE, &newLimit) != 0) + old.rlim_cur = 0; + + // ignore SIGXFSZ so we get EF2BIG when writing the file + signal(SIGXFSZ, SIG_IGN); + } + ~RlimitChanger() + { + if (old.rlim_cur) + setrlimit(RLIMIT_FSIZE, &old); + } + }; + + QFETCH(QIODevice::OpenMode, mode); + mode |= QIODevice::WriteOnly; + + QTemporaryDir dir; + QVERIFY2(dir.isValid(), qPrintable(dir.errorString())); + const QString targetFile = dir.path() + QString::fromLatin1("/outfile"); + QFile::remove(targetFile); + QSaveFile saveFile(targetFile); + QVERIFY2(saveFile.open(mode), msgCannotOpen(saveFile).constData()); + + RlimitChanger changer; + if (changer.old.rlim_cur == 0) + QSKIP("Could not set the file size resource limit"); + + QByteArray data(FileSizeLimit + 16, 'a'); + qint64 written = 0, lastWrite; + do { + lastWrite = saveFile.write(data.constData() + written, data.size() - written); + if (lastWrite > 0) + written += lastWrite; + } while (lastWrite > 0 && written < data.size()); + if (mode & QIODevice::Unbuffered) { + // error reported immediately + QCOMPARE_LT(written, data.size()); + QCOMPARE(lastWrite, -1); + QCOMPARE(saveFile.error(), QFile::WriteError); + QCOMPARE(saveFile.errorString(), qt_error_string(EFBIG)); + } else { + // error reported only on .commit() + QCOMPARE(written, data.size()); + QCOMPARE(saveFile.error(), QFile::NoError); + } + QVERIFY(!saveFile.commit()); + QCOMPARE(saveFile.error(), QFile::WriteError); +} +#endif // RLIMIT_FSIZE && (Linux or macOS) + #ifdef Q_OS_WIN void tst_QSaveFile::alternateDataStream_data() {