Make PerfStdin behave more like other QIODevices

Buffer the data and emit readyRead when new data is available. Avoid the
"oops" momemnt when bytesAvailable() returned >0 but trying to read the
bytes would instead close the device. In fact, that was the "normal" way
to finish the reading from stdin: We would create a garbage event header
by trying to read past the end of the input and then signal an error
because of that.

Now we can actually listen for aboutToClose() on PerfStdin and we will
eventually receive that signal.

Change-Id: Ib159d186f4ce6c69a1fc7fe37bd5673981307b68
Reviewed-by: Milian Wolff <milian.wolff@kdab.com>

8 files changed, 230 insertions, 14 deletions

diff --git a/app/main.cpp b/app/main.cpp
index 04346ff..11257ca 100644
--- a/app/main.cpp
+++ b/app/main.cpp
@@ -282,6 +282,7 @@ int main(int argc, char *argv[])
             return;
         }
 
+        QObject::connect(infile.data(), &QIODevice::aboutToClose, &data, &PerfData::finishReading);
         QObject::connect(infile.data(), &QIODevice::readyRead, &data, &PerfData::read);
         if (infile->bytesAvailable() > 0)
             data.read();
@@ -307,7 +308,8 @@ int main(int argc, char *argv[])
     } else {
         if (!infile->open(QIODevice::ReadOnly))
             return CannotOpen;
-        QMetaObject::invokeMethod(&header, "read", Qt::QueuedConnection);
+        if (qobject_cast<QFile *>(infile.data())) // We don't get readyRead then ...
+            QTimer::singleShot(0, &header, &PerfHeader::read);
     }
 
     return app.exec();
diff --git a/app/perfstdin.cpp b/app/perfstdin.cpp
index c5af9b4..af091eb 100644
--- a/app/perfstdin.cpp
+++ b/app/perfstdin.cpp
@@ -22,30 +22,59 @@
 
 #include <QTimer>
 
+#include <cstring>
 #include <cstdio>
 #include <limits>
 
+PerfStdin::PerfStdin(QObject *parent) : QIODevice(parent)
+{
+    connect(&m_timer, &QTimer::timeout, this, &PerfStdin::receiveData);
+}
+
+PerfStdin::~PerfStdin()
+{
+    if (isOpen())
+        close();
+}
+
 bool PerfStdin::open(QIODevice::OpenMode mode)
 {
     if (!(mode & QIODevice::ReadOnly) || (mode & QIODevice::WriteOnly))
         return false;
 
-    return QIODevice::open(mode);
+    if (QIODevice::open(mode)) {
+        m_buffer.resize(s_minBufferSize);
+        m_timer.start();
+        return true;
+    } else {
+        return false;
+    }
+}
+
+void PerfStdin::close()
+{
+    m_timer.stop();
+    QIODevice::close();
 }
 
 qint64 PerfStdin::readData(char *data, qint64 maxlen)
 {
     if (maxlen <= 0)
         return 0;
-    size_t read = fread(data, 1, static_cast<size_t>(maxlen), stdin);
-    if (feof(stdin) || ferror(stdin))
-        QTimer::singleShot(0, this, &QIODevice::close);
-    if (read == 0) {
-        return -1;
-    } else {
-        Q_ASSERT(read <= static_cast<size_t>(maxlen));
-        return static_cast<qint64>(read);
-    }
+
+    qint64 read = 0;
+    do {
+        Q_ASSERT(m_buffer.length() >= m_bufferPos);
+        Q_ASSERT(m_buffer.length() >= m_bufferUsed);
+        Q_ASSERT(m_bufferPos <= m_bufferUsed);
+        const size_t buffered = static_cast<size_t>(qMin(bufferedAvailable(), maxlen - read));
+        memcpy(data + read, m_buffer.constData() + m_bufferPos, buffered);
+        m_bufferPos += buffered;
+        read += buffered;
+    } while (read < maxlen && fillBuffer(maxlen) > 0);
+
+    Q_ASSERT(read > 0 || bufferedAvailable() == 0);
+    return (read == 0 && stdinAtEnd()) ? -1 : read;
 }
 
 qint64 PerfStdin::writeData(const char *data, qint64 len)
@@ -55,6 +84,55 @@ qint64 PerfStdin::writeData(const char *data, qint64 len)
     return -1;
 }
 
