From: Soeren Apel <redacted>
Date: Wed, 22 Jun 2016 16:02:34 +0000 (+0200)
Subject: Fix #805 by resetting selected device on failure
X-Git-Tag: pulseview-0.4.0~289
X-Git-Url: https://sigrok.org/gitaction?a=commitdiff_plain;h=7e0c99bf95836c89574b53ae3fa7840e2ddca77d;p=pulseview.git

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.
---

diff --git a/pv/mainwindow.cpp b/pv/mainwindow.cpp
index 4f6bc41d..2558cb07 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 73dd339b..b8e98ece 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 8c0b0639..cca4d9d5 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 6e2a7068..9c4b1cd4 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 a0fc10e6..d553aba3 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 ed69a4b6..589fbf23 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.