From 7e0c99bf95836c89574b53ae3fa7840e2ddca77d Mon Sep 17 00:00:00 2001
From: Soeren Apel <soeren@apelpie.net>
Date: Wed, 22 Jun 2016 18:02:34 +0200
Subject: [PATCH] Fix #805 by resetting selected device on failure

It can happen that devices can be selected but not used
(permissions problems, connection issues, driver issues, etc.),
so in those cases we want to fail gracefully instead of
segfaulting.
The reason for the segfault is the device selector button
isn't reset in case the device couldn't be opened, causing
the rest of the application to try and work with a device
instance that is actually invalid.

Resetting the device selector when the device failed to
open not only fixes this but also makes the UI more
consistent with the internal state.
---
 pv/mainwindow.cpp               |  5 ++++-
 pv/session.cpp                  | 10 +++++++++-
 pv/toolbars/mainbar.cpp         |  5 +++++
 pv/toolbars/mainbar.hpp         |  2 ++
 pv/widgets/devicetoolbutton.cpp | 11 ++++++++++-
 pv/widgets/devicetoolbutton.hpp |  6 ++++++
 6 files changed, 36 insertions(+), 3 deletions(-)

diff --git a/pv/mainwindow.cpp b/pv/mainwindow.cpp
index 4f6bc41..2558cb0 100644
--- a/pv/mainwindow.cpp
+++ b/pv/mainwindow.cpp
@@ -882,8 +882,11 @@ void MainWindow::device_selected()
 {
 	// Set the title to include the device/file name
 	const shared_ptr<devices::Device> device = session_.device();
-	if (!device)
+
+	if (!device) {
+		main_bar_->reset_device_selector();
 		return;
+	}
 
 	const string display_name = device->display_name(device_manager_);
 	setWindowTitle(tr("%1 - PulseView").arg(display_name.c_str()));
diff --git a/pv/session.cpp b/pv/session.cpp
index 73dd339..b8e98ec 100644
--- a/pv/session.cpp
+++ b/pv/session.cpp
@@ -161,7 +161,15 @@ void Session::set_device(shared_ptr<devices::Device> device)
 	signals_changed();
 
 	device_ = std::move(device);
-	device_->open();
+
+	try {
+		device_->open();
+	} catch (const QString &e) {
+		device_.reset();
+		device_selected();
+		throw;
+	}
+
 	device_->session()->add_datafeed_callback([=]
 		(shared_ptr<sigrok::Device> device, shared_ptr<Packet> packet) {
 			data_feed_in(device, packet);
diff --git a/pv/toolbars/mainbar.cpp b/pv/toolbars/mainbar.cpp
index 8c0b063..cca4d9d 100644
--- a/pv/toolbars/mainbar.cpp
+++ b/pv/toolbars/mainbar.cpp
@@ -248,6 +248,11 @@ void MainBar::set_capture_state(pv::Session::capture_state state)
 	sample_rate_.setEnabled(ui_enabled);
 }
 
+void MainBar::reset_device_selector()
+{
+	device_selector_.reset();
+}
+
 void MainBar::update_sample_rate_selector()
 {
 	Glib::VariantContainerBase gvar_dict;
diff --git a/pv/toolbars/mainbar.hpp b/pv/toolbars/mainbar.hpp
index 6e2a706..9c4b1cd 100644
--- a/pv/toolbars/mainbar.hpp
+++ b/pv/toolbars/mainbar.hpp
@@ -69,6 +69,8 @@ public:
 
 	void set_capture_state(pv::Session::capture_state state);
 
+	void reset_device_selector();
+
 private:
 	void update_sample_rate_selector();
 	void update_sample_rate_selector_value();
diff --git a/pv/widgets/devicetoolbutton.cpp b/pv/widgets/devicetoolbutton.cpp
index a0fc10e..d553aba 100644
--- a/pv/widgets/devicetoolbutton.cpp
+++ b/pv/widgets/devicetoolbutton.cpp
@@ -73,11 +73,18 @@ void DeviceToolButton::set_device_list(
 {
 	selected_device_ = selected;
 	setText(selected ? QString::fromStdString(
-		selected->display_name(device_manager_)) : "<No Device>");
+		selected->display_name(device_manager_)) : tr("<No Device>"));
 	devices_ = vector< weak_ptr<Device> >(devices.begin(), devices.end());
 	update_device_list();
 }
 
+void DeviceToolButton::reset()
+{
+	setText(tr("<No Device>"));
+	selected_device_.reset();
+	update_device_list();
+}
+
 void DeviceToolButton::update_device_list()
 {
 	menu_.clear();
@@ -108,6 +115,8 @@ void DeviceToolButton::on_action(QObject *action)
 {
 	assert(action);
 
+	selected_device_.reset();
+
 	Device *const dev = (Device*)((QAction*)action)->data().value<void*>();
 	for (weak_ptr<Device> dev_weak_ptr : devices_) {
 		shared_ptr<Device> dev_ptr(dev_weak_ptr);
diff --git a/pv/widgets/devicetoolbutton.hpp b/pv/widgets/devicetoolbutton.hpp
index ed69a4b..589fbf2 100644
--- a/pv/widgets/devicetoolbutton.hpp
+++ b/pv/widgets/devicetoolbutton.hpp
@@ -70,6 +70,12 @@ public:
 		const std::list< std::shared_ptr<devices::Device> > &devices,
 		std::shared_ptr<devices::Device> selected);
 
+	/**
+	 * Sets the current device to "no device". Useful for when a selected
+	 * device fails to open.
+	 */
+	void reset();
+
 private:
 	/**
 	 * Repopulates the menu from the device list.
-- 
2.30.2