Valgrind: Simplify device setup and handling

Make isLocal() less intrusively used and correct.  Use the stored device
more often, also handle errors more quickly.

Change-Id: I146d1f5788ea79d0a9d7b058c81908d451cf00d0
Reviewed-by: Christian Kandeler <christian.kandeler@theqtcompany.com>

src/plugins/valgrind/callgrind/callgrindcontroller.cpp

@@ -93,10 +93,7 @@ void CallgrindController::run(Option option)
     }
     QTC_ASSERT(m_valgrindProc, return);
 
-    QSsh::SshConnection *connection = m_valgrindProc->connection();
+    m_process = new ValgrindProcess(m_valgrindProc->device(), this);
-    m_process = new ValgrindProcess(
-                connection ? connection->connectionParameters() : QSsh::SshConnectionParameters(),
-                connection, this);
 
     connect(m_process, &ValgrindProcess::finished,
             this, &CallgrindController::processFinished);

src/plugins/valgrind/callgrindtool.h

@@ -48,7 +48,7 @@ public:
     CallgrindTool(QObject *parent);
     ~CallgrindTool();
 
-    ValgrindRunControl *createRunControl(ProjectExplorer::RunConfiguration *runConfiguration = 0);
+    ValgrindRunControl *createRunControl(ProjectExplorer::RunConfiguration *runConfiguration);
     QWidget *createWidgets();
 
     void handleShowCostsOfFunction();

src/plugins/valgrind/memcheck/memcheckrunner.cpp

@@ -95,13 +95,11 @@ bool MemcheckRunner::start()
 {
     QTC_ASSERT(d->parser, return false);
 
-    // The remote case is handled in localHostAddressRetrieved().
+    if (!startServers())
-    // FIXME: We start confusing "use startup project" with a "local/remote"
+        return false;
-    // decision here.
+
-    if (useStartupProject()) {
-        startServers(QHostAddress(QHostAddress::LocalHost));
     setValgrindArguments(memcheckLogArguments() + valgrindArguments());
-    }
+
     return ValgrindRunner::start();
 }
 
@@ -135,10 +133,10 @@ void MemcheckRunner::readLogSocket()
     emit logMessageReceived(d->logSocket->readAll());
 }
 
-// Note: The callers of this function cannot handle errors, so they will ignore the return value.
+bool MemcheckRunner::startServers()
-// We still provide it in case the surrounding infrastructure will improve.
-bool MemcheckRunner::startServers(const QHostAddress &localHostAddress)
 {
+    QHostAddress localHostAddress(QHostAddress::LocalHost);
+
     bool check = d->xmlServer.listen(localHostAddress);
     const QString ip = localHostAddress.toString();
     if (!check) {
@@ -176,8 +174,7 @@ QStringList MemcheckRunner::memcheckLogArguments() const
 
 void MemcheckRunner::localHostAddressRetrieved(const QHostAddress &localHostAddress)
 {
-    startServers(localHostAddress);
+    Q_UNUSED(localHostAddress);
-    setValgrindArguments(memcheckLogArguments() + valgrindArguments());
     valgrindProcess()->setValgrindArguments(fullValgrindArguments());
 }
 

src/plugins/valgrind/memcheck/memcheckrunner.h

@@ -62,7 +62,7 @@ private slots:
 private:
     QString tool() const;
 
-    bool startServers(const QHostAddress &localHostAddress);
+    bool startServers();
     QStringList memcheckLogArguments() const;
 
     class Private;

src/plugins/valgrind/valgrindengine.cpp

@@ -52,11 +52,10 @@ namespace Valgrind {
 namespace Internal {
 
 ValgrindRunControl::ValgrindRunControl(RunConfiguration *runConfiguration, Core::Id runMode)
-    : AnalyzerRunControl(runConfiguration, runMode),
+    : AnalyzerRunControl(runConfiguration, runMode)
-      m_settings(0),
-      m_isStopping(false)
 {
-    m_isCustomStart = false;
+    QTC_ASSERT(runConfiguration, return);
+    setRunnable(runConfiguration->runnable());
 
     if (runConfiguration)
         if (IRunConfigurationAspect *aspect = runConfiguration->extraAspect(ANALYZER_VALGRIND_SETTINGS))
@@ -87,8 +86,8 @@ bool ValgrindRunControl::startEngine()
     ValgrindRunner *run = runner();
     run->setValgrindExecutable(m_settings->valgrindExecutable());
     run->setValgrindArguments(genericToolArguments() + toolArguments());
-    run->setConnectionParameters(connection().as<AnalyzerConnection>().connParams);
+    QTC_ASSERT(!device().isNull(), return false);
-    run->setUseStartupProject(!m_isCustomStart);
+    run->setDevice(device());
     run->setDebuggee(runnable().as<StandardRunnable>());
 
     connect(run, &ValgrindRunner::processOutputReceived,

src/plugins/valgrind/valgrindengine.h

@@ -51,16 +51,13 @@ public:
 
     QString executable() const;
 
-    void setCustomStart() { m_isCustomStart = true; }
-
 protected:
     virtual QString progressTitle() const = 0;
     virtual QStringList toolArguments() const = 0;
     virtual Valgrind::ValgrindRunner *runner() = 0;
 
-    ValgrindBaseSettings *m_settings;
+    ValgrindBaseSettings *m_settings = 0;
     QFutureInterface<void> m_progress;
-    bool m_isCustomStart;
 
 private:
     void handleProgressCanceled();
@@ -73,7 +70,7 @@ private:
     QStringList genericToolArguments() const;
 
 private:
-    bool m_isStopping;
+    bool m_isStopping = false;
 };
 
 } // namespace Internal

src/plugins/valgrind/valgrindplugin.cpp

@@ -199,7 +199,6 @@ void ValgrindPlugin::extensionsInitialized()
         connection.connParams = dlg.sshParams();
         rc->setConnection(connection);
         rc->setDisplayName(runnable.executable);
-        rc->setCustomStart();
         ProjectExplorerPlugin::startRunControl(rc, MEMCHECK_RUN_MODE);
     });
     action->setText(tr("Valgrind Memory Analyzer (External Remote Application)"));
@@ -223,7 +222,6 @@ void ValgrindPlugin::extensionsInitialized()
         connection.connParams = dlg.sshParams();
         rc->setConnection(connection);
         rc->setDisplayName(runnable.executable);
-        rc->setCustomStart();
         ProjectExplorerPlugin::startRunControl(rc, CALLGRIND_RUN_MODE);
     });
 

