add write error handling to QTextStream

add new status flag WriteFailed.
use it in flushWriteBuffer().
remove the boolean return values from write(), putString() and
putNumber(), as they were ignored anyway - flushWriteBuffer() does it
correctly now.

Task-number: QTBUG-376
Reviewed-by: mariusSO

3 files changed, 64 insertions, 18 deletions

diff --git a/src/corelib/io/qtextstream.cpp b/src/corelib/io/qtextstream.cpp
index 560766a..242c672 100644
--- a/src/corelib/io/qtextstream.cpp
+++ b/src/corelib/io/qtextstream.cpp
@@ -396,19 +396,19 @@ public:
         npsInvalidPrefix
     };
 
-    inline bool write(const QString &data);
     inline bool getChar(QChar *ch);
     inline void ungetChar(const QChar &ch);
     NumberParsingStatus getNumber(qulonglong *l);
     bool getReal(double *f);
 
-    bool putNumber(qulonglong number, bool negative);
-    inline bool putString(const QString &ch, bool number = false);
+    inline void write(const QString &data);
+    inline void putString(const QString &ch, bool number = false);
+    void putNumber(qulonglong number, bool negative);
 
     // buffers
     bool fillReadBuffer(qint64 maxBytes = -1);
     void resetReadBuffer();
-    bool flushWriteBuffer();
+    void flushWriteBuffer();
     QString writeBuffer;
     QString readBuffer;
     int readBufferOffset;
@@ -642,14 +642,20 @@ void QTextStreamPrivate::resetReadBuffer()
 
 /*! \internal
 */
-bool QTextStreamPrivate::flushWriteBuffer()
+void QTextStreamPrivate::flushWriteBuffer()
 {
     // no buffer next to the QString itself; this function should only
     // be called internally, for devices.
     if (string || !device)
-        return false;
+        return;
+
+    // Stream went bye-bye already. Appending further data may succeed again,
+    // but would create a corrupted stream anyway.
+    if (status != QTextStream::Ok)
+        return;
+
     if (writeBuffer.isEmpty())
-        return true;
+        return;
 
 #if defined (Q_OS_WIN)
     // handle text translation and bypass the Text flag in the device.
@@ -681,8 +687,10 @@ bool QTextStreamPrivate::flushWriteBuffer()
     qDebug("QTextStreamPrivate::flushWriteBuffer(), device->write(\"%s\") == %d",
            qt_prettyDebug(data.constData(), qMin(data.size(),32), data.size()).constData(), int(bytesWritten));
 #endif
-    if (bytesWritten <= 0)
-        return false;
+    if (bytesWritten <= 0) {
+        status = QTextStream::WriteFailed;
+        return;
+    }
 
 #if defined (Q_OS_WIN)
     // replace the text flag
@@ -702,7 +710,8 @@ bool QTextStreamPrivate::flushWriteBuffer()
     qDebug("QTextStreamPrivate::flushWriteBuffer() wrote %d bytes",
            int(bytesWritten));
 #endif
-    return flushed && bytesWritten == qint64(data.size());
+    if (!flushed || bytesWritten != qint64(data.size()))
+        status = QTextStream::WriteFailed;
 }
 
 QString QTextStreamPrivate::read(int maxlen)
@@ -908,7 +917,7 @@ inline void QTextStreamPrivate::restoreToSavedConverterState()
 
 /*! \internal
 */
-inline bool QTextStreamPrivate::write(const QString &data)
+inline void QTextStreamPrivate::write(const QString &data)
 {
     if (string) {
         // ### What about seek()??
@@ -916,9 +925,8 @@ inline bool QTextStreamPrivate::write(const QString &data)
     } else {
         writeBuffer += data;
         if (writeBuffer.size() > QTEXTSTREAM_BUFFERSIZE)
-            return flushWriteBuffer();
+            flushWriteBuffer();
     }
-    return true;
 }
 
 /*! \internal
@@ -959,7 +967,7 @@ inline void QTextStreamPrivate::ungetChar(const QChar &ch)
 
 /*! \internal
 */
