Valgrind: Simplify internals

Change-Id: Ice10c1bf96f3102c6525d37ee7ec25b10604ebb9
Reviewed-by: hjk <hjk@qt.io>

src/plugins/valgrind/callgrindengine.cpp

@@ -57,17 +57,15 @@ CallgrindToolRunner::CallgrindToolRunner(RunControl *runControl)
 {
     setId("CallgrindToolRunner");
 
-    connect(&m_runner, &ValgrindRunner::finished,
-            this, &CallgrindToolRunner::slotFinished);
-    connect(&m_parser, &Callgrind::Parser::parserDataReady,
-            this, &CallgrindToolRunner::slotFinished);
-
     connect(&m_runner, &ValgrindRunner::valgrindStarted, this, [this](qint64 pid) {
         m_pid = pid;
     });
-
-    connect(&m_runner, &ValgrindRunner::extraProcessFinished, this, [this] {
+    connect(&m_runner, &ValgrindRunner::finished, this, [this] {
         triggerParse();
+        emit parserDataReady(this);
+    });
+    connect(&m_parser, &Callgrind::Parser::parserDataReady, this, [this] {
+        emit parserDataReady(this);
     });
 
     m_valgrindRunnable = runControl->runnable();
@@ -152,11 +150,6 @@ Callgrind::ParseData *CallgrindToolRunner::takeParserData()
     return m_parser.takeData();
 }
 
-void CallgrindToolRunner::slotFinished()
-{
-    emit parserDataReady(this);
-}
-
 void CallgrindToolRunner::showStatusMessage(const QString &message)
 {
     Debugger::showPermanentStatusMessage(message);

src/plugins/valgrind/callgrindengine.h

@@ -77,7 +77,6 @@ signals:
     void parserDataReady(CallgrindToolRunner *engine);
 
 private:
-    void slotFinished();
     void showStatusMessage(const QString &message);
 
     /**

src/plugins/valgrind/valgrindrunner.cpp

@@ -44,44 +44,98 @@ namespace Valgrind {
 class ValgrindRunner::Private : public QObject
 {
 public:
-    Private(ValgrindRunner *owner) : q(owner) {}
+    Private(ValgrindRunner *owner) : q(owner) {
+        connect(&m_process, &QtcProcess::started, this, [this] {
+            emit q->valgrindStarted(m_process.processId());
+        });
+        connect(&m_process, &QtcProcess::done, this, [this] {
+            if (m_process.result() != ProcessResult::FinishedWithSuccess)
+                emit q->processErrorReceived(m_process.errorString(), m_process.error());
+            emit q->finished();
+        });
+        connect(&m_process, &QtcProcess::readyReadStandardOutput, this, [this] {
+            emit q->appendMessage(QString::fromUtf8(m_process.readAllStandardOutput()),
+                                  StdOutFormat);
+        });
+        connect(&m_process, &QtcProcess::readyReadStandardError, this, [this] {
+            emit q->appendMessage(QString::fromUtf8(m_process.readAllStandardError()),
+                                  StdErrFormat);
+        });
 
+        connect(&m_xmlServer, &QTcpServer::newConnection, this, &Private::xmlSocketConnected);
+        connect(&m_logServer, &QTcpServer::newConnection, this, &Private::logSocketConnected);
+    }
+
+    void xmlSocketConnected();
+    void logSocketConnected();
+    bool startServers();
     bool run();
 
-    void processStarted();
-    void processDone();
-
     ValgrindRunner *q;
     Runnable m_debuggee;
-    QtcProcess m_valgrindProcess;
 
-    CommandLine m_valgrindCommand;
+    CommandLine m_command;
+    QtcProcess m_process;
 
-    QHostAddress localServerAddress;
-    QProcess::ProcessChannelMode channelMode = QProcess::SeparateChannels;
+    QHostAddress m_localServerAddress;
 
-    QTcpServer xmlServer;
-    XmlProtocol::ThreadedParser parser;
-    QTcpServer logServer;
-    QTcpSocket *logSocket = nullptr;
-
-    // Workaround for valgrind bug when running vgdb with xml output
-    // https://bugs.kde.org/show_bug.cgi?id=343902
-    bool disableXml = false;
+    QTcpServer m_xmlServer;
+    XmlProtocol::ThreadedParser m_parser;
+    QTcpServer m_logServer;
 };
 
+void ValgrindRunner::Private::xmlSocketConnected()
+{
+    QTcpSocket *socket = m_xmlServer.nextPendingConnection();
+    QTC_ASSERT(socket, return);
+    m_xmlServer.close();
+    m_parser.parse(socket);
+}
+
+void ValgrindRunner::Private::logSocketConnected()
+{
+    QTcpSocket *logSocket = m_logServer.nextPendingConnection();
+    QTC_ASSERT(logSocket, return);
+    connect(logSocket, &QIODevice::readyRead, this, [this, logSocket] {
+        emit q->logMessageReceived(logSocket->readAll());
+    });
+    m_logServer.close();
+}
+
+bool ValgrindRunner::Private::startServers()
+{
+    const bool xmlOK = m_xmlServer.listen(m_localServerAddress);
+    const QString ip = m_localServerAddress.toString();
+    if (!xmlOK) {
+        emit q->processErrorReceived(tr("XmlServer on %1:").arg(ip) + ' '
+                                     + m_xmlServer.errorString(), QProcess::FailedToStart );
+        return false;
+    }
+    m_xmlServer.setMaxPendingConnections(1);
+    const bool logOK = m_logServer.listen(m_localServerAddress);
+    if (!logOK) {
+        emit q->processErrorReceived(tr("LogServer on %1:").arg(ip) + ' '
+                                     + m_logServer.errorString(), QProcess::FailedToStart );
+        return false;
+    }
+    m_logServer.setMaxPendingConnections(1);
+    return true;
+}
+
 bool ValgrindRunner::Private::run()
 {
     CommandLine cmd;
-    cmd.setExecutable(m_valgrindCommand.executable());
+    cmd.setExecutable(m_command.executable());
 
-    if (!localServerAddress.isNull()) {
-        if (!q->startServers())
+    if (!m_localServerAddress.isNull()) {
+        if (!startServers())
             return false;
 
         cmd.addArg("--child-silent-after-fork=yes");
 
-        bool enableXml = !disableXml;
+        // Workaround for valgrind bug when running vgdb with xml output
+        // https://bugs.kde.org/show_bug.cgi?id=343902
+        bool enableXml = true;
 
         auto handleSocketParameter = [&enableXml, &cmd](const QString &prefix, const QTcpServer &tcpServer)
         {
@@ -97,32 +151,17 @@ bool ValgrindRunner::Private::run()
             }
         };
 
-        handleSocketParameter("--xml-socket", xmlServer);
-        handleSocketParameter("--log-socket", logServer);
+        handleSocketParameter("--xml-socket", m_xmlServer);
+        handleSocketParameter("--log-socket", m_logServer);
 
         if (enableXml)
             cmd.addArg("--xml=yes");
     }
-    cmd.addArgs(m_valgrindCommand.arguments(), CommandLine::Raw);
+    cmd.addArgs(m_command.arguments(), CommandLine::Raw);
 
-    m_valgrindProcess.setProcessChannelMode(channelMode);
     // consider appending our options last so they override any interfering user-supplied options
     // -q as suggested by valgrind manual
 
-    connect(&m_valgrindProcess, &QtcProcess::started,
-            this, &ValgrindRunner::Private::processStarted);
-    connect(&m_valgrindProcess, &QtcProcess::done,
-            this, &ValgrindRunner::Private::processDone);
-
-    connect(&m_valgrindProcess, &QtcProcess::readyReadStandardOutput, q, [this] {
-        emit q->appendMessage(QString::fromUtf8(m_valgrindProcess.readAllStandardOutput()),
-                              StdOutFormat);
-    });
-    connect(&m_valgrindProcess, &QtcProcess::readyReadStandardError, q, [this] {
-        emit q->appendMessage(QString::fromUtf8(m_valgrindProcess.readAllStandardError()),
-                              StdErrFormat);
-    });
-
     if (cmd.executable().osType() == OsTypeMac) {
         // May be slower to start but without it we get no filenames for symbols.
         cmd.addArg("--dsymutil=yes");
@@ -132,30 +171,13 @@ bool ValgrindRunner::Private::run()
 
     emit q->valgrindExecuted(cmd.toUserOutput());
 
-    m_valgrindProcess.setCommand(cmd);
-    m_valgrindProcess.setWorkingDirectory(m_debuggee.workingDirectory);
-    m_valgrindProcess.setEnvironment(m_debuggee.environment);
-    m_valgrindProcess.start();
+    m_process.setCommand(cmd);
+    m_process.setWorkingDirectory(m_debuggee.workingDirectory);
+    m_process.setEnvironment(m_debuggee.environment);
+    m_process.start();
     return true;
 }
 
-void ValgrindRunner::Private::processStarted()
-{
-    const qint64 pid = m_valgrindProcess.processId();
-    emit q->valgrindStarted(pid);
-}
-
-void ValgrindRunner::Private::processDone()
-{
-    emit q->extraProcessFinished();
-
-    if (m_valgrindProcess.result() != ProcessResult::FinishedWithSuccess)
-        emit q->processErrorReceived(m_valgrindProcess.errorString(), m_valgrindProcess.error());
-
-    // make sure we don't wait for the connection anymore
-    emit q->finished();
-}
-
 ValgrindRunner::ValgrindRunner(QObject *parent)
     : QObject(parent), d(new Private(this))
 {
@@ -163,11 +185,11 @@ ValgrindRunner::ValgrindRunner(QObject *parent)
 
 ValgrindRunner::~ValgrindRunner()
 {
-    if (d->m_valgrindProcess.isRunning()) {
+    if (d->m_process.isRunning()) {
         // make sure we don't delete the thread while it's still running
         waitForFinished();
     }
-    if (d->parser.isRunning()) {
+    if (d->m_parser.isRunning()) {
         // make sure we don't delete the thread while it's still running
         waitForFinished();
     }
@@ -177,7 +199,7 @@ ValgrindRunner::~ValgrindRunner()
 
 void ValgrindRunner::setValgrindCommand(const CommandLine &command)
 {
-    d->m_valgrindCommand = command;
+    d->m_command = command;
 }
 
 void ValgrindRunner::setDebuggee(const Runnable &debuggee)
@@ -187,22 +209,22 @@ void ValgrindRunner::setDebuggee(const Runnable &debuggee)
 
 void ValgrindRunner::setProcessChannelMode(QProcess::ProcessChannelMode mode)
 {
-    d->channelMode = mode;
+    d->m_process.setProcessChannelMode(mode);
 }
 
 void ValgrindRunner::setLocalServerAddress(const QHostAddress &localServerAddress)
 {
-    d->localServerAddress = localServerAddress;
+    d->m_localServerAddress = localServerAddress;
 }
 
 void ValgrindRunner::setUseTerminal(bool on)
 {
-    d->m_valgrindProcess.setTerminalMode(on ? TerminalMode::On : TerminalMode::Off);
+    d->m_process.setTerminalMode(on ? TerminalMode::On : TerminalMode::Off);
 }
 
 void ValgrindRunner::waitForFinished() const
 {
-    if (d->m_valgrindProcess.state() == QProcess::NotRunning)
+    if (d->m_process.state() == QProcess::NotRunning)
         return;
 
     QEventLoop loop;
@@ -212,7 +234,7 @@ void ValgrindRunner::waitForFinished() const
 
 QString ValgrindRunner::errorString() const
 {
-    return d->m_valgrindProcess.errorString();
+    return d->m_process.errorString();
 }
 
 bool ValgrindRunner::start()
@@ -222,59 +244,12 @@ bool ValgrindRunner::start()
 
 void ValgrindRunner::stop()
 {
-    d->m_valgrindProcess.stop();
+    d->m_process.stop();
 }
 
 XmlProtocol::ThreadedParser *ValgrindRunner::parser() const
 {
-    return &d->parser;
-}
-
-void ValgrindRunner::xmlSocketConnected()
-{
-    QTcpSocket *socket = d->xmlServer.nextPendingConnection();
-    QTC_ASSERT(socket, return);
-    d->xmlServer.close();
-    d->parser.parse(socket);
-}
-
-void ValgrindRunner::logSocketConnected()
-{
-    d->logSocket = d->logServer.nextPendingConnection();
-    QTC_ASSERT(d->logSocket, return);
-    connect(d->logSocket, &QIODevice::readyRead,
-            this, &ValgrindRunner::readLogSocket);
-    d->logServer.close();
-}
-
-void ValgrindRunner::readLogSocket()
-{
-    QTC_ASSERT(d->logSocket, return);
-    emit logMessageReceived(d->logSocket->readAll());
-}
-
-bool ValgrindRunner::startServers()
-{
-    bool check = d->xmlServer.listen(d->localServerAddress);
-    const QString ip = d->localServerAddress.toString();
-    if (!check) {
-        emit processErrorReceived(tr("XmlServer on %1:").arg(ip) + ' '
-                                  + d->xmlServer.errorString(), QProcess::FailedToStart );
-        return false;
-    }
-    d->xmlServer.setMaxPendingConnections(1);
-    connect(&d->xmlServer, &QTcpServer::newConnection,
-            this, &ValgrindRunner::xmlSocketConnected);
-    check = d->logServer.listen(d->localServerAddress);
-    if (!check) {
-        emit processErrorReceived(tr("LogServer on %1:").arg(ip) + ' '
-                                  + d->logServer.errorString(), QProcess::FailedToStart );
-        return false;
-    }
-    d->logServer.setMaxPendingConnections(1);
-    connect(&d->logServer, &QTcpServer::newConnection,
-            this, &ValgrindRunner::logSocketConnected);
-    return true;
+    return &d->m_parser;
 }
 
 } // namespace Valgrind

src/plugins/valgrind/valgrindrunner.h

@@ -71,15 +71,8 @@ signals:
     void valgrindExecuted(const QString &);
     void valgrindStarted(qint64 pid);
     void finished();
-    void extraProcessFinished();
 
 private:
-    bool startServers();
-
-    void xmlSocketConnected();
-    void logSocketConnected();
-    void readLogSocket();
-
     class Private;
     Private *d;
 };