From 1e6b424b692b20dcfec920f8d3563e520ec1ff05 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Abecasis?= Date: Mon, 16 Nov 2009 17:38:18 +0100 Subject: Removing unnecessary chunking and stat'ing when reading QIODevice Chunk size increased to QIODEVICE_BUFFERSIZE (currently 16k) where chunking is still needed. Namely, on sequential devices and when QByteArray is unable to allocate a large enough buffer. This is necessary for backward compatibility Improved validation and prevention of overflow in maxSize argument. Updated autotest that relied on a null QByteArray when no data was available and no errors were found. The only guarantee we should be providing in this case is an empty result -- even though that behavior is preserved for the time being. Affected functions: * QIODevice::read(qint64 maxSize) Chunking will still happen for large maxSize (i.e., QByteArray resize fails), where it could be used as a synonym for QIODevice::readAll(). No stat'ing performed. Read from device continues for as long as it is successful. Stops if an error occurs or if we get less data than requested. * QIODevice::readAll() Chunking is performed for sequential devices where total size wouldn't be known beforehand. For sequential devices, reading continues as long as data is returned, even if less than requested. Non-sequential devices will be stat'ed once. If QIODevice::size returns 0, this is taken to mean unknown size and chunking is performed. Otherwise, a single read request is made for the specified size. On failure to resize QByteArray, nothing is returned. * QIODevice::readLine(qint64 maxSize) Chunking is performed for maxSize == 0, or if we can't allocate a large enough buffer. No stat'ing performed at this level. Read from device continues until EOL is found, as long as we get all requested data. Task-number: QT-2347 Reviewed-by: Thiago Macieira Reviewed-by: Miikka Heikkinen --- src/corelib/io/qiodevice.cpp | 135 ++++++++++++++++++++++++----------------- tests/auto/qfile/tst_qfile.cpp | 4 +- 2 files changed, 83 insertions(+), 56 deletions(-) diff --git a/src/corelib/io/qiodevice.cpp b/src/corelib/io/qiodevice.cpp index 7ee65e1..e4e6a15 100644 --- a/src/corelib/io/qiodevice.cpp +++ b/src/corelib/io/qiodevice.cpp @@ -904,10 +904,10 @@ qint64 QIODevice::read(char *data, qint64 maxSize) QByteArray QIODevice::read(qint64 maxSize) { Q_D(QIODevice); - CHECK_MAXLEN(read, QByteArray()); - QByteArray tmp; - qint64 readSoFar = 0; - char buffer[4096]; + QByteArray result; + + CHECK_MAXLEN(read, result); + #if defined QIODEVICE_DEBUG printf("%p QIODevice::read(%d), d->pos = %d, d->buffer.size() = %d\n", this, int(maxSize), int(d->pos), int(d->buffer.size())); @@ -915,16 +915,34 @@ QByteArray QIODevice::read(qint64 maxSize) Q_UNUSED(d); #endif - do { - qint64 bytesToRead = qMin(int(maxSize - readSoFar), int(sizeof(buffer))); - qint64 readBytes = read(buffer, bytesToRead); - if (readBytes <= 0) - break; - tmp.append(buffer, (int) readBytes); - readSoFar += readBytes; - } while (readSoFar < maxSize && bytesAvailable() > 0); + if (maxSize != qint64(int(maxSize))) { + qWarning("QIODevice::read: maxSize argument exceeds QByteArray size limit"); + maxSize = INT_MAX; + } - return tmp; + qint64 readBytes = 0; + if (maxSize) { + result.resize(int(maxSize)); + if (!result.size()) { + // If resize fails, read incrementally. + qint64 readResult; + do { + result.resize(int(qMin(maxSize, result.size() + QIODEVICE_BUFFERSIZE))); + readResult = read(result.data() + readBytes, result.size() - readBytes); + if (readResult > 0 || readBytes == 0) + readBytes += readResult; + } while (readResult == QIODEVICE_BUFFERSIZE); + } else { + readBytes = read(result.data(), result.size()); + } + } + + if (readBytes <= 0) + result.clear(); + else + result.resize(int(readBytes)); + + return result; } /*! @@ -945,28 +963,30 @@ QByteArray QIODevice::readAll() this, int(d->pos), int(d->buffer.size())); #endif - QByteArray tmp; - if (d->isSequential() || size() == 0) { - // Read it in chunks. Use bytesAvailable() as an unreliable hint for - // sequential devices, but try to read 4K as a minimum. - int chunkSize = qMax(qint64(4096), bytesAvailable()); - qint64 totalRead = 0; - forever { - tmp.resize(tmp.size() + chunkSize); - qint64 readBytes = read(tmp.data() + totalRead, chunkSize); - tmp.chop(chunkSize - (readBytes < 0 ? 0 : readBytes)); - if (readBytes <= 0) - return tmp; - totalRead += readBytes; - chunkSize = qMax(qint64(4096), bytesAvailable()); - } + QByteArray result; + qint64 readBytes = 0; + if (d->isSequential() || (readBytes = size()) == 0) { + // Size is unknown, read incrementally. + qint64 readResult; + do { + result.resize(result.size() + QIODEVICE_BUFFERSIZE); + readResult = read(result.data() + readBytes, result.size() - readBytes); + if (readResult > 0 || readBytes == 0) + readBytes += readResult; + } while (readResult > 0); } else { // Read it all in one go. - tmp.resize(int(bytesAvailable())); - qint64 readBytes = read(tmp.data(), tmp.size()); - tmp.resize(readBytes < 0 ? 0 : int(readBytes)); + // If resize fails, don't read anything. + result.resize(int(readBytes - d->pos)); + readBytes = read(result.data(), result.size()); } - return tmp; + + if (readBytes <= 0) + result.clear(); + else + result.resize(int(readBytes)); + + return result; } /*! @@ -1115,11 +1135,9 @@ qint64 QIODevice::readLine(char *data, qint64 maxSize) QByteArray QIODevice::readLine(qint64 maxSize) { Q_D(QIODevice); - CHECK_MAXLEN(readLine, QByteArray()); - QByteArray tmp; - const int BufferGrowth = 4096; - qint64 readSoFar = 0; - qint64 readBytes = 0; + QByteArray result; + + CHECK_MAXLEN(readLine, result); #if defined QIODEVICE_DEBUG printf("%p QIODevice::readLine(%d), d->pos = %d, d->buffer.size() = %d\n", @@ -1128,25 +1146,34 @@ QByteArray QIODevice::readLine(qint64 maxSize) Q_UNUSED(d); #endif - do { - if (maxSize != 0) - tmp.resize(int(readSoFar + qMin(int(maxSize), BufferGrowth))); - else - tmp.resize(int(readSoFar + BufferGrowth)); - readBytes = readLine(tmp.data() + readSoFar, tmp.size() - readSoFar); - if (readBytes <= 0) - break; - - readSoFar += readBytes; - } while ((!maxSize || readSoFar < maxSize) && - readSoFar + 1 == tmp.size() && // +1 due to the ending null - tmp.at(readSoFar - 1) != '\n'); + if (maxSize > INT_MAX) { + qWarning("QIODevice::read: maxSize argument exceeds QByteArray size limit"); + maxSize = INT_MAX; + } - if (readSoFar == 0 && readBytes == -1) - tmp.clear(); // return Null if we found an error + result.resize(int(maxSize)); + qint64 readBytes = 0; + if (!result.size()) { + // If resize fails or maxSize == 0, read incrementally + if (maxSize == 0) + maxSize = INT_MAX; + qint64 readResult; + do { + result.resize(int(qMin(maxSize, result.size() + QIODEVICE_BUFFERSIZE))); + readResult = readLine(result.data() + readBytes, result.size() - readBytes); + if (readResult > 0 || readBytes == 0) + readBytes += readResult; + } while (readResult == QIODEVICE_BUFFERSIZE + && result[int(readBytes)] != '\n'); + } else + readBytes = readLine(result.data(), result.size()); + + if (readBytes <= 0) + result.clear(); else - tmp.resize(int(readSoFar)); - return tmp; + result.resize(readBytes); + + return result; } /*! diff --git a/tests/auto/qfile/tst_qfile.cpp b/tests/auto/qfile/tst_qfile.cpp index e4e63ff..cf46ce1 100644 --- a/tests/auto/qfile/tst_qfile.cpp +++ b/tests/auto/qfile/tst_qfile.cpp @@ -2491,13 +2491,13 @@ void tst_QFile::readEof() } QByteArray ret = file.read(10); - QVERIFY(ret.isNull()); + QVERIFY(ret.isEmpty()); QVERIFY(file.error() == QFile::NoError); QVERIFY(file.atEnd()); // Do it again to ensure that we get the same result ret = file.read(10); - QVERIFY(ret.isNull()); + QVERIFY(ret.isEmpty()); QVERIFY(file.error() == QFile::NoError); QVERIFY(file.atEnd()); } -- cgit v0.12