+void PerfStdin::receiveData()
+{
+    if (fillBuffer() > 0)
+        emit readyRead();
+    else if (stdinAtEnd())
+        close();
+}
+
+void PerfStdin::resizeBuffer(int newSize)
+{
+    QByteArray newBuffer(newSize, Qt::Uninitialized);
+    std::memcpy(newBuffer.data(), m_buffer.data() + m_bufferPos,
+                static_cast<size_t>(m_bufferUsed - m_bufferPos));
+    qSwap(m_buffer, newBuffer);
+    m_bufferUsed -= m_bufferPos;
+    Q_ASSERT(m_buffer.length() >= m_bufferUsed);
+    m_bufferPos = 0;
+}
+
+qint64 PerfStdin::fillBuffer(qint64 targetBufferSize)
+{
+    if (m_bufferUsed == m_bufferPos)
+        m_bufferPos = m_bufferUsed = 0;
+
+    targetBufferSize = qMin(targetBufferSize, static_cast<qint64>(s_maxBufferSize));
+    if (targetBufferSize > m_buffer.length())
+        resizeBuffer(static_cast<int>(targetBufferSize));
+
+    if (m_bufferUsed == m_buffer.length()) {
+        if (m_bufferPos == 0) {
+            resizeBuffer(m_bufferUsed <= s_maxBufferSize / 2 ? m_bufferUsed * 2
+                                                             : s_maxBufferSize);
+        } else {
+            resizeBuffer(m_bufferUsed);
+        }
+    }
+
+    const size_t read = fread(m_buffer.data() + m_bufferUsed, 1,
+                              static_cast<size_t>(m_buffer.length() - m_bufferUsed), stdin);
+    m_bufferUsed += read;
+    Q_ASSERT(m_buffer.length() >= m_bufferUsed);
+    return static_cast<qint64>(read);
+}
+
+bool PerfStdin::stdinAtEnd() const
+{
+    return feof(stdin) || ferror(stdin);
+}
+
 bool PerfStdin::isSequential() const
 {
     return true;
@@ -62,5 +140,5 @@ bool PerfStdin::isSequential() const
 
 qint64 PerfStdin::bytesAvailable() const
 {
-    return isOpen() ? std::numeric_limits<qint64>::max() : 0;
+    return bufferedAvailable() + QIODevice::bytesAvailable();
 }
diff --git a/app/perfstdin.h b/app/perfstdin.h
index 4b4d723..afab1a1 100644
--- a/app/perfstdin.h
+++ b/app/perfstdin.h
@@ -21,15 +21,35 @@
 #pragma once
 
 #include <QIODevice>
+#include <QTimer>
 
 class PerfStdin : public QIODevice
 {
 public:
+    PerfStdin(QObject *parent = nullptr);
+    ~PerfStdin();
+
     bool open(OpenMode mode) override;
+    void close() override;
     bool isSequential() const override;
     qint64 bytesAvailable() const override;
 
 protected:
     qint64 readData(char *data, qint64 maxlen) override;
     qint64 writeData(const char *data, qint64 len) override;
+
+private:
+    static const int s_minBufferSize = 1 << 10;
+    static const int s_maxBufferSize = 1 << 30;
+
+    void receiveData();
+    void resizeBuffer(int newSize);
+    qint64 fillBuffer(qint64 targetSize = -1);
+    qint64 bufferedAvailable() const { return m_bufferUsed - m_bufferPos; }
+    bool stdinAtEnd() const;
+
+    QTimer m_timer;
+    QByteArray m_buffer;
+    int m_bufferPos = 0;
+    int m_bufferUsed = 0;
 };
diff --git a/tests/auto/auto.pro b/tests/auto/auto.pro
index dea8bf5..f0ebef9 100644
--- a/tests/auto/auto.pro
+++ b/tests/auto/auto.pro
@@ -2,6 +2,7 @@ TEMPLATE = subdirs
 SUBDIRS = \
     elfmap \
     kallsyms \
-    perfdata
+    perfdata \
+    perfstdin
 
 OTHER_FILES += auto.qbs
diff --git a/tests/auto/auto.qbs b/tests/auto/auto.qbs
index 08ea2fe..e3d4026 100644
--- a/tests/auto/auto.qbs
+++ b/tests/auto/auto.qbs
@@ -4,6 +4,6 @@ Project {
     name: "PerfParserAutotests"
     condition: project.withAutotests
     references: [
-        "elfmap", "kallsyms", "perfdata"
+        "elfmap", "kallsyms", "perfdata", "perfstdin"
     ]
 }
diff --git a/tests/auto/perfstdin/perfstdin.pro b/tests/auto/perfstdin/perfstdin.pro
new file mode 100644
index 0000000..e4b0074
--- /dev/null
+++ b/tests/auto/perfstdin/perfstdin.pro
@@ -0,0 +1,17 @@
+QT += testlib
+QT -= gui
+
+CONFIG += testcase strict_flags warn_on
+
+INCLUDEPATH += ../../../app
+
+TARGET = tst_perfstdin
+
+SOURCES += \
+    tst_perfstdin.cpp \
+    ../../../app/perfstdin.cpp
+
+HEADERS += \
+    ../../../app/perfstdin.h
+
+OTHER_FILES += perfstdin.qbs
diff --git a/tests/auto/perfstdin/perfstdin.qbs b/tests/auto/perfstdin/perfstdin.qbs
new file mode 100644
index 0000000..a36665c
--- /dev/null
+++ b/tests/auto/perfstdin/perfstdin.qbs
@@ -0,0 +1,11 @@
+import qbs
+
+QtcAutotest {
+    name: "PerfStdin Autotest"
+    files: [
+        "tst_perfstdin.cpp",
+        "../../../app/perfstdin.cpp",
+        "../../../app/perfstdin.h"
+    ]
+    cpp.includePaths: base.concat(["../../../app"])
+}
diff --git a/tests/auto/perfstdin/tst_perfstdin.cpp b/tests/auto/perfstdin/tst_perfstdin.cpp
new file mode 100644
index 0000000..77f7323
--- /dev/null
+++ b/tests/auto/perfstdin/tst_perfstdin.cpp
@@ -0,0 +1,87 @@
+/****************************************************************************
+**
+** Copyright (C) 2018 The Qt Company Ltd
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the Qt Enterprise Perf Profiler Add-on.
+**
+** GNU General Public License Usage
+** This file may be used under the terms of the GNU General Public License
+** version 3 as published by the Free Software Foundation and appearing in
+** the file LICENSE.GPLv3 included in the packaging of this file. Please
+** review the following information to ensure the GNU General Public License
+** requirements will be met: https://www.gnu.org/licenses/gpl.html.
+**
+** If you have questions regarding the use of this file, please use
+** contact form at http://www.qt.io/contact-us
+**
+****************************************************************************/
+
+#include "perfstdin.h"
+
+#include <QString>
+#include <QtTest>
+#include <QTemporaryFile>
+
+#include <cstdio>
+
+class TestPerfstdin : public QObject
+{
+    Q_OBJECT
+
+private slots:
+    void testReadSelf();
+};
+
+void TestPerfstdin::testReadSelf()
+{
+    std::freopen(QCoreApplication::applicationFilePath().toUtf8().constData(), "rb", stdin);
+    QTemporaryFile tempfile;
+    QVERIFY(tempfile.open());
+    PerfStdin device;
+    int i = 0;
+
+    auto doRead = [&](){
+        QVERIFY(device.bytesAvailable() > 0);
+        qint64 r = device.bytesAvailable() + ((++i) % 32) - 16;
+        const QByteArray data = device.read(r);
+        qint64 pos = 0;
+        while (pos < data.length()) {
+            qint64 written = tempfile.write(data.data() + pos, data.length() - pos);
+            QVERIFY(written >= 0);
+            pos += written;
+        }
+        QCOMPARE(pos, data.length());
+    };
+
+    QObject::connect(&device, &QIODevice::readyRead, &tempfile, doRead);
+
+    QObject::connect(&device, &QIODevice::aboutToClose, &tempfile, [&](){
+        while (device.bytesAvailable() > 0)
+            doRead();
+        while (tempfile.bytesToWrite() > 0)
+            QVERIFY(tempfile.flush());
+
+        QFile self(QCoreApplication::applicationFilePath());
+        QCOMPARE(tempfile.pos(), self.size());
+
+        QVERIFY(tempfile.reset());
+        QVERIFY(self.open(QIODevice::ReadOnly));
+        char c1, c2;
+        while (!self.atEnd()) {
+            QVERIFY(self.getChar(&c1));
+            QVERIFY(tempfile.getChar(&c2));
+            QCOMPARE(c1, c2);
+        }
+        QVERIFY(self.atEnd());
+        QVERIFY(tempfile.atEnd());
+        tempfile.close();
+    });
+
+    QVERIFY(device.open(QIODevice::ReadOnly));
+    QTRY_VERIFY(!tempfile.isOpen());
+}
+
+QTEST_MAIN(TestPerfstdin)
+
+#include "tst_perfstdin.moc"