src/plugins/valgrind/valgrindprocess.cpp

@@ -37,12 +37,11 @@ using namespace ProjectExplorer;
 
 namespace Valgrind {
 
-ValgrindProcess::ValgrindProcess(const QSsh::SshConnectionParameters &sshParams,
+ValgrindProcess::ValgrindProcess(const IDevice::ConstPtr &device,
-                                 QSsh::SshConnection *connection, QObject *parent)
+                                 QObject *parent)
-    : QObject(parent)
+    : QObject(parent), m_device(device)
 {
-    m_params = sshParams;
+    m_remote.m_connection = 0;
-    m_remote.m_connection = connection;
     m_remote.m_error = QProcess::UnknownError;
     m_pid = 0;
 }
@@ -126,8 +125,9 @@ void ValgrindProcess::run()
 
     } else {
         // connect to host and wait for connection
+        // FIXME: Use aquireConnection() instead.
         if (!m_remote.m_connection)
-            m_remote.m_connection = new QSsh::SshConnection(m_params, this);
+            m_remote.m_connection = new QSsh::SshConnection(m_device->sshParameters(), this);
 
         if (m_remote.m_connection->state() != QSsh::SshConnection::Connected) {
             connect(m_remote.m_connection, &QSsh::SshConnection::connected,
@@ -227,7 +227,7 @@ QSsh::SshConnection *ValgrindProcess::connection() const
 
 bool ValgrindProcess::isLocal() const
 {
-    return m_params.host.isEmpty();
+    return m_device->type() == ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE;
 }
 
 void ValgrindProcess::localProcessStarted()

src/plugins/valgrind/valgrindprocess.h

@@ -28,6 +28,7 @@
 #define VALGRINDPROCESS_H
 
 #include <projectexplorer/applicationlauncher.h>
+#include <projectexplorer/devicesupport/idevice.h>
 #include <projectexplorer/runnables.h>
 
 #include <ssh/sshremoteprocess.h>
@@ -45,8 +46,7 @@ class ValgrindProcess : public QObject
     Q_OBJECT
 
 public:
-    ValgrindProcess(const QSsh::SshConnectionParameters &sshParams,
+    ValgrindProcess(const ProjectExplorer::IDevice::ConstPtr &device, QObject *parent);
-                    QSsh::SshConnection *connection, QObject *parent);
 
     bool isRunning() const;
 
@@ -63,6 +63,8 @@ public:
     void setProcessChannelMode(QProcess::ProcessChannelMode mode);
     QString workingDirectory() const;
 
+    ProjectExplorer::IDevice::ConstPtr device() const { return m_device; }
+
     qint64 pid() const;
     QSsh::SshConnection *connection() const;
     bool isLocal() const;
@@ -90,6 +92,7 @@ private:
     ProjectExplorer::StandardRunnable m_debuggee;
     ProjectExplorer::ApplicationLauncher m_localProcess;
     qint64 m_pid;
+    ProjectExplorer::IDevice::ConstPtr m_device;
 
     struct Remote {
         QSsh::SshConnection *m_connection;

src/plugins/valgrind/valgrindruncontrolfactory.cpp

@@ -33,25 +33,12 @@
 
 #include <analyzerbase/ianalyzertool.h>
 #include <analyzerbase/analyzermanager.h>
-#include <analyzerbase/analyzerstartparameters.h>
-#include <analyzerbase/analyzerruncontrol.h>
 #include <analyzerbase/analyzerrunconfigwidget.h>
 
-#include <debugger/debuggerrunconfigurationaspect.h>
-
-#include <projectexplorer/environmentaspect.h>
-#include <projectexplorer/kitinformation.h>
-#include <projectexplorer/projectexplorer.h>
-#include <projectexplorer/runnables.h>
-#include <projectexplorer/target.h>
-
 #include <utils/qtcassert.h>
 
-#include <QTcpServer>
-
 using namespace Analyzer;
 using namespace ProjectExplorer;
-using namespace Utils;
 
 namespace Valgrind {
 namespace Internal {
@@ -70,24 +57,7 @@ bool ValgrindRunControlFactory::canRun(RunConfiguration *runConfiguration, Core:
 RunControl *ValgrindRunControlFactory::create(RunConfiguration *runConfiguration, Core::Id mode, QString *errorMessage)
 {
     Q_UNUSED(errorMessage);
-    AnalyzerRunControl *runControl = AnalyzerManager::createRunControl(runConfiguration, mode);
+    return AnalyzerManager::createRunControl(runConfiguration, mode);
-    QTC_ASSERT(runControl, return 0);
-
-    IDevice::ConstPtr device = DeviceKitInformation::device(runConfiguration->target()->kit());
-    AnalyzerConnection connection;
-    if (device->type() == ProjectExplorer::Constants::DESKTOP_DEVICE_TYPE) {
-        QTcpServer server;
-        if (!server.listen(QHostAddress::LocalHost) && !server.listen(QHostAddress::LocalHostIPv6)) {
-            qWarning() << "Cannot open port on host for profiling.";
-            return 0;
-        }
-    } else {
-        connection.connParams = device->sshParameters();
-    }
-
-    runControl->setRunnable(runConfiguration->runnable());
-    runControl->setConnection(connection);
-    return runControl;
 }
 
 class ValgrindRunConfigurationAspect : public IRunConfigurationAspect

src/plugins/valgrind/valgrindrunner.cpp

@@ -56,8 +56,7 @@ public:
     QString valgrindExecutable;
     QStringList valgrindArguments;
     StandardRunnable debuggee;
-    bool useStartupProject = true;
+    IDevice::ConstPtr device;
-    QSsh::SshConnectionParameters connParams;
 };
 
 void ValgrindRunner::Private::run(ValgrindProcess *_process)
@@ -144,29 +143,19 @@ void ValgrindRunner::setDebuggee(const StandardRunnable &debuggee)
     d->debuggee = debuggee;
 }
 
-const QSsh::SshConnectionParameters &ValgrindRunner::connectionParameters() const
-{
-    return d->connParams;
-}
-
-void ValgrindRunner::setConnectionParameters(const QSsh::SshConnectionParameters &connParams)
-{
-    d->connParams = connParams;
-}
-
 void ValgrindRunner::setProcessChannelMode(QProcess::ProcessChannelMode mode)
 {
     d->channelMode = mode;
 }
 
-void ValgrindRunner::setUseStartupProject(bool useStartupProject)
+void ValgrindRunner::setDevice(const IDevice::ConstPtr &device)
 {
-    d->useStartupProject = useStartupProject;
+    d->device = device;
 }
 
-bool ValgrindRunner::useStartupProject() const
+IDevice::ConstPtr ValgrindRunner::device() const
 {
-    return d->useStartupProject;
+    return d->device;
 }
 
 void ValgrindRunner::waitForFinished() const
@@ -181,8 +170,7 @@ void ValgrindRunner::waitForFinished() const
 
 bool ValgrindRunner::start()
 {
-    // FIXME: This wrongly uses "useStartupProject" for a Local/Remote decision.
+    d->run(new ValgrindProcess(d->device, this));
-    d->run(new ValgrindProcess(d->connParams, 0, this));
     return true;
 }
 

src/plugins/valgrind/valgrindrunner.h

@@ -36,8 +36,6 @@
 
 #include <QProcess>
 
-namespace Utils { class SshConnectionParameters; }
-
 namespace Valgrind {
 
 class ValgrindProcess;
@@ -58,11 +56,8 @@ public:
     void setDebuggee(const ProjectExplorer::StandardRunnable &debuggee) ;
     void setProcessChannelMode(QProcess::ProcessChannelMode mode);
 
-    void setUseStartupProject(bool useStartupProject);
+    void setDevice(const ProjectExplorer::IDevice::ConstPtr &device);
-    bool useStartupProject() const;
+    ProjectExplorer::IDevice::ConstPtr device() const;
-
-    void setConnectionParameters(const QSsh::SshConnectionParameters &connParams);
-    const QSsh::SshConnectionParameters &connectionParameters() const;
 
     void waitForFinished() const;
 

tests/auto/valgrind/valgrindautotest.qbs

@@ -2,6 +2,7 @@ import qbs
 
 QtcAutotest {
     Depends { name: "AnalyzerBase" }
+    Depends { name: "Core" }
     Depends { name: "QtcSsh" }
     Depends { name: "Utils" }
     Depends { name: "ProjectExplorer" }