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

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());
     }