From 3b57425aa11e5e1536016ccccdde7657493a0dc8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Thu, 7 Jan 2010 13:31:08 +0100 Subject: Avoid double-buffering in QFile Since Qt 4.3 QIODevice has been providing read buffering for buffered devices; QFile provides a write buffer. Thus, requesting a buffered file from the engine results in unnecessary double-buffering where this is supported natively. By preferring QFile/QIODevice's buffering over the file engine we reduce the number of system calls. On the other hand, buffering inside QIODevice can't easily be disabled without changing the return value of QIODevice::openMode() (function is non-virtual). Reviewed-by: Thiago Macieira --- src/corelib/io/qfile.cpp | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index 72d1d16..eb6f5be 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -968,9 +968,6 @@ bool QFile::isSequential() const mode, if the relevant file does not already exist, this function will try to create a new file before opening it. - \note Because of limitations in the native API, QFile ignores the - Unbuffered flag on Windows. - \sa QIODevice::OpenMode, setFileName() */ bool QFile::open(OpenMode mode) @@ -988,7 +985,9 @@ bool QFile::open(OpenMode mode) qWarning("QIODevice::open: File access not specified"); return false; } - if (fileEngine()->open(mode)) { + + // QIODevice provides the buffering, so there's no need to request it from the file engine. + if (fileEngine()->open(mode | QIODevice::Unbuffered)) { QIODevice::open(mode); if (mode & Append) seek(size()); -- cgit v0.12 From 6bf1674f1e51fd8b08783035cda7493ecd63b443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Thu, 7 Jan 2010 14:50:14 +0100 Subject: Don't drop errors from flush on QFile::close Now that we don't defer to the file engine for the write buffer it is doubly more important to retain errors from flush. The logic employed reproduces what we already had in QFSFileEngine::close, earliest error stays. Reviewed-by: Thiago Macieira --- src/corelib/io/qfile.cpp | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index eb6f5be..b6fa5f3 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -1386,11 +1386,14 @@ QFile::close() Q_D(QFile); if(!isOpen()) return; - flush(); + bool flushed = flush(); QIODevice::close(); - unsetError(); - if(!fileEngine()->close()) + + // keep earlier error from flush + if (fileEngine()->close() && flushed) + unsetError(); + else if (flushed) d->setError(fileEngine()->error(), fileEngine()->errorString()); } -- cgit v0.12 From 183be5348e53ef1df9a6e1eb82f10f24ce612f40 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Thu, 7 Jan 2010 15:54:06 +0100 Subject: Fix bugs evidenced by change to keep flush errors on QFile::close When closing a file, the contents of the write buffer should be emptied even in the case where flush fails, so it doesn't leak to subsequent files opened through the same instance. This is inline with what would happen with native close on a buffered device. Also changed the resource file engine to succeed on flush. Since all writes fail there, logically it's write buffer is empty and flush should succeed. This keeps auto-tests happy :-) tst_QFile::fullDisk auto-test extended to ensure re-opening QFile does not keep the write buffer alive. Reviewed-by: Thiago Macieira --- src/corelib/io/qfile.cpp | 3 +++ src/corelib/io/qresource.cpp | 2 +- tests/auto/qfile/tst_qfile.cpp | 4 +++- 3 files changed, 7 insertions(+), 2 deletions(-) diff --git a/src/corelib/io/qfile.cpp b/src/corelib/io/qfile.cpp index b6fa5f3..6395cc7 100644 --- a/src/corelib/io/qfile.cpp +++ b/src/corelib/io/qfile.cpp @@ -1389,6 +1389,9 @@ QFile::close() bool flushed = flush(); QIODevice::close(); + // reset write buffer + d->lastWasWrite = false; + d->writeBuffer.clear(); // keep earlier error from flush if (fileEngine()->close() && flushed) diff --git a/src/corelib/io/qresource.cpp b/src/corelib/io/qresource.cpp index 3d54969..adfbb15 100644 --- a/src/corelib/io/qresource.cpp +++ b/src/corelib/io/qresource.cpp @@ -1285,7 +1285,7 @@ bool QResourceFileEngine::close() bool QResourceFileEngine::flush() { - return false; + return true; } qint64 QResourceFileEngine::read(char *data, qint64 len) diff --git a/tests/auto/qfile/tst_qfile.cpp b/tests/auto/qfile/tst_qfile.cpp index 453434d..f407b12 100644 --- a/tests/auto/qfile/tst_qfile.cpp +++ b/tests/auto/qfile/tst_qfile.cpp @@ -2119,15 +2119,17 @@ void tst_QFile::fullDisk() file.write(&c, 0); QVERIFY(!file.flush()); QCOMPARE(file.error(), QFile::ResourceError); - file.write(&c, 1); + QCOMPARE(file.write(&c, 1), qint64(1)); QVERIFY(!file.flush()); QCOMPARE(file.error(), QFile::ResourceError); file.close(); QVERIFY(!file.isOpen()); QCOMPARE(file.error(), QFile::ResourceError); + file.open(QIODevice::WriteOnly); QCOMPARE(file.error(), QFile::NoError); + QVERIFY(file.flush()); // Shouldn't inherit write buffer file.close(); QCOMPARE(file.error(), QFile::NoError); -- cgit v0.12 From 3e0d30cbb7f5b8faeb7c7389b935c37da47f6944 Mon Sep 17 00:00:00 2001 From: Thiago Macieira Date: Thu, 7 Jan 2010 11:09:47 +0100 Subject: Autotest: rename test with typo in the name --- tests/auto/qnetworkreply/tst_qnetworkreply.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp index 8f393fe..20cf922 100644 --- a/tests/auto/qnetworkreply/tst_qnetworkreply.cpp +++ b/tests/auto/qnetworkreply/tst_qnetworkreply.cpp @@ -223,7 +223,7 @@ private Q_SLOTS: void ioPostToHttpFromMiddleOfQBufferFiveBytes(); void ioPostToHttpNoBufferFlag(); void ioPostToHttpUploadProgress(); - void ioPostToHttpEmtpyUploadProgress(); + void ioPostToHttpEmptyUploadProgress(); void lastModifiedHeaderForFile(); void lastModifiedHeaderForHttp(); @@ -2921,7 +2921,7 @@ void tst_QNetworkReply::ioPostToHttpUploadProgress() server.close(); } -void tst_QNetworkReply::ioPostToHttpEmtpyUploadProgress() +void tst_QNetworkReply::ioPostToHttpEmptyUploadProgress() { QByteArray ba; ba.resize(0); -- cgit v0.12