From aa52cfa0e6745d517b11c85a013a35f9712257f5 Mon Sep 17 00:00:00 2001
From: Christian Kandeler <christian.kandeler@digia.com>
Date: Fri, 25 Apr 2014 12:19:48 +0200
Subject: [PATCH] Valgrind: Remove dialog asking for IP address.

When running the memcheck tool remotely, it sends its data via a TCP
socket to the development host, so it needs to know that machine's IP
address. The current code gathers all local network addresses and makes
the user choose one of them. However, we can get that information from
the SSH connection, so no user interaction is required.

Change-Id: Ia61decddd5fa1e285ca143605d944d6d9275b3e4
Reviewed-by: hjk <hjk121@nokiamail.com>
---
 .../callgrind/callgrindcontroller.cpp         |   8 +-
 .../valgrind/memcheck/memcheckrunner.cpp      | 136 ++++++------------
 .../valgrind/memcheck/memcheckrunner.h        |   5 +
 src/plugins/valgrind/valgrindprocess.cpp      |  46 ++++--
 src/plugins/valgrind/valgrindprocess.h        |  17 ++-
 src/plugins/valgrind/valgrindrunner.cpp       |  31 ++--
 src/plugins/valgrind/valgrindrunner.h         |   2 +
 7 files changed, 131 insertions(+), 114 deletions(-)

diff --git a/src/plugins/valgrind/callgrind/callgrindcontroller.cpp b/src/plugins/valgrind/callgrind/callgrindcontroller.cpp
index 5a4e4675d41..bd7633bfe31 100644
--- a/src/plugins/valgrind/callgrind/callgrindcontroller.cpp
+++ b/src/plugins/valgrind/callgrind/callgrindcontroller.cpp
@@ -135,9 +135,11 @@ void CallgrindController::run(Option option)
     m_process->setProcessChannelMode(QProcess::ForwardedChannels);
 #endif
     const int pid = Utils::HostOsInfo::isWindowsHost() ? 0 : m_valgrindProc->pid();
-    m_process->run(CALLGRIND_CONTROL_BINARY,
-                   QStringList() << optionString << QString::number(pid),
-                   QString(), QString());
+    m_process->setValgrindExecutable(CALLGRIND_CONTROL_BINARY);
+    m_process->setValgrindArguments(QStringList() << optionString << QString::number(pid));
+    m_process->setDebuggeeExecutable(QString());
+    m_process->setDebugeeArguments(QString());
+    m_process->run();
 }
 
 void CallgrindController::processError(QProcess::ProcessError)
diff --git a/src/plugins/valgrind/memcheck/memcheckrunner.cpp b/src/plugins/valgrind/memcheck/memcheckrunner.cpp
index e0ab359ce94..0f88f984f11 100644
--- a/src/plugins/valgrind/memcheck/memcheckrunner.cpp
+++ b/src/plugins/valgrind/memcheck/memcheckrunner.cpp
@@ -33,6 +33,7 @@
 #include "../xmlprotocol/error.h"
 #include "../xmlprotocol/status.h"
 #include "../xmlprotocol/threadedparser.h"
+#include "../valgrindprocess.h"
 
 #include <utils/qtcassert.h>
 