-inline bool QTextStreamPrivate::putString(const QString &s, bool number)
+inline void QTextStreamPrivate::putString(const QString &s, bool number)
 {
     QString tmp = s;
 
@@ -993,7 +1001,7 @@ inline bool QTextStreamPrivate::putString(const QString &s, bool number)
            qt_prettyDebug(a.constData(), a.size(), qMax(16, a.size())).constData(),
            qt_prettyDebug(b.constData(), b.size(), qMax(16, b.size())).constData());
 #endif
-    return write(tmp);
+    write(tmp);
 }
 
 /*!
@@ -2270,7 +2278,7 @@ QTextStream &QTextStream::operator>>(char *c)
 
 /*! \internal
  */
-bool QTextStreamPrivate::putNumber(qulonglong number, bool negative)
+void QTextStreamPrivate::putNumber(qulonglong number, bool negative)
 {
     QString result;
 
@@ -2310,7 +2318,7 @@ bool QTextStreamPrivate::putNumber(qulonglong number, bool negative)
             result.prepend(QLatin1Char('0'));
         }
     }
-    return putString(result, true);
+    putString(result, true);
 }
 
 /*!
diff --git a/src/corelib/io/qtextstream.h b/src/corelib/io/qtextstream.h
index d82da59..c635691 100644
--- a/src/corelib/io/qtextstream.h
+++ b/src/corelib/io/qtextstream.h
@@ -89,7 +89,8 @@ public:
     enum Status {
         Ok,
         ReadPastEnd,
-        ReadCorruptData
+        ReadCorruptData,
+        WriteFailed
     };
     enum NumberFlag {
         ShowBase = 0x1,
diff --git a/tests/auto/qtextstream/tst_qtextstream.cpp b/tests/auto/qtextstream/tst_qtextstream.cpp
index 4c78ef0..005f686 100644
--- a/tests/auto/qtextstream/tst_qtextstream.cpp
+++ b/tests/auto/qtextstream/tst_qtextstream.cpp
@@ -228,6 +228,7 @@ private slots:
     void status_real_read();
     void status_integer_read();
     void status_word_read();
+    void status_write_error();
 
     // use case tests
     void useCase1();
@@ -4176,6 +4177,42 @@ void tst_QTextStream::status_word_read()
     QCOMPARE(s.status(), QTextStream::ReadPastEnd);
 }
 
+class FakeBuffer : public QBuffer
+{
+protected:
+    qint64 writeData(const char *c, qint64 i) { return m_lock ? 0 : QBuffer::writeData(c, i); }
+public:
+    FakeBuffer(bool locked = false) : m_lock(locked) {}
+    void setLocked(bool locked) { m_lock = locked; }
+private:
+    bool m_lock;
+};
+
+void tst_QTextStream::status_write_error()
+{
+    FakeBuffer fb(false);
+    QVERIFY(fb.open(QBuffer::ReadWrite));
+    QTextStream fs(&fb);
+    fs.setCodec(QTextCodec::codecForName("latin1"));
+    /* first write some initial content */
+    fs << "hello";
+    fs.flush();
+    QCOMPARE(fs.status(), QTextStream::Ok);
+    QCOMPARE(fb.data(), QByteArray("hello"));
+    /* then test that writing can cause an error */
+    fb.setLocked(true);
+    fs << "error";
+    fs.flush();
+    QCOMPARE(fs.status(), QTextStream::WriteFailed);
+    QCOMPARE(fb.data(), QByteArray("hello"));
+    /* finally test that writing after an error doesn't change the stream any more */
+    fb.setLocked(false);
+    fs << "can't do that";
+    fs.flush();
+    QCOMPARE(fs.status(), QTextStream::WriteFailed);
+    QCOMPARE(fb.data(), QByteArray("hello"));
+}
+
 void tst_QTextStream::task180679_alignAccountingStyle()
 {
     {