@@ -96,98 +97,11 @@ bool MemcheckRunner::start()
 {
     QTC_ASSERT(d->parser, return false);
 
-    QString ip;
-    QHostAddress hostAddr;
-
+    // The remote case is handled in localHostAddressRetrieved().
     if (startMode() == Analyzer::StartLocal) {
-        ip = QLatin1String("127.0.0.1");
-        hostAddr = QHostAddress(QHostAddress::LocalHost);
-    }
-
-    if (startMode() == Analyzer::StartRemote) {
-
-        QList<QHostAddress> possibleHostAddresses;
-        //NOTE: ::allAddresses does not seem to work for usb interfaces...
-        foreach (const QNetworkInterface &iface, QNetworkInterface::allInterfaces()) {
-            foreach (const QNetworkAddressEntry &entry, iface.addressEntries()) {
-                const QHostAddress addr = entry.ip();
-                if (addr.toString() != QLatin1String("127.0.0.1")
-                        && addr.toString() != QLatin1String("0:0:0:0:0:0:0:1"))
-                {
-                    possibleHostAddresses << addr;
-                    break;
-                }
-            }
-        }
-
-        if (possibleHostAddresses.isEmpty()) {
-            emit processErrorReceived(tr("No network interface found for remote analysis."),
-                                      QProcess::FailedToStart);
-            return false;
-        } else if (possibleHostAddresses.size() > 1) {
-            QDialog dlg;
-            dlg.setWindowTitle(tr("Select Network Interface"));
-            QVBoxLayout *layout = new QVBoxLayout;
-            QLabel *description = new QLabel;
-            description->setWordWrap(true);
-            description->setText(tr("More than one network interface was found on your machine. Please select the one you want to use for remote analysis."));
-            layout->addWidget(description);
-            QListWidget *list = new QListWidget;
-            foreach (const QHostAddress &address, possibleHostAddresses)
-                list->addItem(address.toString());
-
-            list->setSelectionMode(QAbstractItemView::SingleSelection);
-            list->setCurrentRow(0);
-            layout->addWidget(list);
-
-            QDialogButtonBox *buttons = new QDialogButtonBox;
-            buttons->addButton(QDialogButtonBox::Ok);
-            buttons->addButton(QDialogButtonBox::Cancel);
-            connect(buttons, SIGNAL(accepted()),
-                    &dlg, SLOT(accept()));
-            connect(buttons, SIGNAL(rejected()),
-                    &dlg, SLOT(reject()));
-            layout->addWidget(buttons);
-
-            dlg.setLayout(layout);
-            if (dlg.exec() != QDialog::Accepted) {
-                emit processErrorReceived(tr("No network interface was chosen for remote analysis."), QProcess::FailedToStart);
-                return false;
-            }
-
-            QTC_ASSERT(list->currentRow() >= 0, return false);
-            QTC_ASSERT(list->currentRow() < possibleHostAddresses.size(), return false);
-            hostAddr = possibleHostAddresses.at(list->currentRow());
-        } else {
-            hostAddr = possibleHostAddresses.first();
-        }
-
-        ip = hostAddr.toString();
-        QTC_ASSERT(!ip.isEmpty(), return false);
-        hostAddr = QHostAddress(ip);
+        startServers(QHostAddress(QHostAddress::LocalHost));
+        setValgrindArguments(memcheckLogArguments() + valgrindArguments());
     }
-
-    bool check = d->xmlServer.listen(hostAddr);
-    if (!check) emit processErrorReceived( tr("XmlServer on %1:").arg(ip) + QLatin1Char(' ') + d->xmlServer.errorString(), QProcess::FailedToStart );
-    QTC_ASSERT(check, return false);
-    d->xmlServer.setMaxPendingConnections(1);
-    const quint16 xmlPortNumber = d->xmlServer.serverPort();
-    connect(&d->xmlServer, SIGNAL(newConnection()), SLOT(xmlSocketConnected()));
-
-    check = d->logServer.listen(hostAddr);
-    if (!check) emit processErrorReceived( tr("LogServer on %1:").arg(ip) + QLatin1Char(' ') + d->logServer.errorString(), QProcess::FailedToStart );
-    QTC_ASSERT(check, return false);
-    d->logServer.setMaxPendingConnections(1);
-    const quint16 logPortNumber = d->logServer.serverPort();
-    connect(&d->logServer, SIGNAL(newConnection()), SLOT(logSocketConnected()));
-
-    QStringList memcheckLogArguments;
-    memcheckLogArguments << QLatin1String("--xml=yes")
-                      << QString::fromLatin1("--xml-socket=%1:%2").arg(ip).arg(xmlPortNumber)
-                      << QLatin1String("--child-silent-after-fork=yes")
-                      << QString::fromLatin1("--log-socket=%1:%2").arg(ip).arg(logPortNumber);
-    setValgrindArguments(memcheckLogArguments + valgrindArguments());
-
     return ValgrindRunner::start();
 }
 
@@ -214,5 +128,47 @@ void MemcheckRunner::readLogSocket()
     emit logMessageReceived(d->logSocket->readAll());
 }
 
+// Note: The callers of this function cannot handle errors, so they will ignore the return value.
+// We still provide it in case the surrounding infrastructure will improve.
+bool MemcheckRunner::startServers(const QHostAddress &localHostAddress)
+{
+    bool check = d->xmlServer.listen(localHostAddress);
+    const QString ip = localHostAddress.toString();
+    if (!check) {
+        emit processErrorReceived( tr("XmlServer on %1:").arg(ip) + QLatin1Char(' ')
+                                   + d->xmlServer.errorString(), QProcess::FailedToStart );
+        return false;
+    }
+    d->xmlServer.setMaxPendingConnections(1);
+    connect(&d->xmlServer, SIGNAL(newConnection()), SLOT(xmlSocketConnected()));
+    check = d->logServer.listen(localHostAddress);
+    if (!check) {
+        emit processErrorReceived( tr("LogServer on %1:").arg(ip) + QLatin1Char(' ')
+                                   + d->logServer.errorString(), QProcess::FailedToStart );
+        return false;
+    }
+    d->logServer.setMaxPendingConnections(1);
+    connect(&d->logServer, SIGNAL(newConnection()), SLOT(logSocketConnected()));
+    return true;
+}
+
+QStringList MemcheckRunner::memcheckLogArguments() const
+{
+    return QStringList()
+            << QLatin1String("--xml=yes")
+            << QString::fromLatin1("--xml-socket=%1:%2")
+               .arg(d->xmlServer.serverAddress().toString()).arg(d->xmlServer.serverPort())
+            << QLatin1String("--child-silent-after-fork=yes")
+            << QString::fromLatin1("--log-socket=%1:%2")
+               .arg(d->logServer.serverAddress().toString()).arg(d->logServer.serverPort());
+}
+
+void MemcheckRunner::localHostAddressRetrieved(const QHostAddress &localHostAddress)
+{
+    startServers(localHostAddress);
+    setValgrindArguments(memcheckLogArguments() + valgrindArguments());
+    valgrindProcess()->setValgrindArguments(fullValgrindArguments());
+}
+
 } // namespace Memcheck
 } // namespace Valgrind
diff --git a/src/plugins/valgrind/memcheck/memcheckrunner.h b/src/plugins/valgrind/memcheck/memcheckrunner.h
index cbf4998af48..f57bb9d099f 100644
--- a/src/plugins/valgrind/memcheck/memcheckrunner.h
+++ b/src/plugins/valgrind/memcheck/memcheckrunner.h
@@ -56,6 +56,8 @@ signals:
     void logMessageReceived(const QByteArray &);
 
 private slots:
+    void localHostAddressRetrieved(const QHostAddress &localHostAddress);
+
     void xmlSocketConnected();
     void logSocketConnected();
     void readLogSocket();
@@ -63,6 +65,9 @@ private slots:
 private:
     QString tool() const;
 
+    bool startServers(const QHostAddress &localHostAddress);
+    QStringList memcheckLogArguments() const;
+
     class Private;
     Private *d;
 };
diff --git a/src/plugins/valgrind/valgrindprocess.cpp b/src/plugins/valgrind/valgrindprocess.cpp
index 0a9f9d3281d..7231353abb1 100644
--- a/src/plugins/valgrind/valgrindprocess.cpp
+++ b/src/plugins/valgrind/valgrindprocess.cpp
@@ -79,6 +79,26 @@ bool ValgrindProcess::isRunning() const
         return m_remote.m_process && m_remote.m_process->isRunning();
 }
 
+void ValgrindProcess::setValgrindExecutable(const QString &valgrindExecutable)
+{
+    m_valgrindExecutable = valgrindExecutable;
+}
+
+void ValgrindProcess::setValgrindArguments(const QStringList &valgrindArguments)
+{
+    m_valgrindArguments = valgrindArguments;
+}
+
+void ValgrindProcess::setDebuggeeExecutable(const QString &debuggeeExecutable)
+{
+    m_debuggeeExecutable = debuggeeExecutable;
+}
+
+void ValgrindProcess::setDebugeeArguments(const QString &debuggeeArguments)
+{
+    m_debuggeeArguments = debuggeeArguments;
+}
+
 void ValgrindProcess::setEnvironment(const Utils::Environment &environment)
 {
     if (isLocal())
@@ -108,13 +128,8 @@ void ValgrindProcess::close()
     }
 }
 
-void ValgrindProcess::run(const QString &valgrindExecutable, const QStringList &valgrindArguments,
-                                const QString &debuggeeExecutable, const QString &debuggeeArguments)
+void ValgrindProcess::run()
 {
-    Utils::QtcProcess::addArgs(&m_arguments, valgrindArguments);
-    Utils::QtcProcess::addArg(&m_arguments, debuggeeExecutable);
-    Utils::QtcProcess::addArgs(&m_arguments, debuggeeArguments);
-
     if (isLocal()) {
         connect(&m_localProcess, SIGNAL(finished(int,QProcess::ExitStatus)),
                 this, SIGNAL(finished(int,QProcess::ExitStatus)));
@@ -127,13 +142,14 @@ void ValgrindProcess::run(const QString &valgrindExecutable, const QStringList &
         connect(&m_localProcess, SIGNAL(readyReadStandardOutput()),
                 this, SLOT(handleReadyReadStandardOutput()));
 
-        m_localProcess.setCommand(valgrindExecutable, m_arguments);
+        m_localProcess.setCommand(m_valgrindExecutable,
+                                  argumentString(Utils::HostOsInfo::hostOs()));
         m_localProcess.start();
         m_localProcess.waitForStarted();
         m_pid = Utils::qPidToPid(m_localProcess.pid());
     } else {
-        m_remote.m_valgrindExe = valgrindExecutable;
-        m_remote.m_debuggee = debuggeeExecutable;
+        m_remote.m_valgrindExe = m_valgrindExecutable;
+        m_remote.m_debuggee = m_debuggeeExecutable;
 
         // connect to host and wait for connection
         if (!m_remote.m_connection)
@@ -215,13 +231,15 @@ void ValgrindProcess::connected()
 {
     QTC_ASSERT(m_remote.m_connection->state() == QSsh::SshConnection::Connected, return);
 
+    emit localHostAddressRetrieved(m_remote.m_connection->connectionInfo().localAddress);
+
     // connected, run command
     QString cmd;
 
     if (!m_remote.m_workingDir.isEmpty())
         cmd += QString::fromLatin1("cd '%1' && ").arg(m_remote.m_workingDir);
 
-    cmd += m_remote.m_valgrindExe + QLatin1Char(' ') + m_arguments;
+    cmd += m_remote.m_valgrindExe + QLatin1Char(' ') + argumentString(Utils::OsTypeLinux);
 
     m_remote.m_process = m_remote.m_connection->createRemoteProcess(cmd.toUtf8());
     connect(m_remote.m_process.data(), SIGNAL(readyReadStandardError()),
@@ -287,6 +305,14 @@ void ValgrindProcess::findPIDOutputReceived()
     }
 }
 
+QString ValgrindProcess::argumentString(Utils::OsType osType) const
+{
+    QString arguments = Utils::QtcProcess::joinArgs(m_valgrindArguments, osType);
+    Utils::QtcProcess::addArg(&arguments, m_debuggeeExecutable, osType);
+    Utils::QtcProcess::addArg(&arguments, m_debuggeeArguments, osType);
+    return arguments;
+}
+
 
 ///////////
 
diff --git a/src/plugins/valgrind/valgrindprocess.h b/src/plugins/valgrind/valgrindprocess.h
index f710a33453e..47cd9f81de4 100644
--- a/src/plugins/valgrind/valgrindprocess.h
+++ b/src/plugins/valgrind/valgrindprocess.h
@@ -34,6 +34,7 @@
 #include <utils/qtcprocess.h>
 #include <ssh/sshremoteprocess.h>
 #include <ssh/sshconnection.h>
+#include <utils/osspecificaspects.h>
 #include <utils/outputformat.h>
 
 namespace Valgrind {
@@ -63,8 +64,12 @@ public:
 
     bool isRunning() const;
 
-    void run(const QString &valgrindExecutable, const QStringList &valgrindArguments,
-             const QString &debuggeeExecutable, const QString &debuggeeArguments);
+    void setValgrindExecutable(const QString &valgrindExecutable);
+    void setValgrindArguments(const QStringList &valgrindArguments);
+    void setDebuggeeExecutable(const QString &debuggeeExecutable);
+    void setDebugeeArguments(const QString &debuggeeArguments);
+
+    void run();
     void close();
 
     QString errorString() const;
@@ -84,6 +89,7 @@ signals:
     void finished(int, QProcess::ExitStatus);
     void error(QProcess::ProcessError);
     void processOutput(const QString &, Utils::OutputFormat format);
+    void localHostAddressRetrieved(const QHostAddress &localHostAddress);
 
 private slots:
     void handleReadyReadStandardError();
@@ -96,11 +102,16 @@ private slots:
     void findPIDOutputReceived();
 
 private:
+    QString argumentString(Utils::OsType osType) const;
+
     Utils::QtcProcess m_localProcess;
     qint64 m_pid;
 
     Remote m_remote;
-    QString m_arguments;
+    QString m_valgrindExecutable;
+    QStringList m_valgrindArguments;
+    QString m_debuggeeExecutable;
+    QString m_debuggeeArguments;
     bool m_isLocal;
 };
 
diff --git a/src/plugins/valgrind/valgrindrunner.cpp b/src/plugins/valgrind/valgrindrunner.cpp
index cc671ac39bc..655ac4105a8 100644
--- a/src/plugins/valgrind/valgrindrunner.cpp
+++ b/src/plugins/valgrind/valgrindrunner.cpp
@@ -89,12 +89,6 @@ void ValgrindRunner::Private::run(ValgrindProcess *_process)
     process->setProcessChannelMode(channelMode);
     // consider appending our options last so they override any interfering user-supplied options
     // -q as suggested by valgrind manual
-    QStringList valgrindArgs = valgrindArguments;
-    valgrindArgs << QString::fromLatin1("--tool=%1").arg(q->tool());
-
-    if (Utils::HostOsInfo::isMacHost())
-        // May be slower to start but without it we get no filenames for symbols.
-        valgrindArgs << QLatin1String("--dsymutil=yes");
 
     QObject::connect(process, SIGNAL(processOutput(QString,Utils::OutputFormat)),
             q, SIGNAL(processOutputReceived(QString,Utils::OutputFormat)));
@@ -104,8 +98,14 @@ void ValgrindRunner::Private::run(ValgrindProcess *_process)
             q, SLOT(processFinished(int,QProcess::ExitStatus)));
     QObject::connect(process, SIGNAL(error(QProcess::ProcessError)),
             q, SLOT(processError(QProcess::ProcessError)));
-
-    process->run(valgrindExecutable, valgrindArgs, debuggeeExecutable, debuggeeArguments);
+    QObject::connect(process, SIGNAL(localHostAddressRetrieved(QHostAddress)), q,
+                     SLOT(localHostAddressRetrieved(QHostAddress)));
+
+    process->setValgrindExecutable(valgrindExecutable);
+    process->setValgrindArguments(q->fullValgrindArguments());
+    process->setDebuggeeExecutable(debuggeeExecutable);
+    process->setDebugeeArguments(debuggeeArguments);
+    process->run();
 }
 
 ValgrindRunner::ValgrindRunner(QObject *parent)
@@ -144,6 +144,16 @@ QStringList ValgrindRunner::valgrindArguments() const
     return d->valgrindArguments;
 }
 
+QStringList ValgrindRunner::fullValgrindArguments() const
+{
+    QStringList fullArgs = valgrindArguments();
+    fullArgs << QString::fromLatin1("--tool=%1").arg(tool());
+    if (Utils::HostOsInfo::isMacHost())
+        // May be slower to start but without it we get no filenames for symbols.
+        fullArgs << QLatin1String("--dsymutil=yes");
+    return fullArgs;
+}
+
 QString ValgrindRunner::debuggeeExecutable() const
 {
     return d->debuggeeExecutable;
@@ -246,6 +256,11 @@ void ValgrindRunner::processFinished(int ret, QProcess::ExitStatus status)
         emit processErrorReceived(errorString(), d->process->error());
 }
 
+void ValgrindRunner::localHostAddressRetrieved(const QHostAddress &localHostAddress)
+{
+    Q_UNUSED(localHostAddress);
+}
+
 void ValgrindRunner::processStarted()
 {
     emit started();
diff --git a/src/plugins/valgrind/valgrindrunner.h b/src/plugins/valgrind/valgrindrunner.h
index 30675454f52..02f23b77fbc 100644
--- a/src/plugins/valgrind/valgrindrunner.h
+++ b/src/plugins/valgrind/valgrindrunner.h
@@ -58,6 +58,7 @@ public:
     QString valgrindExecutable() const;
     void setValgrindExecutable(const QString &executable);
     QStringList valgrindArguments() const;
+    QStringList fullValgrindArguments() const;
     void setValgrindArguments(const QStringList &toolArguments);
     QString debuggeeExecutable() const;
     void setDebuggeeExecutable(const QString &executable);
@@ -97,6 +98,7 @@ protected slots:
     virtual void processError(QProcess::ProcessError);
     virtual void processStarted();
     virtual void processFinished(int, QProcess::ExitStatus);
+    virtual void localHostAddressRetrieved(const QHostAddress &localHostAddress);
 
 private:
     class Private;
-- 
GitLab