From 19506296905cddb9a286921a0d56ab22c404aa12 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 20 Apr 2016 15:09:44 -0500
Subject: debug tracers

---
 src/greeter.cpp     | 9 ++++++++-
 src/usb-manager.cpp | 1 +
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index f9cd965..a064ec2 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -66,6 +66,8 @@ private:
 
     void on_bus_ready(GDBusConnection* bus)
     {
+        g_message("%s bus is ready", G_STRLOC);
+
         m_bus = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(bus)));
 
         g_dbus_connection_call(m_bus,
@@ -95,6 +97,7 @@ private:
 
     static void on_get_is_active_ready(GObject* source, GAsyncResult* res, gpointer gself)
     {
+        g_message("%s", G_STRLOC);
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
@@ -102,6 +105,7 @@ private:
                 g_warning("UsbSnap: Error getting session bus: %s", error->message);
             g_clear_error(&error);
         } else {
+            g_message("%s v is %s", G_STRLOC, g_variant_print(v, true));
             GVariant* is_active {};
             g_variant_get_child(v, 0, "v", &is_active);
             static_cast<Impl*>(gself)->m_is_active.set(g_variant_get_boolean(is_active));
@@ -124,10 +128,13 @@ private:
         g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE)));
 
         auto v = g_variant_get_child_value (parameters, 1);
+        if (v != nullptr)
+            g_message("%s v is %s", G_STRLOC, g_variant_print(v, true));
+
         gboolean is_active {};
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
-            g_debug("%s is_active changed to %d", G_STRLOC, int(is_active));
+            g_message("%s is_active changed to %d", G_STRLOC, int(is_active));
             static_cast<Impl*>(gself)->m_is_active.set(is_active);
         }
         g_clear_pointer(&v, g_variant_unref);
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 4d750c0..d2d82c3 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -92,6 +92,7 @@ private:
 
     void maybe_snap()
     {
+g_message("%s m_req.public_key.empty() %d && m_greeter->is_active().get() %d", G_STRLOC, int(m_req.public_key.empty()), int(m_greeter->is_active().get()));
         // don't prompt in the greeter!
         if (!m_req.public_key.empty() && !m_greeter->is_active().get())
             snap();
-- 
cgit v1.2.3


From e683501043c00810558db55461b49c66ecf1aa0b Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 20 Apr 2016 16:36:10 -0500
Subject: more tracers

---
 src/greeter.cpp | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index a064ec2..d2e34a0 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -29,6 +29,7 @@ public:
     Impl():
         m_cancellable{g_cancellable_new()}
     {
+        g_message("%s getting bus", G_STRLOC);
         g_bus_get(G_BUS_TYPE_SESSION, m_cancellable, on_bus_ready_static, this);
     }
 
@@ -52,6 +53,7 @@ private:
 
     static void on_bus_ready_static(GObject* /*source*/, GAsyncResult* res, gpointer gself)
     {
+        g_message("%s %s", G_STRLOC, G_STRFUNC);
         GError* error {};
         auto bus = g_bus_get_finish (res, &error);
         if (error != nullptr) {
@@ -122,6 +124,8 @@ private:
                                              GVariant* parameters,
                                              gpointer gself)
     {
+        g_message("%s", G_STRLOC);
+
         g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH));
         g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Properties::INTERFACE));
         g_return_if_fail(!g_strcmp0(signal_name, DBusNames::Properties::PropertiesChanged::NAME));
-- 
cgit v1.2.3


From cb46051005d5d9d1f21d7cb82b9fffd21f88c4c9 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 20 Apr 2016 17:09:14 -0500
Subject: watch for the Greeter to appear on the bus

---
 src/greeter.cpp | 144 ++++++++++++++++++++++++++++++++------------------------
 src/greeter.h   |   4 +-
 src/main.cpp    |  12 ++++-
 3 files changed, 96 insertions(+), 64 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index d2e34a0..9d331d7 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -26,21 +26,40 @@ class UnityGreeter::Impl
 {
 public:
 
-    Impl():
+    explicit Impl(GDBusConnection* connection):
+        m_bus(G_DBUS_CONNECTION(g_object_ref(connection))),
         m_cancellable{g_cancellable_new()}
     {
-        g_message("%s getting bus", G_STRLOC);
-        g_bus_get(G_BUS_TYPE_SESSION, m_cancellable, on_bus_ready_static, this);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
+
+        m_watcher_id = g_bus_watch_name_on_connection(
+            m_bus,
+            DBusNames::UnityGreeter::NAME,
+            G_BUS_NAME_WATCHER_FLAGS_AUTO_START,
+            on_greeter_appeared,
+            on_greeter_vanished,
+            this,
+            nullptr);
+
+        m_subscription_id = g_dbus_connection_signal_subscribe(
+            m_bus,
+            DBusNames::UnityGreeter::NAME,
+            DBusNames::Properties::INTERFACE,
+            DBusNames::Properties::PropertiesChanged::NAME,
+            DBusNames::UnityGreeter::PATH,
+            DBusNames::UnityGreeter::INTERFACE,
+            G_DBUS_SIGNAL_FLAGS_NONE,
+            on_properties_changed_signal,
+            this,
+            nullptr);
     }
 
     ~Impl()
     {
         g_cancellable_cancel(m_cancellable);
         g_clear_object(&m_cancellable);
-
-        if (m_subscription_id != 0)
-            g_dbus_connection_signal_unsubscribe (m_bus, m_subscription_id);
-
+        g_bus_unwatch_name(m_watcher_id);
+        g_dbus_connection_signal_unsubscribe(m_bus, m_subscription_id);
         g_clear_object(&m_bus);
     }
 
@@ -51,60 +70,55 @@ public:
 
 private:
 
-    static void on_bus_ready_static(GObject* /*source*/, GAsyncResult* res, gpointer gself)
+    static void on_greeter_appeared(
+        GDBusConnection* /*session_bus*/,
+        const char* /*name*/,
+        const char* name_owner,
+        gpointer gself)
     {
         g_message("%s %s", G_STRLOC, G_STRFUNC);
-        GError* error {};
-        auto bus = g_bus_get_finish (res, &error);
-        if (error != nullptr) {
-            if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-                g_warning("UsbSnap: Error getting session bus: %s", error->message);
-            g_clear_error(&error);
-        } else {
-            static_cast<Impl*>(gself)->on_bus_ready(bus);
-        }
-        g_clear_object(&bus);
+        auto self = static_cast<Impl*>(gself);
+
+        self->m_owner = name_owner;
+
+        g_dbus_connection_call(
+            self->m_bus,
+            DBusNames::UnityGreeter::NAME,
+            DBusNames::UnityGreeter::PATH,
+            DBusNames::Properties::INTERFACE,
+            "Get",
+            g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"),
+            G_VARIANT_TYPE("(v)"),
+            G_DBUS_CALL_FLAGS_NONE,
+            -1,
+            self->m_cancellable,
+            on_get_is_active_ready,
+            gself);
     }
 
-    void on_bus_ready(GDBusConnection* bus)
+    static void on_greeter_vanished(
+        GDBusConnection* /*session_bus*/,
+        const char* /*name*/,
+        gpointer gself)
     {
-        g_message("%s bus is ready", G_STRLOC);
-
-        m_bus = G_DBUS_CONNECTION(g_object_ref(G_OBJECT(bus)));
-
-        g_dbus_connection_call(m_bus,
-                               DBusNames::UnityGreeter::NAME,
-                               DBusNames::UnityGreeter::PATH,
-                               DBusNames::Properties::INTERFACE,
-                               "Get",
-                               g_variant_new("(ss)", DBusNames::UnityGreeter::INTERFACE, "IsActive"),
-                               G_VARIANT_TYPE("(v)"),
-                               G_DBUS_CALL_FLAGS_NONE,
-                               -1,
-                               m_cancellable,
-                               on_get_is_active_ready,
-                               this);
-
-        m_subscription_id = g_dbus_connection_signal_subscribe(m_bus,
-                                                               DBusNames::UnityGreeter::NAME,
-                                                               DBusNames::Properties::INTERFACE,
-                                                               DBusNames::Properties::PropertiesChanged::NAME,
-                                                               DBusNames::UnityGreeter::PATH,
-                                                               DBusNames::UnityGreeter::INTERFACE,
-                                                               G_DBUS_SIGNAL_FLAGS_NONE,
-                                                               on_properties_changed_signal,
-                                                               this,
-                                                               nullptr);
+        g_message("%s %s", G_STRLOC, G_STRFUNC);
+        auto self = static_cast<Impl*>(gself);
+
+        self->m_owner.clear();
+        self->m_is_active.set(false);
     }
 
-    static void on_get_is_active_ready(GObject* source, GAsyncResult* res, gpointer gself)
+    static void on_get_is_active_ready(
+        GObject* source,
+        GAsyncResult* res, 
+        gpointer gself)
     {
         g_message("%s", G_STRLOC);
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
             if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-                g_warning("UsbSnap: Error getting session bus: %s", error->message);
+                g_warning("Greeter: Error getting IsActive property: %s", error->message);
             g_clear_error(&error);
         } else {
             g_message("%s v is %s", G_STRLOC, g_variant_print(v, true));
@@ -116,22 +130,24 @@ private:
         g_clear_pointer(&v, g_variant_unref);
     }
 
-    static void on_properties_changed_signal(GDBusConnection* /*connection*/,
-                                             const gchar* /*sender_name*/,
-                                             const gchar* object_path,
-                                             const gchar* interface_name,
-                                             const gchar* signal_name,
-                                             GVariant* parameters,
-                                             gpointer gself)
+    static void on_properties_changed_signal(
+        GDBusConnection* /*connection*/,
+        const gchar* sender_name,
+        const gchar* object_path,
+        const gchar* interface_name,
+        const gchar* signal_name,
+        GVariant* parameters,
+        gpointer gself)
     {
-        g_message("%s", G_STRLOC);
+        auto self = static_cast<Impl*>(gself);
 
+        g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str()));
         g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH));
         g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Properties::INTERFACE));
         g_return_if_fail(!g_strcmp0(signal_name, DBusNames::Properties::PropertiesChanged::NAME));
         g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE)));
 
-        auto v = g_variant_get_child_value (parameters, 1);
+        auto v = g_variant_get_child_value(parameters, 1);
         if (v != nullptr)
             g_message("%s v is %s", G_STRLOC, g_variant_print(v, true));
 
@@ -139,15 +155,19 @@ private:
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
             g_message("%s is_active changed to %d", G_STRLOC, int(is_active));
-            static_cast<Impl*>(gself)->m_is_active.set(is_active);
+            self->m_is_active.set(is_active);
         }
         g_clear_pointer(&v, g_variant_unref);
+
     }
 
-    core::Property<bool> m_is_active;
-    GCancellable* m_cancellable {};
+    core::Property<bool> m_is_active {false};
+
     GDBusConnection* m_bus {};
+    GCancellable* m_cancellable {};
+    guint m_watcher_id {};
     unsigned int m_subscription_id {};
+    std::string m_owner;
 };
 
 /***
@@ -158,8 +178,8 @@ Greeter::Greeter() =default;
 
 Greeter::~Greeter() =default;
 
-UnityGreeter::UnityGreeter():
-    impl{new Impl{}}
+UnityGreeter::UnityGreeter(GDBusConnection* connection):
+    impl{new Impl{connection}}
 {
 }
 
diff --git a/src/greeter.h b/src/greeter.h
index e084d25..6707c4f 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -21,6 +21,8 @@
 
 #include <core/property.h>
 
+#include <gio/gio.h>
+
 #include <memory>
 #include <string>
 
@@ -36,7 +38,7 @@ public:
 class UnityGreeter: public Greeter
 {
 public:
-    UnityGreeter();
+    explicit UnityGreeter(GDBusConnection* connection);
     virtual ~UnityGreeter();
     core::Property<bool>& is_active() override;
 
diff --git a/src/main.cpp b/src/main.cpp
index 52cdd58..ee05f52 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -47,6 +47,15 @@ main(int /*argc*/, char** /*argv*/)
       g_main_loop_quit(loop);
     };
 
+    // get the session bus
+    GError* error {};
+    auto session_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &error);
+    if (error != nullptr) {
+        g_critical("Unable to get session bus: %s", error->message);
+        g_clear_error(&error);
+        return 0;
+    }
+
     // build all our indicators.
     // Right now we've only got one -- rotation lock -- but hey, we can dream.
     std::vector<std::shared_ptr<Indicator>> indicators;
@@ -63,7 +72,7 @@ main(int /*argc*/, char** /*argv*/)
     static constexpr char const * ADB_SOCKET_PATH {"/dev/socket/adbd"};
     static constexpr char const * PUBLIC_KEYS_FILENAME {"/data/misc/adb/adb_keys"};
     auto usb_monitor = std::make_shared<GUDevUsbMonitor>();
-    auto greeter = std::make_shared<UnityGreeter>();
+    auto greeter = std::make_shared<UnityGreeter>(session_bus);
     UsbManager usb_manager {ADB_SOCKET_PATH, PUBLIC_KEYS_FILENAME, usb_monitor, greeter};
 
     // let's go!
@@ -71,5 +80,6 @@ main(int /*argc*/, char** /*argv*/)
 
     // cleanup
     g_main_loop_unref(loop);
+    g_clear_object(&session_bus);
     return 0;
 }
-- 
cgit v1.2.3


From e1284eb774d75f96d61787f116b0d79328b27286 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 20 Apr 2016 17:32:17 -0500
Subject: remove tracers

---
 src/greeter.cpp     | 10 +---------
 src/usb-manager.cpp |  1 -
 2 files changed, 1 insertion(+), 10 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index 9d331d7..903bd2a 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -30,8 +30,6 @@ public:
         m_bus(G_DBUS_CONNECTION(g_object_ref(connection))),
         m_cancellable{g_cancellable_new()}
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-
         m_watcher_id = g_bus_watch_name_on_connection(
             m_bus,
             DBusNames::UnityGreeter::NAME,
@@ -76,7 +74,6 @@ private:
         const char* name_owner,
         gpointer gself)
     {
-        g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner = name_owner;
@@ -101,7 +98,6 @@ private:
         const char* /*name*/,
         gpointer gself)
     {
-        g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner.clear();
@@ -113,7 +109,6 @@ private:
         GAsyncResult* res, 
         gpointer gself)
     {
-        g_message("%s", G_STRLOC);
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
@@ -121,7 +116,6 @@ private:
                 g_warning("Greeter: Error getting IsActive property: %s", error->message);
             g_clear_error(&error);
         } else {
-            g_message("%s v is %s", G_STRLOC, g_variant_print(v, true));
             GVariant* is_active {};
             g_variant_get_child(v, 0, "v", &is_active);
             static_cast<Impl*>(gself)->m_is_active.set(g_variant_get_boolean(is_active));
@@ -148,13 +142,11 @@ private:
         g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE)));
 
         auto v = g_variant_get_child_value(parameters, 1);
-        if (v != nullptr)
-            g_message("%s v is %s", G_STRLOC, g_variant_print(v, true));
 
         gboolean is_active {};
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
-            g_message("%s is_active changed to %d", G_STRLOC, int(is_active));
+            g_debug("%s is_active changed to %d", G_STRLOC, int(is_active));
             self->m_is_active.set(is_active);
         }
         g_clear_pointer(&v, g_variant_unref);
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index d2d82c3..4d750c0 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -92,7 +92,6 @@ private:
 
     void maybe_snap()
     {
-g_message("%s m_req.public_key.empty() %d && m_greeter->is_active().get() %d", G_STRLOC, int(m_req.public_key.empty()), int(m_greeter->is_active().get()));
         // don't prompt in the greeter!
         if (!m_req.public_key.empty() && !m_greeter->is_active().get())
             snap();
-- 
cgit v1.2.3


From e1c1a9ae367c53561cdb4f53ad8589e2bc859b0b Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 20 Apr 2016 19:57:03 -0500
Subject: add unit tests for greeter

---
 tests/unit/CMakeLists.txt         |   5 ++
 tests/unit/greeter-test.cpp       | 151 ++++++++++++++++++++++++++++++++++++++
 tests/utils/mock-unity-greeter.py |  41 +++++++++++
 3 files changed, 197 insertions(+)
 create mode 100644 tests/unit/greeter-test.cpp
 create mode 100644 tests/utils/mock-unity-greeter.py

diff --git a/tests/unit/CMakeLists.txt b/tests/unit/CMakeLists.txt
index fe70461..9d8cad2 100644
--- a/tests/unit/CMakeLists.txt
+++ b/tests/unit/CMakeLists.txt
@@ -14,6 +14,10 @@ set(TEST_LINK_LIBRARIES
     ${GMOCK_LIBRARIES}
 )
 
+add_definitions(
+  -DGREETER_TEMPLATE="${CMAKE_SOURCE_DIR}/tests/utils/mock-unity-greeter.py"
+)
+
 function(add_test_by_name name)
   set(TEST_NAME ${name})
   add_executable (${TEST_NAME} ${TEST_NAME}.cpp)
@@ -31,4 +35,5 @@ function(add_qt_test_by_name name)
   set_property(TEST ${TEST_NAME} APPEND PROPERTY ENVIRONMENT ${CTEST_ENVIRONMENT})
   target_link_libraries(${TEST_NAME} ${SERVICE_LINK_LIBRARIES} ${QT_LINK_LIBRARIES} ${TEST_LINK_LIBRARIES} ${THREAD_LINK_LIBRARIES})
 endfunction()
+add_qt_test_by_name(greeter-test)
 add_qt_test_by_name(usb-snap-test)
diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
new file mode 100644
index 0000000..5d29023
--- /dev/null
+++ b/tests/unit/greeter-test.cpp
@@ -0,0 +1,151 @@
+/*
+ * Copyright 2016 Canonical Ltd.
+ *
+ * This program is free software: you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 3, as published
+ * by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful, but
+ * WITHOUT ANY WARRANTY; without even the implied warranties of
+ * MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+ * PURPOSE.  See the GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program.  If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *   Charles Kerr <charles.kerr@canonical.com>
+ */
+
+#include <tests/utils/qt-fixture.h>
+
+#include <src/dbus-names.h>
+#include <src/greeter.h>
+
+#include <libqtdbustest/DBusTestRunner.h>
+#include <libqtdbustest/QProcessDBusService.h>
+#include <libqtdbusmock/DBusMock.h>
+
+class GreeterFixture: public QtFixture
+{
+private:
+
+    using super = QtFixture;
+
+public:
+
+    GreeterFixture() =default;
+    ~GreeterFixture() =default;
+
+protected:
+
+    std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner;
+    std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock;
+    GDBusConnection* m_session_bus;
+
+    void SetUp() override
+    {
+        super::SetUp();
+  
+        // use a fresh bus for each test
+ 
+        m_dbus_runner.reset(new QtDBusTest::DBusTestRunner());
+        m_dbus_mock.reset(new QtDBusMock::DBusMock(*m_dbus_runner.get()));
+
+        GError* error {};
+        m_session_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &error);
+        g_assert_no_error(error);
+        g_dbus_connection_set_exit_on_close(m_session_bus, false);
+    }
+
+    void TearDown() override
+    {
+        g_clear_object(&m_session_bus);
+        m_dbus_mock.reset();
+        m_dbus_runner.reset();
+
+        super::TearDown();
+    }
+
+    void start_greeter_service(bool is_active)
+    {
+        // set a watcher to look for our mock to appear
+        bool owned {};
+        QDBusServiceWatcher watcher(
+            DBusNames::UnityGreeter::NAME,
+            m_dbus_runner->sessionConnection()
+        );
+        QObject::connect(
+            &watcher,
+            &QDBusServiceWatcher::serviceRegistered,
+            [&owned](const QString&){owned = true;}
+        );
+
+        // start the mock greeter
+        QVariantMap parameters;
+        parameters["IsActive"] = QVariant(is_active);
+        m_dbus_mock->registerTemplate(
+            DBusNames::UnityGreeter::NAME,
+            GREETER_TEMPLATE,
+            parameters,
+            QDBusConnection::SessionBus
+        );
+        m_dbus_runner->startServices();
+
+        // wait for the watcher to be triggered
+        ASSERT_TRUE(wait_for([&owned]{return owned;}));
+    }
+};
+
+#define ASSERT_PROPERTY_EVENTUALLY(expected_in, property_in) \
+    do { \
+        const auto& e = expected_in; \
+        const auto& p = property_in; \
+        ASSERT_TRUE(wait_for([e, &p](){ return e == p.get(); })) \
+            << "expected " << e << " but got " << p.get(); \
+   } while(0)
+
+TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
+{
+    constexpr bool expected {true};
+
+    start_greeter_service(expected);
+
+    UnityGreeter greeter(m_session_bus);
+
+    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+}
+
+TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
+{
+    constexpr bool expected {true};
+
+    UnityGreeter greeter(m_session_bus);
+
+    start_greeter_service(expected);
+
+    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+}
+
+TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
+{
+    constexpr bool expected {false};
+
+    start_greeter_service(expected);
+
+    UnityGreeter greeter(m_session_bus);
+
+    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+}
+
+TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
+{
+    constexpr bool expected {false};
+
+    UnityGreeter greeter(m_session_bus);
+
+    start_greeter_service(expected);
+
+    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+}
+
diff --git a/tests/utils/mock-unity-greeter.py b/tests/utils/mock-unity-greeter.py
new file mode 100644
index 0000000..70fb791
--- /dev/null
+++ b/tests/utils/mock-unity-greeter.py
@@ -0,0 +1,41 @@
+'''unity greeter mock template
+
+Very basic template that just mocks the greeter is-active flag
+'''
+
+# This program is free software; you can redistribute it and/or modify it under
+# the terms of the GNU Lesser General Public License as published by the Free
+# Software Foundation; either version 3 of the License, or (at your option) any
+# later version.  See http://www.gnu.org/copyleft/lgpl.html for the full text
+# of the license.
+
+__author__ = 'Charles Kerr'
+__email__ = 'charles.kerr@canonical.com'
+__copyright__ = '(c) 2016 Canonical Ltd.'
+__license__ = 'LGPL 3+'
+
+import dbus
+import os
+
+from dbusmock import MOCK_IFACE, mockobject
+
+BUS_NAME = 'com.canonical.UnityGreeter'
+MAIN_OBJ = '/'
+MAIN_IFACE = 'com.canonical.UnityGreeter'
+SYSTEM_BUS = False
+
+
+def load(mock, parameters):
+    mock.AddMethods(
+        MAIN_IFACE, [
+            ('HideGreeter', '', '', 'self.Set("com.canonical.UnityGreeter", "IsActive", False)'),
+            ('ShowGreeter', '', '', 'self.Set("com.canonical.UnityGreeter", "IsActive", True)')
+        ]
+    )
+    mock.AddProperties(
+        MAIN_IFACE,
+        dbus.Dictionary({
+            'IsActive': parameters.get('IsActive', False),
+        }, signature='sv')
+    )
+
-- 
cgit v1.2.3


From 3b84a37b2caae0c7c971b3c7cc5df9be1319a5ac Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 21 Apr 2016 09:19:25 -0500
Subject: don't pass a GDBusConnection to the Greeter ctor

---
 src/greeter.cpp             | 109 +++++++++++++++++++++++++++-----------------
 src/greeter.h               |   4 +-
 src/main.cpp                |  12 +----
 tests/unit/greeter-test.cpp |  10 ++--
 4 files changed, 75 insertions(+), 60 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index 903bd2a..7f79bf9 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -26,40 +26,20 @@ class UnityGreeter::Impl
 {
 public:
 
-    explicit Impl(GDBusConnection* connection):
-        m_bus(G_DBUS_CONNECTION(g_object_ref(connection))),
-        m_cancellable{g_cancellable_new()}
+    explicit Impl()
     {
-        m_watcher_id = g_bus_watch_name_on_connection(
-            m_bus,
-            DBusNames::UnityGreeter::NAME,
-            G_BUS_NAME_WATCHER_FLAGS_AUTO_START,
-            on_greeter_appeared,
-            on_greeter_vanished,
-            this,
-            nullptr);
-
-        m_subscription_id = g_dbus_connection_signal_subscribe(
-            m_bus,
-            DBusNames::UnityGreeter::NAME,
-            DBusNames::Properties::INTERFACE,
-            DBusNames::Properties::PropertiesChanged::NAME,
-            DBusNames::UnityGreeter::PATH,
-            DBusNames::UnityGreeter::INTERFACE,
-            G_DBUS_SIGNAL_FLAGS_NONE,
-            on_properties_changed_signal,
-            this,
-            nullptr);
+        m_cancellable.reset(
+            g_cancellable_new(),
+            [](GCancellable* c){
+                g_cancellable_cancel(c);
+                g_clear_object(&c);
+            }
+        );
+
+        g_bus_get(G_BUS_TYPE_SESSION, m_cancellable.get(), on_bus_ready, this);
     }
 
-    ~Impl()
-    {
-        g_cancellable_cancel(m_cancellable);
-        g_clear_object(&m_cancellable);
-        g_bus_unwatch_name(m_watcher_id);
-        g_dbus_connection_signal_unsubscribe(m_bus, m_subscription_id);
-        g_clear_object(&m_bus);
-    }
+    ~Impl() =default;
 
     core::Property<bool>& is_active()
     {
@@ -68,8 +48,57 @@ public:
 
 private:
 
+    static void on_bus_ready(
+        GObject* /*source*/,
+        GAsyncResult* res,
+        gpointer gself)
+    {
+        GError* error {};
+        auto bus = g_bus_get_finish(res, &error);
+        if (error != nullptr)
+        {
+            if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+                g_warning("Greeter: Error getting bus: %s", error->message);
+            g_clear_error(&error);
+        }
+        else
+        {
+            auto self = static_cast<Impl*>(gself);
+
+            const auto watcher_id = g_bus_watch_name_on_connection(
+                bus,
+                DBusNames::UnityGreeter::NAME,
+                G_BUS_NAME_WATCHER_FLAGS_AUTO_START,
+                on_greeter_appeared,
+                on_greeter_vanished,
+                gself,
+                nullptr);
+
+            const auto subscription_id = g_dbus_connection_signal_subscribe(
+                bus,
+                DBusNames::UnityGreeter::NAME,
+                DBusNames::Properties::INTERFACE,
+                DBusNames::Properties::PropertiesChanged::NAME,
+                DBusNames::UnityGreeter::PATH,
+                DBusNames::UnityGreeter::INTERFACE,
+                G_DBUS_SIGNAL_FLAGS_NONE,
+                on_properties_changed_signal,
+                gself,
+                nullptr);
+
+            self->m_bus.reset(
+                bus,
+                [watcher_id, subscription_id](GDBusConnection* o){
+                    g_bus_unwatch_name(watcher_id);
+                    g_dbus_connection_signal_unsubscribe(o, subscription_id);
+                    g_clear_object(&o);
+                }
+            );
+        }
+    }
+
     static void on_greeter_appeared(
-        GDBusConnection* /*session_bus*/,
+        GDBusConnection* bus,
         const char* /*name*/,
         const char* name_owner,
         gpointer gself)
@@ -79,7 +108,7 @@ private:
         self->m_owner = name_owner;
 
         g_dbus_connection_call(
-            self->m_bus,
+            bus,
             DBusNames::UnityGreeter::NAME,
             DBusNames::UnityGreeter::PATH,
             DBusNames::Properties::INTERFACE,
@@ -88,13 +117,13 @@ private:
             G_VARIANT_TYPE("(v)"),
             G_DBUS_CALL_FLAGS_NONE,
             -1,
-            self->m_cancellable,
+            self->m_cancellable.get(),
             on_get_is_active_ready,
             gself);
     }
 
     static void on_greeter_vanished(
-        GDBusConnection* /*session_bus*/,
+        GDBusConnection* /*bus*/,
         const char* /*name*/,
         gpointer gself)
     {
@@ -155,10 +184,8 @@ private:
 
     core::Property<bool> m_is_active {false};
 
-    GDBusConnection* m_bus {};
-    GCancellable* m_cancellable {};
-    guint m_watcher_id {};
-    unsigned int m_subscription_id {};
+    std::shared_ptr<GDBusConnection> m_bus;
+    std::shared_ptr<GCancellable> m_cancellable;
     std::string m_owner;
 };
 
@@ -170,8 +197,8 @@ Greeter::Greeter() =default;
 
 Greeter::~Greeter() =default;
 
-UnityGreeter::UnityGreeter(GDBusConnection* connection):
-    impl{new Impl{connection}}
+UnityGreeter::UnityGreeter():
+    impl{new Impl{}}
 {
 }
 
diff --git a/src/greeter.h b/src/greeter.h
index 6707c4f..9508220 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -21,8 +21,6 @@
 
 #include <core/property.h>
 
-#include <gio/gio.h>
-
 #include <memory>
 #include <string>
 
@@ -38,7 +36,7 @@ public:
 class UnityGreeter: public Greeter
 {
 public:
-    explicit UnityGreeter(GDBusConnection* connection);
+    explicit UnityGreeter();
     virtual ~UnityGreeter();
     core::Property<bool>& is_active() override;
 
diff --git a/src/main.cpp b/src/main.cpp
index ee05f52..52cdd58 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -47,15 +47,6 @@ main(int /*argc*/, char** /*argv*/)
       g_main_loop_quit(loop);
     };
 
-    // get the session bus
-    GError* error {};
-    auto session_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &error);
-    if (error != nullptr) {
-        g_critical("Unable to get session bus: %s", error->message);
-        g_clear_error(&error);
-        return 0;
-    }
-
     // build all our indicators.
     // Right now we've only got one -- rotation lock -- but hey, we can dream.
     std::vector<std::shared_ptr<Indicator>> indicators;
@@ -72,7 +63,7 @@ main(int /*argc*/, char** /*argv*/)
     static constexpr char const * ADB_SOCKET_PATH {"/dev/socket/adbd"};
     static constexpr char const * PUBLIC_KEYS_FILENAME {"/data/misc/adb/adb_keys"};
     auto usb_monitor = std::make_shared<GUDevUsbMonitor>();
-    auto greeter = std::make_shared<UnityGreeter>(session_bus);
+    auto greeter = std::make_shared<UnityGreeter>();
     UsbManager usb_manager {ADB_SOCKET_PATH, PUBLIC_KEYS_FILENAME, usb_monitor, greeter};
 
     // let's go!
@@ -80,6 +71,5 @@ main(int /*argc*/, char** /*argv*/)
 
     // cleanup
     g_main_loop_unref(loop);
-    g_clear_object(&session_bus);
     return 0;
 }
diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
index 5d29023..874efed 100644
--- a/tests/unit/greeter-test.cpp
+++ b/tests/unit/greeter-test.cpp
@@ -41,7 +41,7 @@ protected:
 
     std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner;
     std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock;
-    GDBusConnection* m_session_bus;
+    GDBusConnection* m_session_bus {};
 
     void SetUp() override
     {
@@ -111,7 +111,7 @@ TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 
     start_greeter_service(expected);
 
-    UnityGreeter greeter(m_session_bus);
+    UnityGreeter greeter;
 
     ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
 }
@@ -120,7 +120,7 @@ TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
 {
     constexpr bool expected {true};
 
-    UnityGreeter greeter(m_session_bus);
+    UnityGreeter greeter;
 
     start_greeter_service(expected);
 
@@ -133,7 +133,7 @@ TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
 
     start_greeter_service(expected);
 
-    UnityGreeter greeter(m_session_bus);
+    UnityGreeter greeter;
 
     ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
 }
@@ -142,7 +142,7 @@ TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
 {
     constexpr bool expected {false};
 
-    UnityGreeter greeter(m_session_bus);
+    UnityGreeter greeter;
 
     start_greeter_service(expected);
 
-- 
cgit v1.2.3


From d4e82e7d4fa1937d27d1397a036af2b6b03349b3 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 21 Apr 2016 09:33:08 -0500
Subject: code cleanup to prepare for MR

---
 src/greeter.cpp             | 15 ++++++---------
 src/greeter.h               |  2 +-
 tests/unit/greeter-test.cpp | 40 +++++++++++++++-------------------------
 3 files changed, 22 insertions(+), 35 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index 7f79bf9..d41160f 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -26,13 +26,13 @@ class UnityGreeter::Impl
 {
 public:
 
-    explicit Impl()
+    Impl()
     {
         m_cancellable.reset(
             g_cancellable_new(),
-            [](GCancellable* c){
-                g_cancellable_cancel(c);
-                g_clear_object(&c);
+            [](GCancellable* o){
+                g_cancellable_cancel(o);
+                g_clear_object(&o);
             }
         );
 
@@ -154,7 +154,7 @@ private:
     }
 
     static void on_properties_changed_signal(
-        GDBusConnection* /*connection*/,
+        GDBusConnection* /*bus*/,
         const gchar* sender_name,
         const gchar* object_path,
         const gchar* interface_name,
@@ -171,7 +171,6 @@ private:
         g_return_if_fail(g_variant_is_of_type(parameters, G_VARIANT_TYPE(DBusNames::Properties::PropertiesChanged::ARGS_VARIANT_TYPE)));
 
         auto v = g_variant_get_child_value(parameters, 1);
-
         gboolean is_active {};
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
@@ -179,13 +178,11 @@ private:
             self->m_is_active.set(is_active);
         }
         g_clear_pointer(&v, g_variant_unref);
-
     }
 
     core::Property<bool> m_is_active {false};
-
-    std::shared_ptr<GDBusConnection> m_bus;
     std::shared_ptr<GCancellable> m_cancellable;
+    std::shared_ptr<GDBusConnection> m_bus;
     std::string m_owner;
 };
 
diff --git a/src/greeter.h b/src/greeter.h
index 9508220..e084d25 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -36,7 +36,7 @@ public:
 class UnityGreeter: public Greeter
 {
 public:
-    explicit UnityGreeter();
+    UnityGreeter();
     virtual ~UnityGreeter();
     core::Property<bool>& is_active() override;
 
diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
index 874efed..b33c300 100644
--- a/tests/unit/greeter-test.cpp
+++ b/tests/unit/greeter-test.cpp
@@ -41,35 +41,19 @@ protected:
 
     std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner;
     std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock;
-    GDBusConnection* m_session_bus {};
 
     void SetUp() override
     {
         super::SetUp();
   
-        // use a fresh bus for each test
- 
+        // use a fresh bus for each test run
         m_dbus_runner.reset(new QtDBusTest::DBusTestRunner());
         m_dbus_mock.reset(new QtDBusMock::DBusMock(*m_dbus_runner.get()));
-
-        GError* error {};
-        m_session_bus = g_bus_get_sync(G_BUS_TYPE_SESSION, nullptr, &error);
-        g_assert_no_error(error);
-        g_dbus_connection_set_exit_on_close(m_session_bus, false);
-    }
-
-    void TearDown() override
-    {
-        g_clear_object(&m_session_bus);
-        m_dbus_mock.reset();
-        m_dbus_runner.reset();
-
-        super::TearDown();
     }
 
     void start_greeter_service(bool is_active)
     {
-        // set a watcher to look for our mock to appear
+        // set a watcher to look for our mock greeter to appear
         bool owned {};
         QDBusServiceWatcher watcher(
             DBusNames::UnityGreeter::NAME,
@@ -92,19 +76,25 @@ protected:
         );
         m_dbus_runner->startServices();
 
-        // wait for the watcher to be triggered
+        // wait for the watcher
         ASSERT_TRUE(wait_for([&owned]{return owned;}));
     }
 };
 
-#define ASSERT_PROPERTY_EVENTUALLY(expected_in, property_in) \
+#define ASSERT_PROPERTY_EQ_EVENTUALLY(expected_in, property_in) \
     do { \
         const auto& e = expected_in; \
         const auto& p = property_in; \
-        ASSERT_TRUE(wait_for([e, &p](){ return e == p.get(); })) \
+        ASSERT_TRUE(wait_for([e, &p](){return e == p.get();})) \
             << "expected " << e << " but got " << p.get(); \
    } while(0)
 
+/**
+ * Test startup timing by looking at four different cases:
+ * [unity greeter shows up on bus (before, after) we start listening]
+ *   x [unity greeter is (active, inactive)]
+ */
+
 TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 {
     constexpr bool expected {true};
@@ -113,7 +103,7 @@ TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
@@ -124,7 +114,7 @@ TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
 
     start_greeter_service(expected);
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
 TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
@@ -135,7 +125,7 @@ TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
@@ -146,6 +136,6 @@ TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
 
     start_greeter_service(expected);
 
-    ASSERT_PROPERTY_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
 }
 
-- 
cgit v1.2.3


From b37db33c628675971f118fb3e241dc32a9f0a5d0 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 21 Apr 2016 11:22:56 -0500
Subject: tell glib not to exit when the gdbusconnection is closed

---
 tests/unit/greeter-test.cpp | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
index b33c300..72df3bc 100644
--- a/tests/unit/greeter-test.cpp
+++ b/tests/unit/greeter-test.cpp
@@ -41,6 +41,7 @@ protected:
 
     std::shared_ptr<QtDBusTest::DBusTestRunner> m_dbus_runner;
     std::shared_ptr<QtDBusMock::DBusMock> m_dbus_mock;
+    GDBusConnection* m_bus {};
 
     void SetUp() override
     {
@@ -49,6 +50,18 @@ protected:
         // use a fresh bus for each test run
         m_dbus_runner.reset(new QtDBusTest::DBusTestRunner());
         m_dbus_mock.reset(new QtDBusMock::DBusMock(*m_dbus_runner.get()));
+
+        GError* error {};
+        m_bus = g_bus_get_sync (G_BUS_TYPE_SESSION, nullptr, &error);
+        g_assert_no_error(error);
+        g_dbus_connection_set_exit_on_close(m_bus, FALSE);
+    }
+
+    void TearDown() override
+    {
+        g_clear_object(&m_bus);
+
+        super::TearDown();
     }
 
     void start_greeter_service(bool is_active)
-- 
cgit v1.2.3


From ce5234162fa0c534ff9abf3fce3d03f4b01e893e Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Fri, 22 Apr 2016 09:43:44 -0500
Subject: don't prompt when the greeter's not running yet: change greeter's
 payload from an 'is_active' bool to a three-value state of active, inactive,
 and unavailable

---
 src/greeter.cpp                        | 18 +++++++++---------
 src/greeter.h                          |  6 ++++--
 src/usb-manager.cpp                    | 14 ++++++++++----
 tests/integration/usb-manager-test.cpp |  4 ++--
 tests/unit/greeter-test.cpp            | 29 +++++++++++++++++------------
 tests/utils/gtest-print-helpers.h      | 18 ++++++++++++++++++
 tests/utils/mock-greeter.h             |  4 ++--
 7 files changed, 62 insertions(+), 31 deletions(-)
 create mode 100644 tests/utils/gtest-print-helpers.h

diff --git a/src/greeter.cpp b/src/greeter.cpp
index d41160f..317a829 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -41,9 +41,9 @@ public:
 
     ~Impl() =default;
 
-    core::Property<bool>& is_active()
+    core::Property<State>& state()
     {
-        return m_is_active;
+        return m_state;
     }
 
 private:
@@ -130,7 +130,7 @@ private:
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner.clear();
-        self->m_is_active.set(false);
+        self->m_state.set(State::UNAVAILABLE);
     }
 
     static void on_get_is_active_ready(
@@ -147,7 +147,7 @@ private:
         } else {
             GVariant* is_active {};
             g_variant_get_child(v, 0, "v", &is_active);
-            static_cast<Impl*>(gself)->m_is_active.set(g_variant_get_boolean(is_active));
+            static_cast<Impl*>(gself)->m_state.set(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE);
             g_clear_pointer(&is_active, g_variant_unref);
         }
         g_clear_pointer(&v, g_variant_unref);
@@ -175,12 +175,12 @@ private:
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
         {
             g_debug("%s is_active changed to %d", G_STRLOC, int(is_active));
-            self->m_is_active.set(is_active);
+            self->m_state.set(is_active ? State::ACTIVE : State::INACTIVE);
         }
         g_clear_pointer(&v, g_variant_unref);
     }
 
-    core::Property<bool> m_is_active {false};
+    core::Property<State> m_state {State::UNAVAILABLE};
     std::shared_ptr<GCancellable> m_cancellable;
     std::shared_ptr<GDBusConnection> m_bus;
     std::string m_owner;
@@ -201,8 +201,8 @@ UnityGreeter::UnityGreeter():
 
 UnityGreeter::~UnityGreeter() =default;
 
-core::Property<bool>&
-UnityGreeter::is_active()
+core::Property<Greeter::State>&
+UnityGreeter::state()
 {
-    return impl->is_active();
+    return impl->state();
 }
diff --git a/src/greeter.h b/src/greeter.h
index e084d25..cde1429 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -29,7 +29,9 @@ class Greeter
 public:
     Greeter();
     virtual ~Greeter();
-    virtual core::Property<bool>& is_active() =0;
+
+    enum class State { UNAVAILABLE, INACTIVE, ACTIVE };
+    virtual core::Property<State>& state() =0;
 };
 
 
@@ -38,7 +40,7 @@ class UnityGreeter: public Greeter
 public:
     UnityGreeter();
     virtual ~UnityGreeter();
-    core::Property<bool>& is_active() override;
+    core::Property<State>& state() override;
 
 protected:
     class Impl;
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 4d750c0..80b274b 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -49,7 +49,7 @@ public:
             restart();
         });
 
-        m_greeter->is_active().changed().connect([this](bool /*is_active*/) {
+        m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
             maybe_snap();
         });
 
@@ -92,9 +92,15 @@ private:
 
     void maybe_snap()
     {
-        // don't prompt in the greeter!
-        if (!m_req.public_key.empty() && !m_greeter->is_active().get())
-            snap();
+        // only prompt if there's something to prompt about
+        if (m_req.public_key.empty())
+            return;
+
+        // only prompt in an unlocked session
+        if (m_greeter->state().get() != Greeter::State::INACTIVE)
+            return;
+
+        snap();
     }
 
     void snap()
diff --git a/tests/integration/usb-manager-test.cpp b/tests/integration/usb-manager-test.cpp
index d62756f..7320640 100644
--- a/tests/integration/usb-manager-test.cpp
+++ b/tests/integration/usb-manager-test.cpp
@@ -206,7 +206,7 @@ TEST_F(UsbManagerFixture, Greeter)
     auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key});
 
     // set up a UsbManager to process the request
-    m_greeter->m_is_active.set(true);
+    m_greeter->m_state.set(Greeter::State::ACTIVE);
     auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter);
 
     // add a signal spy to listen to the notification daemon
@@ -219,7 +219,7 @@ TEST_F(UsbManagerFixture, Greeter)
     EXPECT_FALSE(notificationsSpy.wait(2000));
 
     // disable the greeter, the notification should appear
-    m_greeter->m_is_active.set(false);
+    m_greeter->m_state.set(Greeter::State::INACTIVE);
     wait_for_signals(notificationsSpy, 1);
     EXPECT_EQ("Notify", notificationsSpy.at(0).at(0));
     notificationsSpy.clear();
diff --git a/tests/unit/greeter-test.cpp b/tests/unit/greeter-test.cpp
index 72df3bc..bfa88e8 100644
--- a/tests/unit/greeter-test.cpp
+++ b/tests/unit/greeter-test.cpp
@@ -18,6 +18,7 @@
  */
 
 #include <tests/utils/qt-fixture.h>
+#include <tests/utils/gtest-print-helpers.h>
 
 #include <src/dbus-names.h>
 #include <src/greeter.h>
@@ -110,45 +111,49 @@ protected:
 
 TEST_F(GreeterFixture, ActiveServiceStartsBeforeWatcher)
 {
-    constexpr bool expected {true};
+    constexpr bool is_active {true};
+    constexpr Greeter::State expected {Greeter::State::ACTIVE};
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeActiveService)
 {
-    constexpr bool expected {true};
+    constexpr bool is_active {true};
+    constexpr Greeter::State expected {Greeter::State::ACTIVE};
 
     UnityGreeter greeter;
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
 TEST_F(GreeterFixture, InactiveServiceStartsBeforeWatcher)
 {
-    constexpr bool expected {false};
+    constexpr bool is_active {false};
+    constexpr Greeter::State expected {Greeter::State::INACTIVE};
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
     UnityGreeter greeter;
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
 TEST_F(GreeterFixture, WatcherStartsBeforeInactiveService)
 {
-    constexpr bool expected {false};
+    constexpr bool is_active {false};
+    constexpr Greeter::State expected {Greeter::State::INACTIVE};
 
     UnityGreeter greeter;
 
-    start_greeter_service(expected);
+    start_greeter_service(is_active);
 
-    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.is_active());
+    ASSERT_PROPERTY_EQ_EVENTUALLY(expected, greeter.state());
 }
 
diff --git a/tests/utils/gtest-print-helpers.h b/tests/utils/gtest-print-helpers.h
new file mode 100644
index 0000000..75ee13b
--- /dev/null
+++ b/tests/utils/gtest-print-helpers.h
@@ -0,0 +1,18 @@
+
+#pragma once
+
+#include <src/greeter.h>
+
+inline void PrintTo(const Greeter::State& state, std::ostream* os) {
+    switch(state) {
+        case Greeter::State::ACTIVE:   *os << "Active"; break;
+        case Greeter::State::INACTIVE: *os << "Inactive"; break;
+        default:                       *os << "Unavailable"; break;
+    }
+}
+
+std::ostream& operator<<(std::ostream& os, const Greeter::State& state) {
+    PrintTo(state, &os);
+    return os;
+}
+
diff --git a/tests/utils/mock-greeter.h b/tests/utils/mock-greeter.h
index 5ac85a0..5015087 100644
--- a/tests/utils/mock-greeter.h
+++ b/tests/utils/mock-greeter.h
@@ -26,7 +26,7 @@ class MockGreeter: public Greeter
 public:
     MockGreeter() =default;
     virtual ~MockGreeter() =default;
-    core::Property<bool>& is_active() override {return m_is_active;}
-    core::Property<bool> m_is_active {false};
+    core::Property<Greeter::State>& state() override {return m_state;}
+    core::Property<Greeter::State> m_state {State::INACTIVE};
 };
 
-- 
cgit v1.2.3


From 0f5534d22903f73e2d33ea2e29efb2307463bfa4 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Sat, 23 Apr 2016 12:29:45 +0200
Subject: in tests/utils/adbd-server.h, fix a timing bug in the test
 scaffolding by creating the adb socket in AdbdServer's ctor instead of in its
 worker thread.

---
 tests/utils/adbd-server.h | 20 +++++++++-----------
 1 file changed, 9 insertions(+), 11 deletions(-)

diff --git a/tests/utils/adbd-server.h b/tests/utils/adbd-server.h
index b574622..585380f 100644
--- a/tests/utils/adbd-server.h
+++ b/tests/utils/adbd-server.h
@@ -38,7 +38,7 @@ public:
     GAdbdServer(const std::string& socket_path,
                 const std::vector<std::string>& requests):
         m_requests{requests},
-        m_socket_path{socket_path},
+        m_server_socket{create_server_socket(socket_path)},
         m_cancellable{g_cancellable_new()},
         m_worker_thread{&GAdbdServer::worker_func, this}
     {
@@ -50,6 +50,7 @@ public:
         g_cancellable_cancel(m_cancellable);
         m_worker_thread.join();
         g_clear_object(&m_cancellable);
+        g_clear_object(&m_server_socket);
     }
 
     const std::vector<std::string> m_requests;
@@ -59,18 +60,14 @@ private:
 
     void worker_func() // runs in worker thread
     {
-        auto server_socket = create_server_socket(m_socket_path);
         auto requests = m_requests;
 
-        GError* error {};
-        g_socket_listen (server_socket, &error);
-        g_assert_no_error (error);
-
         while (!g_cancellable_is_cancelled(m_cancellable) && !requests.empty())
         {
             // wait for a client connection
             g_message("GAdbdServer::Impl::worker_func() calling g_socket_accept()");
-            auto client_socket = g_socket_accept(server_socket, m_cancellable, &error);
+            GError* error {};
+            auto client_socket = g_socket_accept(m_server_socket, m_cancellable, &error);
             if (error != nullptr) {
                 if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
                     g_message("GAdbdServer: Error accepting socket connection: %s", error->message);
@@ -121,8 +118,6 @@ private:
             // cleanup
             g_clear_object(&client_socket);
         }
-
-        g_clear_object(&server_socket);
     }
 
     // bind to a local domain socket
@@ -139,11 +134,14 @@ private:
         g_assert_no_error (error);
         g_clear_object (&address);
 
+        g_socket_listen (socket, &error);
+        g_assert_no_error (error);
+
         return socket;
     }
 
-    const std::string m_socket_path;
-    GCancellable* m_cancellable = nullptr;
+    GSocket* m_server_socket {};
+    GCancellable* m_cancellable {};
     std::thread m_worker_thread;
 };
 
-- 
cgit v1.2.3


From 7d2fac8f46110a84d8396251cd0f38351b92f6ce Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Mon, 25 Apr 2016 11:05:22 +0200
Subject: add tracer log messages

---
 src/greeter.cpp     | 24 ++++++++++++++++--------
 src/greeter.h       |  4 ++++
 src/usb-manager.cpp |  6 +++++-
 3 files changed, 25 insertions(+), 9 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index 317a829..ed210b0 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -48,6 +48,12 @@ public:
 
 private:
 
+    void set_state(const State& state)
+    {
+g_message("%s setting state to %s", G_STRLOC, state_str(state));
+        m_state.set(state);
+    }
+
     static void on_bus_ready(
         GObject* /*source*/,
         GAsyncResult* res,
@@ -99,10 +105,11 @@ private:
 
     static void on_greeter_appeared(
         GDBusConnection* bus,
-        const char* /*name*/,
+        const char* name,
         const char* name_owner,
         gpointer gself)
     {
+g_message("%s %s appared on bus, owned by %s", G_STRLOC, name, name_owner);
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner = name_owner;
@@ -124,13 +131,14 @@ private:
 
     static void on_greeter_vanished(
         GDBusConnection* /*bus*/,
-        const char* /*name*/,
+        const char* name,
         gpointer gself)
     {
+g_message("%s %s disappeared from bus", G_STRLOC, name);
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner.clear();
-        self->m_state.set(State::UNAVAILABLE);
+        self->set_state(State::UNAVAILABLE);
     }
 
     static void on_get_is_active_ready(
@@ -138,6 +146,7 @@ private:
         GAsyncResult* res, 
         gpointer gself)
     {
+g_message("%s", G_STRLOC);
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
@@ -145,9 +154,10 @@ private:
                 g_warning("Greeter: Error getting IsActive property: %s", error->message);
             g_clear_error(&error);
         } else {
+g_message("%s got '%s'", G_STRLOC, g_variant_print(v, true));
             GVariant* is_active {};
             g_variant_get_child(v, 0, "v", &is_active);
-            static_cast<Impl*>(gself)->m_state.set(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE);
+            static_cast<Impl*>(gself)->set_state(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE);
             g_clear_pointer(&is_active, g_variant_unref);
         }
         g_clear_pointer(&v, g_variant_unref);
@@ -163,6 +173,7 @@ private:
         gpointer gself)
     {
         auto self = static_cast<Impl*>(gself);
+g_message("%s on_properties_changed got %s", G_STRLOC, g_variant_print(parameters, true));
 
         g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str()));
         g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH));
@@ -173,10 +184,7 @@ private:
         auto v = g_variant_get_child_value(parameters, 1);
         gboolean is_active {};
         if (g_variant_lookup(v, "IsActive", "b", &is_active))
-        {
-            g_debug("%s is_active changed to %d", G_STRLOC, int(is_active));
-            self->m_state.set(is_active ? State::ACTIVE : State::INACTIVE);
-        }
+            self->set_state(is_active ? State::ACTIVE : State::INACTIVE);
         g_clear_pointer(&v, g_variant_unref);
     }
 
diff --git a/src/greeter.h b/src/greeter.h
index cde1429..f3012f6 100644
--- a/src/greeter.h
+++ b/src/greeter.h
@@ -31,6 +31,10 @@ public:
     virtual ~Greeter();
 
     enum class State { UNAVAILABLE, INACTIVE, ACTIVE };
+static inline const char* state_str(const State& state) {
+    static constexpr char const * state_str[] = { "Unavailable", "Inactive", "Active" };
+    return state_str[int(state)];
+}
     virtual core::Property<State>& state() =0;
 };
 
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 80b274b..9f67720 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -50,6 +50,7 @@ public:
         });
 
         m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
+g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC);
             maybe_snap();
         });
 
@@ -83,7 +84,7 @@ private:
         m_adbd_client.reset(new GAdbdClient{m_socket_path});
         m_adbd_client->on_pk_request().connect(
             [this](const AdbdClient::PKRequest& req) {
-                g_debug("%s got pk request: %s", G_STRLOC, req.fingerprint.c_str());
+                g_message("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str());
                 m_req = req;
                 maybe_snap();
             }
@@ -92,14 +93,17 @@ private:
 
     void maybe_snap()
     {
+g_message("%s m_req.public_key [%s]", G_STRLOC, m_req.public_key.c_str());
         // only prompt if there's something to prompt about
         if (m_req.public_key.empty())
             return;
 
+g_message("%s m_greeter->state() %s", G_STRLOC, Greeter::state_str(m_greeter->state().get()));
         // only prompt in an unlocked session
         if (m_greeter->state().get() != Greeter::State::INACTIVE)
             return;
 
+g_message("%s calling snap!", G_STRLOC);
         snap();
     }
 
-- 
cgit v1.2.3


From f8a8646b5cc0e9b59a5f953dcd8129c93d162915 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 09:56:53 +0200
Subject: add tracers for the notification warning

---
 src/usb-manager.cpp | 11 +++++++++--
 src/usb-snap.cpp    |  6 ++++++
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 9f67720..ce546fe 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -50,6 +50,8 @@ public:
         });
 
         m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
+g_message("%s clearing old snap, if any", G_STRLOC);
+            clear_snap();
 g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC);
             maybe_snap();
         });
@@ -67,11 +69,16 @@ g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC);
 
 private:
 
-    void clear()
+    void clear_snap()
     {
-        // clear out old state
         m_snap_connections.clear();
         m_snap.reset();
+    }
+
+    void clear()
+    {
+        // clear out old state
+        clear_snap();
         m_req = decltype(m_req)();
         m_adbd_client.reset();
     }
diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp
index ba964fb..2b5424d 100644
--- a/src/usb-snap.cpp
+++ b/src/usb-snap.cpp
@@ -119,6 +119,7 @@ private:
         g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-snap-decisions", g_variant_new_string("true"));
         g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-private-affirmative-tint", g_variant_new_string("true"));
 
+g_message("%s calling notification notify", G_STRLOC);
         auto args = g_variant_new("(susssasa{sv}i)",
                                   "",
                                   uint32_t(0),
@@ -153,6 +154,7 @@ private:
                 g_warning("UsbSnap: Error calling Notify: %s", error->message);
             g_clear_error(&error);
         } else {
+g_message("%s on_notify reply %s", G_STRLOC, g_variant_print(reply, true));
             uint32_t id {};
             g_variant_get(reply, "(u)", &id);
             static_cast<Impl*>(gself)->on_notify_reply(id);
@@ -162,6 +164,7 @@ private:
 
     void on_notify_reply(uint32_t id)
     {
+g_message("%s setting m_notification_id to %d", G_STRLOC, int(id));
         m_notification_id = id;
     }
 
@@ -177,6 +180,7 @@ private:
         g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Notify::INTERFACE));
 
         auto self = static_cast<Impl*>(gself);
+g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_variant_print(parameters, true));
 
         if (!g_strcmp0(signal_name, DBusNames::Notify::ActionInvoked::NAME))
         {
@@ -212,9 +216,11 @@ private:
 
     void on_notification_closed(uint32_t close_reason)
     {
+g_message("%s closed with reason %d", G_STRLOC, int(close_reason));
         if (close_reason == DBusNames::Notify::NotificationClosed::Reason::EXPIRED)
             m_on_user_response(AdbdClient::PKResponse::DENY, false);
 
+g_message("%s setting m_notification_id to 0", G_STRLOC);
         m_notification_id = 0;
     }
 
-- 
cgit v1.2.3


From 2e88906dc5ced1c5f601cef2514d8e559246e0d5 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 10:16:39 +0200
Subject: silence clang warning in PrintTo gtest helper

---
 tests/utils/gtest-print-helpers.h | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/tests/utils/gtest-print-helpers.h b/tests/utils/gtest-print-helpers.h
index 75ee13b..6dc1217 100644
--- a/tests/utils/gtest-print-helpers.h
+++ b/tests/utils/gtest-print-helpers.h
@@ -5,9 +5,9 @@
 
 inline void PrintTo(const Greeter::State& state, std::ostream* os) {
     switch(state) {
-        case Greeter::State::ACTIVE:   *os << "Active"; break;
-        case Greeter::State::INACTIVE: *os << "Inactive"; break;
-        default:                       *os << "Unavailable"; break;
+        case Greeter::State::ACTIVE:      *os << "Active"; break;
+        case Greeter::State::INACTIVE:    *os << "Inactive"; break;
+        case Greeter::State::UNAVAILABLE: *os << "Unavailable"; break;
     }
 }
 
-- 
cgit v1.2.3


From a277302b1c1c2568c456abcb3432eb8f8ead0925 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 10:22:00 +0200
Subject: choosing an action in the snap decision closes the notification, so
 when that happens clear our notification_id s.t. our dtor doesn't try to
 remove a notification that no longer exists

---
 src/usb-snap.cpp | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp
index 2b5424d..629898b 100644
--- a/src/usb-snap.cpp
+++ b/src/usb-snap.cpp
@@ -212,6 +212,8 @@ g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_varian
         const bool remember_this_choice = response == AdbdClient::PKResponse::ALLOW;
 
         m_on_user_response(response, remember_this_choice);
+g_message("%s clearing m_notification_id", G_STRLOC);
+        m_notification_id = 0;
     }
 
     void on_notification_closed(uint32_t close_reason)
@@ -220,7 +222,7 @@ g_message("%s closed with reason %d", G_STRLOC, int(close_reason));
         if (close_reason == DBusNames::Notify::NotificationClosed::Reason::EXPIRED)
             m_on_user_response(AdbdClient::PKResponse::DENY, false);
 
-g_message("%s setting m_notification_id to 0", G_STRLOC);
+g_message("%s clearing m_notification_id", G_STRLOC);
         m_notification_id = 0;
     }
 
-- 
cgit v1.2.3


From fcf75a545e25baa8777a310c648846537590fa2b Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 10:33:05 +0200
Subject: silence 'no previous prototype' warning from clang

---
 tests/utils/gtest-print-helpers.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/utils/gtest-print-helpers.h b/tests/utils/gtest-print-helpers.h
index 6dc1217..60f42b4 100644
--- a/tests/utils/gtest-print-helpers.h
+++ b/tests/utils/gtest-print-helpers.h
@@ -11,7 +11,7 @@ inline void PrintTo(const Greeter::State& state, std::ostream* os) {
     }
 }
 
-std::ostream& operator<<(std::ostream& os, const Greeter::State& state) {
+inline std::ostream& operator<<(std::ostream& os, const Greeter::State& state) {
     PrintTo(state, &os);
     return os;
 }
-- 
cgit v1.2.3


From ccfe9ef48c6e072f5d0ec9918de90213a2f4a7ee Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 10:40:05 +0200
Subject: sync tests to r33 changes

---
 tests/unit/usb-snap-test.cpp | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

diff --git a/tests/unit/usb-snap-test.cpp b/tests/unit/usb-snap-test.cpp
index 3b778dd..6cfbbd9 100644
--- a/tests/unit/usb-snap-test.cpp
+++ b/tests/unit/usb-snap-test.cpp
@@ -130,14 +130,9 @@ TEST_F(UsbSnapFixture, TestRoundTrip)
         EXPECT_TRUE(user_response_set);
         ASSERT_EQ(test.expected_response, user_response);
 
-        // confirm that the snap dtor cleans up the notification
+        // confirm that the snap dtor doesn't try to close
+        // the notification that's already been closed by user choice
         snap.reset();
-        wait_for_signals(notificationsSpy, 1);
-        {
-            QVariantList const& call(notificationsSpy.at(0));
-            EXPECT_EQ("CloseNotification", call.at(0));
-            QVariantList const& args(call.at(1).toList());
-            EXPECT_EQ(id, args.at(0));
-        }
+        EXPECT_FALSE(notificationsSpy.wait(1000));
     }
 }
-- 
cgit v1.2.3


From 020e49163abd558ae095ab9a00b6b69b4fdaef2a Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 10:43:08 +0200
Subject: remove the temporary tracers

---
 src/greeter.cpp     | 6 ------
 src/usb-manager.cpp | 7 +------
 src/usb-snap.cpp    | 7 -------
 3 files changed, 1 insertion(+), 19 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index ed210b0..962a01d 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -50,7 +50,6 @@ private:
 
     void set_state(const State& state)
     {
-g_message("%s setting state to %s", G_STRLOC, state_str(state));
         m_state.set(state);
     }
 
@@ -109,7 +108,6 @@ g_message("%s setting state to %s", G_STRLOC, state_str(state));
         const char* name_owner,
         gpointer gself)
     {
-g_message("%s %s appared on bus, owned by %s", G_STRLOC, name, name_owner);
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner = name_owner;
@@ -134,7 +132,6 @@ g_message("%s %s appared on bus, owned by %s", G_STRLOC, name, name_owner);
         const char* name,
         gpointer gself)
     {
-g_message("%s %s disappeared from bus", G_STRLOC, name);
         auto self = static_cast<Impl*>(gself);
 
         self->m_owner.clear();
@@ -146,7 +143,6 @@ g_message("%s %s disappeared from bus", G_STRLOC, name);
         GAsyncResult* res, 
         gpointer gself)
     {
-g_message("%s", G_STRLOC);
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
@@ -154,7 +150,6 @@ g_message("%s", G_STRLOC);
                 g_warning("Greeter: Error getting IsActive property: %s", error->message);
             g_clear_error(&error);
         } else {
-g_message("%s got '%s'", G_STRLOC, g_variant_print(v, true));
             GVariant* is_active {};
             g_variant_get_child(v, 0, "v", &is_active);
             static_cast<Impl*>(gself)->set_state(g_variant_get_boolean(is_active) ? State::ACTIVE : State::INACTIVE);
@@ -173,7 +168,6 @@ g_message("%s got '%s'", G_STRLOC, g_variant_print(v, true));
         gpointer gself)
     {
         auto self = static_cast<Impl*>(gself);
-g_message("%s on_properties_changed got %s", G_STRLOC, g_variant_print(parameters, true));
 
         g_return_if_fail(!g_strcmp0(sender_name, self->m_owner.c_str()));
         g_return_if_fail(!g_strcmp0(object_path, DBusNames::UnityGreeter::PATH));
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index ce546fe..cf3330d 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -50,9 +50,7 @@ public:
         });
 
         m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
-g_message("%s clearing old snap, if any", G_STRLOC);
             clear_snap();
-g_message("%s greeter state changed; calling maybe_snap()", G_STRLOC);
             maybe_snap();
         });
 
@@ -91,7 +89,7 @@ private:
         m_adbd_client.reset(new GAdbdClient{m_socket_path});
         m_adbd_client->on_pk_request().connect(
             [this](const AdbdClient::PKRequest& req) {
-                g_message("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str());
+                g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str());
                 m_req = req;
                 maybe_snap();
             }
@@ -100,17 +98,14 @@ private:
 
     void maybe_snap()
     {
-g_message("%s m_req.public_key [%s]", G_STRLOC, m_req.public_key.c_str());
         // only prompt if there's something to prompt about
         if (m_req.public_key.empty())
             return;
 
-g_message("%s m_greeter->state() %s", G_STRLOC, Greeter::state_str(m_greeter->state().get()));
         // only prompt in an unlocked session
         if (m_greeter->state().get() != Greeter::State::INACTIVE)
             return;
 
-g_message("%s calling snap!", G_STRLOC);
         snap();
     }
 
diff --git a/src/usb-snap.cpp b/src/usb-snap.cpp
index 629898b..53db6c4 100644
--- a/src/usb-snap.cpp
+++ b/src/usb-snap.cpp
@@ -119,7 +119,6 @@ private:
         g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-snap-decisions", g_variant_new_string("true"));
         g_variant_builder_add(&hints_builder, "{sv}", "x-canonical-private-affirmative-tint", g_variant_new_string("true"));
 
-g_message("%s calling notification notify", G_STRLOC);
         auto args = g_variant_new("(susssasa{sv}i)",
                                   "",
                                   uint32_t(0),
@@ -154,7 +153,6 @@ g_message("%s calling notification notify", G_STRLOC);
                 g_warning("UsbSnap: Error calling Notify: %s", error->message);
             g_clear_error(&error);
         } else {
-g_message("%s on_notify reply %s", G_STRLOC, g_variant_print(reply, true));
             uint32_t id {};
             g_variant_get(reply, "(u)", &id);
             static_cast<Impl*>(gself)->on_notify_reply(id);
@@ -164,7 +162,6 @@ g_message("%s on_notify reply %s", G_STRLOC, g_variant_print(reply, true));
 
     void on_notify_reply(uint32_t id)
     {
-g_message("%s setting m_notification_id to %d", G_STRLOC, int(id));
         m_notification_id = id;
     }
 
@@ -180,7 +177,6 @@ g_message("%s setting m_notification_id to %d", G_STRLOC, int(id));
         g_return_if_fail(!g_strcmp0(interface_name, DBusNames::Notify::INTERFACE));
 
         auto self = static_cast<Impl*>(gself);
-g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_variant_print(parameters, true));
 
         if (!g_strcmp0(signal_name, DBusNames::Notify::ActionInvoked::NAME))
         {
@@ -212,17 +208,14 @@ g_message("%s got signal %s with parameters %s", G_STRLOC, signal_name, g_varian
         const bool remember_this_choice = response == AdbdClient::PKResponse::ALLOW;
 
         m_on_user_response(response, remember_this_choice);
-g_message("%s clearing m_notification_id", G_STRLOC);
         m_notification_id = 0;
     }
 
     void on_notification_closed(uint32_t close_reason)
     {
-g_message("%s closed with reason %d", G_STRLOC, int(close_reason));
         if (close_reason == DBusNames::Notify::NotificationClosed::Reason::EXPIRED)
             m_on_user_response(AdbdClient::PKResponse::DENY, false);
 
-g_message("%s clearing m_notification_id", G_STRLOC);
         m_notification_id = 0;
     }
 
-- 
cgit v1.2.3


From 6dfd01c5db87f659beb5c53c0c3ea1f57ab0701b Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 10:45:24 +0200
Subject: fix two trivial 'unused parameter' warnings in glib callbacks

---
 src/greeter.cpp | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/greeter.cpp b/src/greeter.cpp
index 962a01d..9bd5db0 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -104,7 +104,7 @@ private:
 
     static void on_greeter_appeared(
         GDBusConnection* bus,
-        const char* name,
+        const char* /*name*/,
         const char* name_owner,
         gpointer gself)
     {
@@ -129,7 +129,7 @@ private:
 
     static void on_greeter_vanished(
         GDBusConnection* /*bus*/,
-        const char* name,
+        const char* /*name*/,
         gpointer gself)
     {
         auto self = static_cast<Impl*>(gself);
-- 
cgit v1.2.3


From dfa098f6fcd7203a8975ec891bc1d9f7f9e735ca Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 27 Apr 2016 13:30:59 +0200
Subject: increase timeout interval on usb manager integration test

---
 tests/integration/usb-manager-test.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/integration/usb-manager-test.cpp b/tests/integration/usb-manager-test.cpp
index 7320640..6532829 100644
--- a/tests/integration/usb-manager-test.cpp
+++ b/tests/integration/usb-manager-test.cpp
@@ -143,7 +143,7 @@ TEST_F(UsbManagerFixture, Allow)
     );
 
     // confirm that the AdbdServer got the right response
-    wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 2000);
+    wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 5000);
     ASSERT_EQ(1, adbd_server->m_responses.size());
     EXPECT_EQ("OK", adbd_server->m_responses.front());
 
-- 
cgit v1.2.3


From d7a0cac8ebd836172ad6e4b9298ae92b3f9d8de9 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Fri, 29 Apr 2016 11:51:22 +0200
Subject: don't clear the snap decision when the greeter changes state

---
 src/usb-manager.cpp | 1 -
 1 file changed, 1 deletion(-)

diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index cf3330d..9c45a56 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -50,7 +50,6 @@ public:
         });
 
         m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
-            clear_snap();
             maybe_snap();
         });
 
-- 
cgit v1.2.3


From c66c33c4c1c70b197941f8c486196f3a56abd0e3 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Tue, 3 May 2016 18:07:48 -0500
Subject: always send a response to adb even in edge cases; e.g. send an
 auto-NO if the usb connection is broken while the prompt is up

---
 src/usb-manager.cpp | 92 +++++++++++++++++++++++++++--------------------------
 1 file changed, 47 insertions(+), 45 deletions(-)

diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 9c45a56..65617c5 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -46,59 +46,54 @@ public:
         m_greeter{greeter}
     {
         m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) {
-            restart();
+            m_req.reset();
         });
 
-        m_greeter->state().changed().connect([this](Greeter::State /*state*/) {
-            maybe_snap();
+        m_greeter->state().changed().connect([this](const Greeter::State& state) {
+            if (state == Greeter::State::INACTIVE)
+                maybe_snap();
+            else
+                stop_snap();
         });
 
-        restart();
+        // create a new adbd client
+        m_adbd_client.reset(new GAdbdClient{m_socket_path});
+        m_adbd_client->on_pk_request().connect(
+            [this](const AdbdClient::PKRequest& req) {
+                g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str());
+
+                m_response = AdbdClient::PKResponse::DENY; // set the fallback response
+                m_req.reset(
+                    new AdbdClient::PKRequest(req),
+                    [this](AdbdClient::PKRequest* r) {
+                        stop_snap();
+                        r->respond(m_response);
+                        delete r;
+                    }
+                );
+                maybe_snap();
+            }
+        );
     }
 
     ~Impl()
     {
-        if (m_restart_idle_tag)
-            g_source_remove(m_restart_idle_tag);
-
-        clear();
+        if (m_request_complete_idle_tag)
+            g_source_remove(m_request_complete_idle_tag);
     }
 
 private:
 
-    void clear_snap()
+    void stop_snap()
     {
         m_snap_connections.clear();
         m_snap.reset();
     }
 
-    void clear()
-    {
-        // clear out old state
-        clear_snap();
-        m_req = decltype(m_req)();
-        m_adbd_client.reset();
-    }
-
-    void restart()
-    {
-        clear();
-
-        // set a new client
-        m_adbd_client.reset(new GAdbdClient{m_socket_path});
-        m_adbd_client->on_pk_request().connect(
-            [this](const AdbdClient::PKRequest& req) {
-                g_debug("%s got pk request: %s, calling maybe_snap()", G_STRLOC, req.fingerprint.c_str());
-                m_req = req;
-                maybe_snap();
-            }
-        );
-    }
-
     void maybe_snap()
     {
         // only prompt if there's something to prompt about
-        if (m_req.public_key.empty())
+        if (!m_req)
             return;
 
         // only prompt in an unlocked session
@@ -110,19 +105,25 @@ private:
 
     void snap()
     {
-        m_snap = std::make_shared<UsbSnap>(m_req.fingerprint);
+        m_snap = std::make_shared<UsbSnap>(m_req->fingerprint);
         m_snap_connections.insert((*m_snap).on_user_response().connect(
             [this](AdbdClient::PKResponse response, bool remember_choice){
-                g_debug("%s user responded! response %d, remember %d", G_STRLOC, int(response), int(remember_choice));
-                m_req.respond(response);
+
                 if (remember_choice && (response == AdbdClient::PKResponse::ALLOW))
-                    write_public_key(m_req.public_key);
-                m_restart_idle_tag = g_idle_add([](gpointer gself){
-                    auto self = static_cast<Impl*>(gself);
-                    self->m_restart_idle_tag = 0;
-                    self->restart();
-                    return G_SOURCE_REMOVE;
-                }, this);
+                    write_public_key(m_req->public_key);
+
+                m_response = response;
+
+                // defer finishing the request into an idle func because
+                // ScopedConnections can't be destroyed inside their callbacks
+                if (m_request_complete_idle_tag == 0) {
+                    m_request_complete_idle_tag = g_idle_add([](gpointer gself){
+                        auto self = static_cast<Impl*>(gself);
+                        self->m_request_complete_idle_tag = 0;
+                        self->m_req.reset();
+                        return G_SOURCE_REMOVE;
+                    }, this);
+                }
             }
         ));
     }
@@ -163,12 +164,13 @@ private:
     const std::shared_ptr<UsbMonitor> m_usb_monitor;
     const std::shared_ptr<Greeter> m_greeter;
  
-    unsigned int m_restart_idle_tag {};
+    unsigned int m_request_complete_idle_tag {};
 
     std::shared_ptr<GAdbdClient> m_adbd_client;
-    AdbdClient::PKRequest m_req;
     std::shared_ptr<UsbSnap> m_snap;
     std::set<core::ScopedConnection> m_snap_connections;
+    AdbdClient::PKResponse m_response {AdbdClient::PKResponse::DENY};
+    std::shared_ptr<AdbdClient::PKRequest> m_req;
 };
 
 /***
-- 
cgit v1.2.3


From 403fd0a388e565daf195729d9211ff03c23c93a0 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Tue, 3 May 2016 18:11:00 -0500
Subject: sync UsbManagerFixture::USBDisconnectedDuringPrompt to r40, to expect
 a 'no' each time there's a usb disconnect

---
 tests/integration/usb-manager-test.cpp | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/tests/integration/usb-manager-test.cpp b/tests/integration/usb-manager-test.cpp
index 6532829..805ba6e 100644
--- a/tests/integration/usb-manager-test.cpp
+++ b/tests/integration/usb-manager-test.cpp
@@ -163,13 +163,16 @@ TEST_F(UsbManagerFixture, USBDisconnectedDuringPrompt)
     const std::shared_ptr<std::string> public_keys_path {new std::string{*m_tmpdir+"/adb_keys"}, file_deleter};
 
     // start a mock AdbdServer ready to submit a request
+    const size_t N_TESTS {3};
     const std::string public_key {"public_key"};
-    auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, std::vector<std::string>{"PK"+public_key});
+    const std::vector<std::string> requests(N_TESTS, "PK"+public_key);
+    const std::vector<std::string> expected_responses(N_TESTS, "NO");
+    auto adbd_server = std::make_shared<GAdbdServer>(*socket_path, requests);
 
     // set up a UsbManager to process the request
     auto usb_manager = std::make_shared<UsbManager>(*socket_path, *public_keys_path, m_usb_monitor, m_greeter);
 
-    for (int i=0; i<3; i++)
+    for (std::remove_const<decltype(N_TESTS)>::type i=0; i<N_TESTS; ++i)
     {
         // add a signal spy to listen to the notification daemon
         QSignalSpy notificationsSpy(
@@ -194,6 +197,9 @@ TEST_F(UsbManagerFixture, USBDisconnectedDuringPrompt)
         EXPECT_EQ("CloseNotification", notificationsSpy.at(0).at(0));
         notificationsSpy.clear();
     }
+
+    EXPECT_TRUE(wait_for([adbd_server, N_TESTS](){return adbd_server->m_responses.size() == N_TESTS;}, 5000));
+    EXPECT_EQ(expected_responses, adbd_server->m_responses);
 }
 
 TEST_F(UsbManagerFixture, Greeter)
-- 
cgit v1.2.3

-- 
cgit v1.2.3


From 996f5b96f12e77e922114734ed8524ce7d5f62bc Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 21 Sep 2016 17:18:13 -0500
Subject: in PKIdleData, take the std::string argument as a const& to make
 cppcheck happy on yakkety

---
 src/adbd-client.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 400c7c9..e436a43 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -66,7 +66,7 @@ private:
         GCancellable* cancellable = nullptr;
         const std::string public_key;
 
-        PKIdleData(Impl* self_, GCancellable* cancellable_, std::string public_key_):
+        PKIdleData(Impl* self_, GCancellable* cancellable_, const std::string& public_key_):
             self(self_),
             cancellable(G_CANCELLABLE(g_object_ref(cancellable_))),
             public_key(public_key_) {}
-- 
cgit v1.2.3


From 3b7185fdf0d21897fd874d337d57379f911cc63c Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 21 Sep 2016 19:05:02 -0500
Subject: in adbd-client's dtor, check the worker thread's joinable() state
 before calling join() on it

---
 src/adbd-client.cpp | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index e436a43..85389f4 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -48,7 +48,8 @@ public:
         g_cancellable_cancel(m_cancellable);
         m_pkresponse_cv.notify_one();
         m_sleep_cv.notify_one();
-        m_worker_thread.join();
+        if (m_worker_thread.joinable())
+            m_worker_thread.join();
         g_clear_object(&m_cancellable);
     }
 
@@ -104,6 +105,8 @@ private:
 
     void on_public_key_response(PKResponse response)
     {
+        g_debug("%s got response %d", G_STRLOC, int(response));
+
         // set m_pkresponse and wake up the waiting worker thread
         std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
         m_pkresponse = response;
@@ -141,11 +144,13 @@ private:
                     std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
                     m_pkresponse_ready = false;
                     pass_public_key_to_main_thread(public_key);
+                    g_debug("%s thread %p waiting", G_STRLOC, g_thread_self());
                     m_pkresponse_cv.wait(lk, [this](){
                         return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
                     });
                     response = m_pkresponse;
-                    g_debug("%s got response '%d', is-cancelled %d", G_STRLOC,
+                    g_debug("%s thread %p got response '%d', is-cancelled %d", G_STRLOC,
+                            g_thread_self(),
                             int(response),
                             int(g_cancellable_is_cancelled(m_cancellable)));
                 }
-- 
cgit v1.2.3


From b74f5f6a37edaab28156e551462407096cfbfcbd Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 21 Sep 2016 20:35:21 -0500
Subject: adbd-client's std::condition_variable::wait() call seems to be the
 system_error culprit, so wrapping in try-catch to be cautious

---
 src/adbd-client.cpp | 17 +++++++++++------
 src/usb-manager.cpp |  2 +-
 2 files changed, 12 insertions(+), 7 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 85389f4..83b15ac 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -124,11 +124,11 @@ private:
 
         while (!g_cancellable_is_cancelled(m_cancellable))
         {
-            g_debug("%s creating a client socket to '%s'", G_STRLOC, socket_path.c_str());
+            g_debug("%s thread %p creating a client socket to '%s'", G_STRLOC, g_thread_self(), socket_path.c_str());
             auto socket = create_client_socket(socket_path);
             bool got_valid_req = false;
 
-            g_debug("%s calling read_request", G_STRLOC);
+            g_debug("%s thread %p calling read_request", g_thread_self(), G_STRLOC);
             std::string reqstr;
             if (socket != nullptr)
                 reqstr = read_request(socket);
@@ -138,16 +138,21 @@ private:
             if (reqstr.substr(0,2) == "PK") {
                 PKResponse response = PKResponse::DENY;
                 const auto public_key = reqstr.substr(2);
-                g_debug("%s got pk [%s]", G_STRLOC, public_key.c_str());
+                g_debug("%s thread %p got pk [%s]", G_STRLOC, g_thread_self(), public_key.c_str());
                 if (!public_key.empty()) {
                     got_valid_req = true;
                     std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
                     m_pkresponse_ready = false;
+                    m_pkresponse = AdbdClient::PKResponse::DENY;
                     pass_public_key_to_main_thread(public_key);
                     g_debug("%s thread %p waiting", G_STRLOC, g_thread_self());
-                    m_pkresponse_cv.wait(lk, [this](){
-                        return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
-                    });
+                    try {
+                        m_pkresponse_cv.wait(lk, [this](){
+                            return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
+                        });
+                    } catch (std::system_error& e) {
+                        g_critical("%s thread %p unable to wait for response because of unexpected error '%s'", G_STRLOC, g_thread_self(), e.what());
+                    }
                     response = m_pkresponse;
                     g_debug("%s thread %p got response '%d', is-cancelled %d", G_STRLOC,
                             g_thread_self(),
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 4d750c0..aac289a 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -102,7 +102,7 @@ private:
         m_snap = std::make_shared<UsbSnap>(m_req.fingerprint);
         m_snap_connections.insert((*m_snap).on_user_response().connect(
             [this](AdbdClient::PKResponse response, bool remember_choice){
-                g_debug("%s user responded! response %d, remember %d", G_STRLOC, int(response), int(remember_choice));
+                g_debug("%s thread %p user responded! response %d, remember %d", G_STRLOC, g_thread_self(), int(response), int(remember_choice));
                 m_req.respond(response);
                 if (remember_choice && (response == AdbdClient::PKResponse::ALLOW))
                     write_public_key(m_req.public_key);
-- 
cgit v1.2.3


From c58d22061d944dc8b9b0b072082665c9f10877ac Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 09:10:48 -0500
Subject: add a plethora of log statements to help figure out what's causing
 the silo test failures

---
 src/adbd-client.cpp | 93 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 src/usb-manager.cpp | 42 ++++++++++++++++++++++++
 2 files changed, 135 insertions(+)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 83b15ac..1539982 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -39,22 +39,31 @@ public:
         m_cancellable{g_cancellable_new()},
         m_worker_thread{&Impl::worker_func, this}
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     ~Impl()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // tell the worker thread to stop whatever it's doing and exit.
         g_debug("%s Client::Impl dtor, cancelling m_cancellable", G_STRLOC);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_cancellable_cancel(m_cancellable);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_sleep_cv.notify_one();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (m_worker_thread.joinable())
             m_worker_thread.join();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_object(&m_cancellable);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     core::Signal<const PKRequest&>& on_pk_request()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         return m_on_pk_request;
     }
 
@@ -78,6 +87,7 @@ private:
 
     void pass_public_key_to_main_thread(const std::string& public_key)
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_idle_add_full(G_PRIORITY_DEFAULT_IDLE,
                         on_public_key_request_static,
                         new PKIdleData{this, m_cancellable, public_key},
@@ -88,30 +98,47 @@ private:
     {
         /* NB: It's possible (though unlikely) that data.self was destroyed
            while this callback was pending, so we must check is-cancelled FIRST */
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         auto data = static_cast<PKIdleData*>(gdata);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (!g_cancellable_is_cancelled(data->cancellable))
         {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             // notify our listeners of the request
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             auto self = data->self;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             struct PKRequest req;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             req.public_key = data->public_key;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             req.fingerprint = get_fingerprint(req.public_key);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             req.respond = [self](PKResponse response){self->on_public_key_response(response);};
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             self->m_on_pk_request(req);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         }
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         return G_SOURCE_REMOVE;
     }
 
     void on_public_key_response(PKResponse response)
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s got response %d", G_STRLOC, int(response));
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // set m_pkresponse and wake up the waiting worker thread
         std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse = response;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_ready = true;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     /***
@@ -120,59 +147,89 @@ private:
 
     void worker_func() // runs in worker thread
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         const std::string socket_path {m_socket_path};
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         while (!g_cancellable_is_cancelled(m_cancellable))
         {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             g_debug("%s thread %p creating a client socket to '%s'", G_STRLOC, g_thread_self(), socket_path.c_str());
             auto socket = create_client_socket(socket_path);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             bool got_valid_req = false;
 
             g_debug("%s thread %p calling read_request", g_thread_self(), G_STRLOC);
             std::string reqstr;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             if (socket != nullptr)
                 reqstr = read_request(socket);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             if (!reqstr.empty())
                 g_debug("%s got request [%s]", G_STRLOC, reqstr.c_str());
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             if (reqstr.substr(0,2) == "PK") {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 PKResponse response = PKResponse::DENY;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 const auto public_key = reqstr.substr(2);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s thread %p got pk [%s]", G_STRLOC, g_thread_self(), public_key.c_str());
                 if (!public_key.empty()) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     got_valid_req = true;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse_ready = false;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse = AdbdClient::PKResponse::DENY;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     pass_public_key_to_main_thread(public_key);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     g_debug("%s thread %p waiting", G_STRLOC, g_thread_self());
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     try {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                         m_pkresponse_cv.wait(lk, [this](){
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                             return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
                         });
                     } catch (std::system_error& e) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                         g_critical("%s thread %p unable to wait for response because of unexpected error '%s'", G_STRLOC, g_thread_self(), e.what());
                     }
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     response = m_pkresponse;
                     g_debug("%s thread %p got response '%d', is-cancelled %d", G_STRLOC,
                             g_thread_self(),
                             int(response),
                             int(g_cancellable_is_cancelled(m_cancellable)));
                 }
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 if (!g_cancellable_is_cancelled(m_cancellable))
                     send_pk_response(socket, response);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             } else if (!reqstr.empty()) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 g_warning("Invalid ADB request: [%s]", reqstr.c_str());
             }
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_object(&socket);
 
             // If nothing interesting's happening, sleep a bit.
             // (Interval copied from UsbDebuggingManager.java)
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             static constexpr std::chrono::seconds sleep_interval {std::chrono::seconds(1)};
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             if (!got_valid_req && !g_cancellable_is_cancelled(m_cancellable)) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 std::unique_lock<std::mutex> lk(m_sleep_mutex);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 m_sleep_cv.wait_for(lk, sleep_interval);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             }
         }
     }
@@ -181,10 +238,12 @@ private:
     GSocket* create_client_socket(const std::string& socket_path)
     {
         GError* error {};
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         auto socket = g_socket_new(G_SOCKET_FAMILY_UNIX,
                                    G_SOCKET_TYPE_STREAM,
                                    G_SOCKET_PROTOCOL_DEFAULT,
                                    &error);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (error != nullptr) {
             g_warning("Error creating adbd client socket: %s", error->message);
             g_clear_error(&error);
@@ -192,55 +251,80 @@ private:
             return nullptr;
         }
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         auto address = g_unix_socket_address_new(socket_path.c_str());
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         const auto connected = g_socket_connect(socket, address, m_cancellable, &error);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_object(&address);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (!connected) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             g_debug("unable to connect to '%s': %s", socket_path.c_str(), error->message);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_error(&error);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_object(&socket);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             return nullptr;
         }
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         return socket;
     }
 
     std::string read_request(GSocket* socket)
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         char buf[4096] = {};
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s calling g_socket_receive()", G_STRLOC);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         const auto n_bytes = g_socket_receive (socket, buf, sizeof(buf), m_cancellable, nullptr);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         std::string ret;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (n_bytes > 0)
             ret.append(buf, std::string::size_type(n_bytes));
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s g_socket_receive got %d bytes: [%s]", G_STRLOC, int(n_bytes), ret.c_str());
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         return ret;
     }
 
     void send_pk_response(GSocket* socket, PKResponse response)
     {
         std::string response_str;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         switch(response) {
             case PKResponse::ALLOW: response_str = "OK"; break;
             case PKResponse::DENY:  response_str = "NO"; break;
         }
         g_debug("%s sending reply: [%s]", G_STRLOC, response_str.c_str());
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         GError* error {};
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_socket_send(socket,
                       response_str.c_str(),
                       response_str.size(),
                       m_cancellable,
                       &error);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (error != nullptr) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 g_warning("GAdbdServer: Error accepting socket connection: %s", error->message);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_error(&error);
         }
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     static std::string get_fingerprint(const std::string& public_key)
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // The first token is base64-encoded data, so cut on the first whitespace
         const std::string base64 (
             public_key.begin(),
@@ -250,14 +334,20 @@ private:
             )
         );
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         gsize digest_len {};
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         auto digest = g_base64_decode(base64.c_str(), &digest_len);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
         auto checksum = g_compute_checksum_for_data(G_CHECKSUM_MD5, digest, digest_len);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         const gsize checksum_len = checksum ? strlen(checksum) : 0;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
         // insert ':' between character pairs; eg "ff27b5f3" --> "ff:27:b5:f3"
         std::string fingerprint;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         for (gsize i=0; i<checksum_len; ) {
             fingerprint.append(checksum+i, checksum+i+2);
             if (i < checksum_len-2)
@@ -265,8 +355,11 @@ private:
             i += 2;
         }
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_pointer(&digest, g_free);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_pointer(&checksum, g_free);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         return fingerprint;
     }
 
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index aac289a..8d6a0c9 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -45,89 +45,127 @@ public:
         m_usb_monitor{usb_monitor},
         m_greeter{greeter}
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             restart();
         });
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_greeter->is_active().changed().connect([this](bool /*is_active*/) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             maybe_snap();
         });
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         restart();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     ~Impl()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (m_restart_idle_tag)
             g_source_remove(m_restart_idle_tag);
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         clear();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
 private:
 
     void clear()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // clear out old state
         m_snap_connections.clear();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_snap.reset();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_req = decltype(m_req)();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_adbd_client.reset();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void restart()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         clear();
 
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // set a new client
         m_adbd_client.reset(new GAdbdClient{m_socket_path});
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_adbd_client->on_pk_request().connect(
             [this](const AdbdClient::PKRequest& req) {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s got pk request: %s", G_STRLOC, req.fingerprint.c_str());
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 m_req = req;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 maybe_snap();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             }
         );
     }
 
     void maybe_snap()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // don't prompt in the greeter!
         if (!m_req.public_key.empty() && !m_greeter->is_active().get())
             snap();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void snap()
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_snap = std::make_shared<UsbSnap>(m_req.fingerprint);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_snap_connections.insert((*m_snap).on_user_response().connect(
             [this](AdbdClient::PKResponse response, bool remember_choice){
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s thread %p user responded! response %d, remember %d", G_STRLOC, g_thread_self(), int(response), int(remember_choice));
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 m_req.respond(response);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 if (remember_choice && (response == AdbdClient::PKResponse::ALLOW))
                     write_public_key(m_req.public_key);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                 m_restart_idle_tag = g_idle_add([](gpointer gself){
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     auto self = static_cast<Impl*>(gself);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     self->m_restart_idle_tag = 0;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     self->restart();
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
                     return G_SOURCE_REMOVE;
                 }, this);
             }
         ));
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void write_public_key(const std::string& public_key)
     {
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s writing public key '%s' to '%s'", G_STRLOC, public_key.c_str(), m_public_keys_filename.c_str());
 
         // confirm the directory exists
         auto dirname = g_path_get_dirname(m_public_keys_filename.c_str());
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         const auto dir_exists = g_file_test(dirname, G_FILE_TEST_IS_DIR);
         if (!dir_exists)
             g_warning("ADB data directory '%s' does not exist", dirname);
         g_clear_pointer(&dirname, g_free);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (!dir_exists)
             return;
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
         // open the file in append mode, with user rw and group r permissions
         const auto fd = open(
@@ -135,16 +173,20 @@ private:
             O_APPEND|O_CREAT|O_WRONLY,
             S_IRUSR|S_IWUSR|S_IRGRP
         );
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (fd == -1) {
             g_warning("Error opening ADB datafile: %s", g_strerror(errno));
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
             return;
         }
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
         // write the new public key on its own line
         std::string buf {public_key + '\n'};
         if (write(fd, buf.c_str(), buf.size()) == -1)
             g_warning("Error writing ADB datafile: %d %s", errno, g_strerror(errno));
         close(fd);
+g_debug("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     const std::string m_socket_path;
-- 
cgit v1.2.3


From baa9f937ac60cf1077a0f65197478a376135134d Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 12:03:49 -0500
Subject: GAdbdClient::Impl::on_public_key_response() doesn't need a condition
 variable wait, so use std::lock_guard rather than std::unique_lock

---
 src/adbd-client.cpp | 13 +++++++------
 src/main.cpp        |  1 +
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 1539982..0d955ab 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -44,7 +44,7 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
     ~Impl()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_debug("%s %s DTOR DTOR dtor", G_STRLOC, G_STRFUNC);
         // tell the worker thread to stop whatever it's doing and exit.
         g_debug("%s Client::Impl dtor, cancelling m_cancellable", G_STRLOC);
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
@@ -63,7 +63,7 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
     core::Signal<const PKRequest&>& on_pk_request()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         return m_on_pk_request;
     }
 
@@ -98,7 +98,7 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
     {
         /* NB: It's possible (though unlikely) that data.self was destroyed
            while this callback was pending, so we must check is-cancelled FIRST */
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_debug("%s %s thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
         auto data = static_cast<PKIdleData*>(gdata);
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         if (!g_cancellable_is_cancelled(data->cancellable))
@@ -127,11 +127,12 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
     void on_public_key_response(PKResponse response)
     {
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
-        g_debug("%s got response %d", G_STRLOC, int(response));
+        g_debug("%s thread %p got response %d", G_STRLOC, g_thread_self(), int(response));
 
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // set m_pkresponse and wake up the waiting worker thread
-        std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
+        std::lock_guard<std::mutex> lk(m_sleep_mutex);
+        //std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse = response;
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
@@ -147,7 +148,7 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
     void worker_func() // runs in worker thread
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_debug("%s %s worker thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
         const std::string socket_path {m_socket_path};
 
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
diff --git a/src/main.cpp b/src/main.cpp
index 52cdd58..43dc5f5 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -46,6 +46,7 @@ main(int /*argc*/, char** /*argv*/)
       g_warning("busname lost: '%s'", name.c_str());
       g_main_loop_quit(loop);
     };
+g_debug("%s %s main thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
 
     // build all our indicators.
     // Right now we've only got one -- rotation lock -- but hey, we can dream.
-- 
cgit v1.2.3


From c17e7b8f11eead4105b6fc706650780e68f6634c Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 12:43:02 -0500
Subject: fix r25 copypaste error, lock on the correct mutex

---
 src/adbd-client.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 0d955ab..25aefa8 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -131,7 +131,7 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // set m_pkresponse and wake up the waiting worker thread
-        std::lock_guard<std::mutex> lk(m_sleep_mutex);
+        std::lock_guard<std::mutex> lk(m_pkresponse_mutex);
         //std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse = response;
-- 
cgit v1.2.3


From 3a644063077a86899b59614048d84f86e77f75cd Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 12:52:56 -0500
Subject: The notifying thread does not need to hold the lock on the same mutex
 as the one held by the waiting thread(s); in fact doing so is a
 pessimization.

---
 src/adbd-client.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 25aefa8..597e69f 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -131,7 +131,7 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         // set m_pkresponse and wake up the waiting worker thread
-        std::lock_guard<std::mutex> lk(m_pkresponse_mutex);
+        //std::lock_guard<std::mutex> lk(m_pkresponse_mutex);
         //std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
 g_debug("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse = response;
-- 
cgit v1.2.3


From c19c2ea1691955e9691a2ad7efd9bbd3044be1e8 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 13:18:58 -0500
Subject: more g_debug() tracers

---
 tests/unit/adbd-client-test.cpp | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/tests/unit/adbd-client-test.cpp b/tests/unit/adbd-client-test.cpp
index 754f76c..c72de9b 100644
--- a/tests/unit/adbd-client-test.cpp
+++ b/tests/unit/adbd-client-test.cpp
@@ -74,10 +74,15 @@ TEST_F(AdbdClientFixture, SocketPlumbing)
         std::string pk;
         auto adbd_client = std::make_shared<GAdbdClient>(socket_path);
         adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){
+g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             EXPECT_EQ(main_thread, g_thread_self());
+g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             g_message("in on_pk_request with %s", req.public_key.c_str());
+g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             pk = req.public_key;
+g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             req.respond(test.response);
+g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         });
 
         // start a mock AdbdServer with to fire test key requests and wait for a response
-- 
cgit v1.2.3


From ec9196ac95986edbd92596991930a430e4dcc5aa Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 13:56:54 -0500
Subject: in AdbdClientDest, explicitly disconnect the core::Signals at the end
 of the test

---
 tests/unit/adbd-client-test.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/unit/adbd-client-test.cpp b/tests/unit/adbd-client-test.cpp
index c72de9b..148a46b 100644
--- a/tests/unit/adbd-client-test.cpp
+++ b/tests/unit/adbd-client-test.cpp
@@ -73,7 +73,7 @@ TEST_F(AdbdClientFixture, SocketPlumbing)
         // start an AdbdClient that listens for PKRequests
         std::string pk;
         auto adbd_client = std::make_shared<GAdbdClient>(socket_path);
-        adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){
+        auto connection = adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){
 g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             EXPECT_EQ(main_thread, g_thread_self());
 g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
@@ -93,6 +93,7 @@ g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         EXPECT_EQ(test.expected_response, adbd_server->m_responses.front());
    
         // cleanup
+        connection.disconnect();
         adbd_client.reset();
         adbd_server.reset();
         g_unlink(socket_path.c_str());
-- 
cgit v1.2.3


From 0236d509029b91b496faf0c3a0b06be25a0af9a3 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 13:57:27 -0500
Subject: in AdbdClientTest, raise the test timeout interval from 2s to 5s

---
 tests/unit/adbd-client-test.cpp | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tests/unit/adbd-client-test.cpp b/tests/unit/adbd-client-test.cpp
index 148a46b..6506e28 100644
--- a/tests/unit/adbd-client-test.cpp
+++ b/tests/unit/adbd-client-test.cpp
@@ -87,7 +87,7 @@ g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
 
         // start a mock AdbdServer with to fire test key requests and wait for a response
         auto adbd_server = std::make_shared<GAdbdServer>(socket_path, std::vector<std::string>{test.request});
-        wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 2000);
+        wait_for([adbd_server](){return !adbd_server->m_responses.empty();}, 5000);
         EXPECT_EQ(test.expected_pk, pk);
         ASSERT_EQ(1, adbd_server->m_responses.size());
         EXPECT_EQ(test.expected_response, adbd_server->m_responses.front());
-- 
cgit v1.2.3


From 42e7cc5b9c1c4053112d570b94689f11219b71ce Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 13:59:03 -0500
Subject: in UsbSnapTest, explicitly disconnect the core::Signals at the end of
 the test

---
 tests/unit/usb-snap-test.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tests/unit/usb-snap-test.cpp b/tests/unit/usb-snap-test.cpp
index 3b778dd..53548f4 100644
--- a/tests/unit/usb-snap-test.cpp
+++ b/tests/unit/usb-snap-test.cpp
@@ -88,7 +88,7 @@ TEST_F(UsbSnapFixture, TestRoundTrip)
         auto snap = std::make_shared<UsbSnap>(test.fingerprint);
         AdbdClient::PKResponse user_response {};
         bool user_response_set = false;
-        snap->on_user_response().connect([&user_response,&user_response_set](AdbdClient::PKResponse response, bool /*remember*/){
+        auto connection = snap->on_user_response().connect([&user_response,&user_response_set](AdbdClient::PKResponse response, bool /*remember*/){
             user_response = response;
             user_response_set = true;
         });
@@ -131,6 +131,7 @@ TEST_F(UsbSnapFixture, TestRoundTrip)
         ASSERT_EQ(test.expected_response, user_response);
 
         // confirm that the snap dtor cleans up the notification
+        connection.disconnect();
         snap.reset();
         wait_for_signals(notificationsSpy, 1);
         {
-- 
cgit v1.2.3


From c997668e2517d414dfa52e3204e54d67540ba662 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 15:34:08 -0500
Subject: more g_debug() tracers

---
 tests/unit/adbd-client-test.cpp | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/tests/unit/adbd-client-test.cpp b/tests/unit/adbd-client-test.cpp
index 6506e28..20d6ee4 100644
--- a/tests/unit/adbd-client-test.cpp
+++ b/tests/unit/adbd-client-test.cpp
@@ -70,19 +70,20 @@ TEST_F(AdbdClientFixture, SocketPlumbing)
 
     for (const auto& test : tests)
     {
+g_message("%s %s thread %p starting test", G_STRLOC, G_STRFUNC, g_thread_self());
         // start an AdbdClient that listens for PKRequests
         std::string pk;
         auto adbd_client = std::make_shared<GAdbdClient>(socket_path);
         auto connection = adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){
-g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             EXPECT_EQ(main_thread, g_thread_self());
-g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             g_message("in on_pk_request with %s", req.public_key.c_str());
-g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             pk = req.public_key;
-g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             req.respond(test.response);
-g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         });
 
         // start a mock AdbdServer with to fire test key requests and wait for a response
@@ -93,9 +94,11 @@ g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         EXPECT_EQ(test.expected_response, adbd_server->m_responses.front());
    
         // cleanup
+g_message("%s %s thread %p cleanup", G_STRLOC, G_STRFUNC, g_thread_self());
         connection.disconnect();
         adbd_client.reset();
         adbd_server.reset();
         g_unlink(socket_path.c_str());
+g_message("%s %s thread %p test exit", G_STRLOC, G_STRFUNC, g_thread_self());
     }
 }
-- 
cgit v1.2.3


From a17e7fc7ed94bf201b37cf7d9744ef272404c0f7 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 16:10:01 -0500
Subject: tweak the debug tracers

---
 src/adbd-client.cpp | 186 ++++++++++++++++++++++++++--------------------------
 src/main.cpp        |   2 +-
 src/usb-manager.cpp |  84 ++++++++++++------------
 3 files changed, 136 insertions(+), 136 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 597e69f..fbaf377 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -39,31 +39,31 @@ public:
         m_cancellable{g_cancellable_new()},
         m_worker_thread{&Impl::worker_func, this}
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     ~Impl()
     {
-g_debug("%s %s DTOR DTOR dtor", G_STRLOC, G_STRFUNC);
+g_message("%s %s DTOR DTOR dtor", G_STRLOC, G_STRFUNC);
         // tell the worker thread to stop whatever it's doing and exit.
         g_debug("%s Client::Impl dtor, cancelling m_cancellable", G_STRLOC);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_cancellable_cancel(m_cancellable);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_sleep_cv.notify_one();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (m_worker_thread.joinable())
             m_worker_thread.join();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_object(&m_cancellable);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     core::Signal<const PKRequest&>& on_pk_request()
     {
-g_debug("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         return m_on_pk_request;
     }
 
@@ -87,7 +87,7 @@ private:
 
     void pass_public_key_to_main_thread(const std::string& public_key)
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_idle_add_full(G_PRIORITY_DEFAULT_IDLE,
                         on_public_key_request_static,
                         new PKIdleData{this, m_cancellable, public_key},
@@ -98,48 +98,48 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
     {
         /* NB: It's possible (though unlikely) that data.self was destroyed
            while this callback was pending, so we must check is-cancelled FIRST */
-g_debug("%s %s thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
         auto data = static_cast<PKIdleData*>(gdata);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (!g_cancellable_is_cancelled(data->cancellable))
         {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             // notify our listeners of the request
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             auto self = data->self;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             struct PKRequest req;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             req.public_key = data->public_key;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             req.fingerprint = get_fingerprint(req.public_key);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             req.respond = [self](PKResponse response){self->on_public_key_response(response);};
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             self->m_on_pk_request(req);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         }
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         return G_SOURCE_REMOVE;
     }
 
     void on_public_key_response(PKResponse response)
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s thread %p got response %d", G_STRLOC, g_thread_self(), int(response));
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         // set m_pkresponse and wake up the waiting worker thread
         //std::lock_guard<std::mutex> lk(m_pkresponse_mutex);
         //std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse = response;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_ready = true;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     /***
@@ -148,89 +148,89 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
 
     void worker_func() // runs in worker thread
     {
-g_debug("%s %s worker thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s worker thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
         const std::string socket_path {m_socket_path};
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         while (!g_cancellable_is_cancelled(m_cancellable))
         {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_debug("%s thread %p creating a client socket to '%s'", G_STRLOC, g_thread_self(), socket_path.c_str());
             auto socket = create_client_socket(socket_path);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             bool got_valid_req = false;
 
             g_debug("%s thread %p calling read_request", g_thread_self(), G_STRLOC);
             std::string reqstr;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (socket != nullptr)
                 reqstr = read_request(socket);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (!reqstr.empty())
                 g_debug("%s got request [%s]", G_STRLOC, reqstr.c_str());
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (reqstr.substr(0,2) == "PK") {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 PKResponse response = PKResponse::DENY;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 const auto public_key = reqstr.substr(2);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s thread %p got pk [%s]", G_STRLOC, g_thread_self(), public_key.c_str());
                 if (!public_key.empty()) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     got_valid_req = true;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse_ready = false;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse = AdbdClient::PKResponse::DENY;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     pass_public_key_to_main_thread(public_key);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     g_debug("%s thread %p waiting", G_STRLOC, g_thread_self());
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     try {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                         m_pkresponse_cv.wait(lk, [this](){
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                             return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
                         });
                     } catch (std::system_error& e) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                         g_critical("%s thread %p unable to wait for response because of unexpected error '%s'", G_STRLOC, g_thread_self(), e.what());
                     }
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     response = m_pkresponse;
                     g_debug("%s thread %p got response '%d', is-cancelled %d", G_STRLOC,
                             g_thread_self(),
                             int(response),
                             int(g_cancellable_is_cancelled(m_cancellable)));
                 }
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 if (!g_cancellable_is_cancelled(m_cancellable))
                     send_pk_response(socket, response);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             } else if (!reqstr.empty()) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_warning("Invalid ADB request: [%s]", reqstr.c_str());
             }
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_object(&socket);
 
             // If nothing interesting's happening, sleep a bit.
             // (Interval copied from UsbDebuggingManager.java)
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             static constexpr std::chrono::seconds sleep_interval {std::chrono::seconds(1)};
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (!got_valid_req && !g_cancellable_is_cancelled(m_cancellable)) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 std::unique_lock<std::mutex> lk(m_sleep_mutex);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 m_sleep_cv.wait_for(lk, sleep_interval);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             }
         }
     }
@@ -239,12 +239,12 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
     GSocket* create_client_socket(const std::string& socket_path)
     {
         GError* error {};
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto socket = g_socket_new(G_SOCKET_FAMILY_UNIX,
                                    G_SOCKET_TYPE_STREAM,
                                    G_SOCKET_PROTOCOL_DEFAULT,
                                    &error);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (error != nullptr) {
             g_warning("Error creating adbd client socket: %s", error->message);
             g_clear_error(&error);
@@ -252,80 +252,80 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
             return nullptr;
         }
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto address = g_unix_socket_address_new(socket_path.c_str());
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         const auto connected = g_socket_connect(socket, address, m_cancellable, &error);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_object(&address);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (!connected) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_debug("unable to connect to '%s': %s", socket_path.c_str(), error->message);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_error(&error);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_object(&socket);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             return nullptr;
         }
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         return socket;
     }
 
     std::string read_request(GSocket* socket)
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         char buf[4096] = {};
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s calling g_socket_receive()", G_STRLOC);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         const auto n_bytes = g_socket_receive (socket, buf, sizeof(buf), m_cancellable, nullptr);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         std::string ret;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (n_bytes > 0)
             ret.append(buf, std::string::size_type(n_bytes));
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s g_socket_receive got %d bytes: [%s]", G_STRLOC, int(n_bytes), ret.c_str());
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         return ret;
     }
 
     void send_pk_response(GSocket* socket, PKResponse response)
     {
         std::string response_str;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         switch(response) {
             case PKResponse::ALLOW: response_str = "OK"; break;
             case PKResponse::DENY:  response_str = "NO"; break;
         }
         g_debug("%s sending reply: [%s]", G_STRLOC, response_str.c_str());
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         GError* error {};
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_socket_send(socket,
                       response_str.c_str(),
                       response_str.size(),
                       m_cancellable,
                       &error);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (error != nullptr) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_warning("GAdbdServer: Error accepting socket connection: %s", error->message);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_error(&error);
         }
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     static std::string get_fingerprint(const std::string& public_key)
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         // The first token is base64-encoded data, so cut on the first whitespace
         const std::string base64 (
             public_key.begin(),
@@ -335,20 +335,20 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
             )
         );
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         gsize digest_len {};
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto digest = g_base64_decode(base64.c_str(), &digest_len);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         auto checksum = g_compute_checksum_for_data(G_CHECKSUM_MD5, digest, digest_len);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         const gsize checksum_len = checksum ? strlen(checksum) : 0;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         // insert ':' between character pairs; eg "ff27b5f3" --> "ff:27:b5:f3"
         std::string fingerprint;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         for (gsize i=0; i<checksum_len; ) {
             fingerprint.append(checksum+i, checksum+i+2);
             if (i < checksum_len-2)
@@ -356,11 +356,11 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
             i += 2;
         }
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_pointer(&digest, g_free);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_pointer(&checksum, g_free);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         return fingerprint;
     }
 
diff --git a/src/main.cpp b/src/main.cpp
index 43dc5f5..0ece7dc 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -46,7 +46,7 @@ main(int /*argc*/, char** /*argv*/)
       g_warning("busname lost: '%s'", name.c_str());
       g_main_loop_quit(loop);
     };
-g_debug("%s %s main thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
+g_message("%s %s main thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
 
     // build all our indicators.
     // Right now we've only got one -- rotation lock -- but hey, we can dream.
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 8d6a0c9..8b24f17 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -45,127 +45,127 @@ public:
         m_usb_monitor{usb_monitor},
         m_greeter{greeter}
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             restart();
         });
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_greeter->is_active().changed().connect([this](bool /*is_active*/) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             maybe_snap();
         });
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         restart();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     ~Impl()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (m_restart_idle_tag)
             g_source_remove(m_restart_idle_tag);
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         clear();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
 private:
 
     void clear()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         // clear out old state
         m_snap_connections.clear();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_snap.reset();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_req = decltype(m_req)();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_adbd_client.reset();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void restart()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         clear();
 
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         // set a new client
         m_adbd_client.reset(new GAdbdClient{m_socket_path});
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_adbd_client->on_pk_request().connect(
             [this](const AdbdClient::PKRequest& req) {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s got pk request: %s", G_STRLOC, req.fingerprint.c_str());
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 m_req = req;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 maybe_snap();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             }
         );
     }
 
     void maybe_snap()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         // don't prompt in the greeter!
         if (!m_req.public_key.empty() && !m_greeter->is_active().get())
             snap();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void snap()
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_snap = std::make_shared<UsbSnap>(m_req.fingerprint);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_snap_connections.insert((*m_snap).on_user_response().connect(
             [this](AdbdClient::PKResponse response, bool remember_choice){
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s thread %p user responded! response %d, remember %d", G_STRLOC, g_thread_self(), int(response), int(remember_choice));
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 m_req.respond(response);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 if (remember_choice && (response == AdbdClient::PKResponse::ALLOW))
                     write_public_key(m_req.public_key);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                 m_restart_idle_tag = g_idle_add([](gpointer gself){
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     auto self = static_cast<Impl*>(gself);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     self->m_restart_idle_tag = 0;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     self->restart();
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
                     return G_SOURCE_REMOVE;
                 }, this);
             }
         ));
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void write_public_key(const std::string& public_key)
     {
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s writing public key '%s' to '%s'", G_STRLOC, public_key.c_str(), m_public_keys_filename.c_str());
 
         // confirm the directory exists
         auto dirname = g_path_get_dirname(m_public_keys_filename.c_str());
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         const auto dir_exists = g_file_test(dirname, G_FILE_TEST_IS_DIR);
         if (!dir_exists)
             g_warning("ADB data directory '%s' does not exist", dirname);
         g_clear_pointer(&dirname, g_free);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (!dir_exists)
             return;
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         // open the file in append mode, with user rw and group r permissions
         const auto fd = open(
@@ -173,20 +173,20 @@ g_debug("%s %s", G_STRLOC, G_STRFUNC);
             O_APPEND|O_CREAT|O_WRONLY,
             S_IRUSR|S_IWUSR|S_IRGRP
         );
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (fd == -1) {
             g_warning("Error opening ADB datafile: %s", g_strerror(errno));
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
             return;
         }
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         // write the new public key on its own line
         std::string buf {public_key + '\n'};
         if (write(fd, buf.c_str(), buf.size()) == -1)
             g_warning("Error writing ADB datafile: %d %s", errno, g_strerror(errno));
         close(fd);
-g_debug("%s %s", G_STRLOC, G_STRFUNC);
+g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     const std::string m_socket_path;
-- 
cgit v1.2.3


From fe6fe0ae1e87d46c6060045cc3b25865df4533bd Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 16:11:46 -0500
Subject: keep piling 'em on

---
 src/adbd-client.cpp | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index fbaf377..dfa97da 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -121,6 +121,7 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
         }
 
 g_message("%s %s", G_STRLOC, G_STRFUNC);
+fflush(nullptr);
         return G_SOURCE_REMOVE;
     }
 
@@ -140,6 +141,7 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
 g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
 g_message("%s %s", G_STRLOC, G_STRFUNC);
+fflush(nullptr);
     }
 
     /***
-- 
cgit v1.2.3


From e616541cde47710de062367cc74c9b18821d9a73 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 16:33:48 -0500
Subject: fix tyop

---
 src/adbd-client.cpp | 17 +++--------------
 src/usb-manager.cpp |  1 +
 2 files changed, 4 insertions(+), 14 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index dfa97da..585352c 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -190,20 +190,9 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse = AdbdClient::PKResponse::DENY;
 g_message("%s %s", G_STRLOC, G_STRFUNC);
                     pass_public_key_to_main_thread(public_key);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                    g_debug("%s thread %p waiting", G_STRLOC, g_thread_self());
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                    try {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                        m_pkresponse_cv.wait(lk, [this](){
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                            return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
-                        });
-                    } catch (std::system_error& e) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-                        g_critical("%s thread %p unable to wait for response because of unexpected error '%s'", G_STRLOC, g_thread_self(), e.what());
-                    }
-g_message("%s %s", G_STRLOC, G_STRFUNC);
+                    m_pkresponse_cv.wait(lk, [this](){
+                        return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
+                    });
                     response = m_pkresponse;
                     g_debug("%s thread %p got response '%d', is-cancelled %d", G_STRLOC,
                             g_thread_self(),
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 81c1b9d..ee9b463 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -80,6 +80,7 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
                 maybe_snap();
             }
         );
+    }
 
     ~Impl()
     {
-- 
cgit v1.2.3


From 549cd2b06a89216cdbf198d225723134fb551b32 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Thu, 22 Sep 2016 16:54:55 -0500
Subject: use a std::atomic for adbd-client's m_pkresponse_ready

---
 src/adbd-client.cpp | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 585352c..f36a823 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -23,6 +23,7 @@
 #include <gio/gunixsocketaddress.h>
 
 #include <algorithm>
+#include <atomic>
 #include <cctype>
 #include <cstring>
 #include <chrono>
@@ -365,7 +366,7 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
 
     std::mutex m_pkresponse_mutex;
     std::condition_variable m_pkresponse_cv;
-    bool m_pkresponse_ready = false;
+    std::atomic<bool> m_pkresponse_ready {false};
     PKResponse m_pkresponse = PKResponse::DENY;
 };
 
-- 
cgit v1.2.3


From 9e5e5d7e39c9f7bbeafa4d6f0dfa4b8697e9d770 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 19 Oct 2016 11:41:02 +0200
Subject: remove the temporary tracers from r34 and r35

---
 src/adbd-client.cpp             | 89 -----------------------------------------
 src/main.cpp                    |  1 -
 src/usb-manager.cpp             | 24 +----------
 tests/unit/adbd-client-test.cpp |  8 ----
 4 files changed, 1 insertion(+), 121 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index f36a823..3f86818 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -40,31 +40,22 @@ public:
         m_cancellable{g_cancellable_new()},
         m_worker_thread{&Impl::worker_func, this}
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     ~Impl()
     {
-g_message("%s %s DTOR DTOR dtor", G_STRLOC, G_STRFUNC);
         // tell the worker thread to stop whatever it's doing and exit.
         g_debug("%s Client::Impl dtor, cancelling m_cancellable", G_STRLOC);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_cancellable_cancel(m_cancellable);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_sleep_cv.notify_one();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (m_worker_thread.joinable())
             m_worker_thread.join();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_object(&m_cancellable);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     core::Signal<const PKRequest&>& on_pk_request()
     {
-g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         return m_on_pk_request;
     }
 
@@ -88,7 +79,6 @@ private:
 
     void pass_public_key_to_main_thread(const std::string& public_key)
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_idle_add_full(G_PRIORITY_DEFAULT_IDLE,
                         on_public_key_request_static,
                         new PKIdleData{this, m_cancellable, public_key},
@@ -99,50 +89,31 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
     {
         /* NB: It's possible (though unlikely) that data.self was destroyed
            while this callback was pending, so we must check is-cancelled FIRST */
-g_message("%s %s thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
         auto data = static_cast<PKIdleData*>(gdata);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (!g_cancellable_is_cancelled(data->cancellable))
         {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             // notify our listeners of the request
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             auto self = data->self;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             struct PKRequest req;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             req.public_key = data->public_key;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             req.fingerprint = get_fingerprint(req.public_key);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             req.respond = [self](PKResponse response){self->on_public_key_response(response);};
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             self->m_on_pk_request(req);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         }
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-fflush(nullptr);
         return G_SOURCE_REMOVE;
     }
 
     void on_public_key_response(PKResponse response)
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s thread %p got response %d", G_STRLOC, g_thread_self(), int(response));
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         // set m_pkresponse and wake up the waiting worker thread
         //std::lock_guard<std::mutex> lk(m_pkresponse_mutex);
         //std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse = response;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_ready = true;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_pkresponse_cv.notify_one();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
-fflush(nullptr);
     }
 
     /***
@@ -151,45 +122,30 @@ fflush(nullptr);
 
     void worker_func() // runs in worker thread
     {
-g_message("%s %s worker thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
         const std::string socket_path {m_socket_path};
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         while (!g_cancellable_is_cancelled(m_cancellable))
         {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_debug("%s thread %p creating a client socket to '%s'", G_STRLOC, g_thread_self(), socket_path.c_str());
             auto socket = create_client_socket(socket_path);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             bool got_valid_req = false;
 
             g_debug("%s thread %p calling read_request", g_thread_self(), G_STRLOC);
             std::string reqstr;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (socket != nullptr)
                 reqstr = read_request(socket);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (!reqstr.empty())
                 g_debug("%s got request [%s]", G_STRLOC, reqstr.c_str());
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (reqstr.substr(0,2) == "PK") {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 PKResponse response = PKResponse::DENY;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 const auto public_key = reqstr.substr(2);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_debug("%s thread %p got pk [%s]", G_STRLOC, g_thread_self(), public_key.c_str());
                 if (!public_key.empty()) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                     got_valid_req = true;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                     std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse_ready = false;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                     m_pkresponse = AdbdClient::PKResponse::DENY;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                     pass_public_key_to_main_thread(public_key);
                     m_pkresponse_cv.wait(lk, [this](){
                         return m_pkresponse_ready || g_cancellable_is_cancelled(m_cancellable);
@@ -200,29 +156,20 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
                             int(response),
                             int(g_cancellable_is_cancelled(m_cancellable)));
                 }
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 if (!g_cancellable_is_cancelled(m_cancellable))
                     send_pk_response(socket, response);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             } else if (!reqstr.empty()) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_warning("Invalid ADB request: [%s]", reqstr.c_str());
             }
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_object(&socket);
 
             // If nothing interesting's happening, sleep a bit.
             // (Interval copied from UsbDebuggingManager.java)
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             static constexpr std::chrono::seconds sleep_interval {std::chrono::seconds(1)};
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (!got_valid_req && !g_cancellable_is_cancelled(m_cancellable)) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 std::unique_lock<std::mutex> lk(m_sleep_mutex);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 m_sleep_cv.wait_for(lk, sleep_interval);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             }
         }
     }
@@ -231,12 +178,10 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
     GSocket* create_client_socket(const std::string& socket_path)
     {
         GError* error {};
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto socket = g_socket_new(G_SOCKET_FAMILY_UNIX,
                                    G_SOCKET_TYPE_STREAM,
                                    G_SOCKET_PROTOCOL_DEFAULT,
                                    &error);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (error != nullptr) {
             g_warning("Error creating adbd client socket: %s", error->message);
             g_clear_error(&error);
@@ -244,80 +189,55 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
             return nullptr;
         }
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto address = g_unix_socket_address_new(socket_path.c_str());
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         const auto connected = g_socket_connect(socket, address, m_cancellable, &error);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_object(&address);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (!connected) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_debug("unable to connect to '%s': %s", socket_path.c_str(), error->message);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_error(&error);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_object(&socket);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             return nullptr;
         }
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         return socket;
     }
 
     std::string read_request(GSocket* socket)
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         char buf[4096] = {};
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s calling g_socket_receive()", G_STRLOC);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         const auto n_bytes = g_socket_receive (socket, buf, sizeof(buf), m_cancellable, nullptr);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         std::string ret;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (n_bytes > 0)
             ret.append(buf, std::string::size_type(n_bytes));
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s g_socket_receive got %d bytes: [%s]", G_STRLOC, int(n_bytes), ret.c_str());
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         return ret;
     }
 
     void send_pk_response(GSocket* socket, PKResponse response)
     {
         std::string response_str;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         switch(response) {
             case PKResponse::ALLOW: response_str = "OK"; break;
             case PKResponse::DENY:  response_str = "NO"; break;
         }
         g_debug("%s sending reply: [%s]", G_STRLOC, response_str.c_str());
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         GError* error {};
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_socket_send(socket,
                       response_str.c_str(),
                       response_str.size(),
                       m_cancellable,
                       &error);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (error != nullptr) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
-g_message("%s %s", G_STRLOC, G_STRFUNC);
                 g_warning("GAdbdServer: Error accepting socket connection: %s", error->message);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             g_clear_error(&error);
         }
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     static std::string get_fingerprint(const std::string& public_key)
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         // The first token is base64-encoded data, so cut on the first whitespace
         const std::string base64 (
             public_key.begin(),
@@ -327,20 +247,14 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
             )
         );
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         gsize digest_len {};
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         auto digest = g_base64_decode(base64.c_str(), &digest_len);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         auto checksum = g_compute_checksum_for_data(G_CHECKSUM_MD5, digest, digest_len);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         const gsize checksum_len = checksum ? strlen(checksum) : 0;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         // insert ':' between character pairs; eg "ff27b5f3" --> "ff:27:b5:f3"
         std::string fingerprint;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         for (gsize i=0; i<checksum_len; ) {
             fingerprint.append(checksum+i, checksum+i+2);
             if (i < checksum_len-2)
@@ -348,11 +262,8 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
             i += 2;
         }
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_pointer(&digest, g_free);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_clear_pointer(&checksum, g_free);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         return fingerprint;
     }
 
diff --git a/src/main.cpp b/src/main.cpp
index 0ece7dc..52cdd58 100644
--- a/src/main.cpp
+++ b/src/main.cpp
@@ -46,7 +46,6 @@ main(int /*argc*/, char** /*argv*/)
       g_warning("busname lost: '%s'", name.c_str());
       g_main_loop_quit(loop);
     };
-g_message("%s %s main thread is %p", G_STRLOC, G_STRFUNC, g_thread_self());
 
     // build all our indicators.
     // Right now we've only got one -- rotation lock -- but hey, we can dream.
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index ee9b463..7d66e74 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -45,21 +45,15 @@ public:
         m_usb_monitor{usb_monitor},
         m_greeter{greeter}
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_usb_monitor->on_usb_disconnected().connect([this](const std::string& /*usb_name*/) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             m_req.reset();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         });
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_greeter->state().changed().connect([this](const Greeter::State& state) {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             if (state == Greeter::State::INACTIVE)
                 maybe_snap();
             else
                 stop_snap();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         });
 
         // create a new adbd client
@@ -84,31 +78,24 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
 
     ~Impl()
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (m_request_complete_idle_tag)
             g_source_remove(m_request_complete_idle_tag);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
 private:
 
     void stop_snap()
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_snap_connections.clear();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         m_snap.reset();
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void maybe_snap()
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         // only prompt if there's something to prompt about
         if (!m_req)
             return;
 
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         // only prompt in an unlocked session
         if (m_greeter->state().get() != Greeter::State::INACTIVE)
             return;
@@ -139,25 +126,20 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
                 }
             }
         ));
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     void write_public_key(const std::string& public_key)
     {
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         g_debug("%s writing public key '%s' to '%s'", G_STRLOC, public_key.c_str(), m_public_keys_filename.c_str());
 
         // confirm the directory exists
         auto dirname = g_path_get_dirname(m_public_keys_filename.c_str());
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         const auto dir_exists = g_file_test(dirname, G_FILE_TEST_IS_DIR);
         if (!dir_exists)
             g_warning("ADB data directory '%s' does not exist", dirname);
         g_clear_pointer(&dirname, g_free);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (!dir_exists)
             return;
-g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         // open the file in append mode, with user rw and group r permissions
         const auto fd = open(
@@ -165,20 +147,16 @@ g_message("%s %s", G_STRLOC, G_STRFUNC);
             O_APPEND|O_CREAT|O_WRONLY,
             S_IRUSR|S_IWUSR|S_IRGRP
         );
-g_message("%s %s", G_STRLOC, G_STRFUNC);
         if (fd == -1) {
             g_warning("Error opening ADB datafile: %s", g_strerror(errno));
-g_message("%s %s", G_STRLOC, G_STRFUNC);
             return;
         }
-g_message("%s %s", G_STRLOC, G_STRFUNC);
 
         // write the new public key on its own line
         std::string buf {public_key + '\n'};
         if (write(fd, buf.c_str(), buf.size()) == -1)
             g_warning("Error writing ADB datafile: %d %s", errno, g_strerror(errno));
         close(fd);
-g_message("%s %s", G_STRLOC, G_STRFUNC);
     }
 
     const std::string m_socket_path;
@@ -211,4 +189,4 @@ UsbManager::UsbManager(
 
 UsbManager::~UsbManager()
 {
-}
\ No newline at end of file
+}
diff --git a/tests/unit/adbd-client-test.cpp b/tests/unit/adbd-client-test.cpp
index 20d6ee4..d1ce740 100644
--- a/tests/unit/adbd-client-test.cpp
+++ b/tests/unit/adbd-client-test.cpp
@@ -70,20 +70,14 @@ TEST_F(AdbdClientFixture, SocketPlumbing)
 
     for (const auto& test : tests)
     {
-g_message("%s %s thread %p starting test", G_STRLOC, G_STRFUNC, g_thread_self());
         // start an AdbdClient that listens for PKRequests
         std::string pk;
         auto adbd_client = std::make_shared<GAdbdClient>(socket_path);
         auto connection = adbd_client->on_pk_request().connect([&pk, main_thread, test](const AdbdClient::PKRequest& req){
-g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             EXPECT_EQ(main_thread, g_thread_self());
-g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             g_message("in on_pk_request with %s", req.public_key.c_str());
-g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             pk = req.public_key;
-g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
             req.respond(test.response);
-g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         });
 
         // start a mock AdbdServer with to fire test key requests and wait for a response
@@ -94,11 +88,9 @@ g_message("%s %s thread %p", G_STRLOC, G_STRFUNC, g_thread_self());
         EXPECT_EQ(test.expected_response, adbd_server->m_responses.front());
    
         // cleanup
-g_message("%s %s thread %p cleanup", G_STRLOC, G_STRFUNC, g_thread_self());
         connection.disconnect();
         adbd_client.reset();
         adbd_server.reset();
         g_unlink(socket_path.c_str());
-g_message("%s %s thread %p test exit", G_STRLOC, G_STRFUNC, g_thread_self());
     }
 }
-- 
cgit v1.2.3


From 144228c916b574e5f4cbc90b9b90c16f0ed9ac73 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Wed, 19 Oct 2016 11:43:52 +0200
Subject: remove dead code

---
 src/adbd-client.cpp | 2 --
 1 file changed, 2 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 3f86818..8b675d5 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -109,8 +109,6 @@ private:
         g_debug("%s thread %p got response %d", G_STRLOC, g_thread_self(), int(response));
 
         // set m_pkresponse and wake up the waiting worker thread
-        //std::lock_guard<std::mutex> lk(m_pkresponse_mutex);
-        //std::unique_lock<std::mutex> lk(m_pkresponse_mutex);
         m_pkresponse = response;
         m_pkresponse_ready = true;
         m_pkresponse_cv.notify_one();
-- 
cgit v1.2.3


From 7002fc4e6a6496fb5c0d3294540c957787689847 Mon Sep 17 00:00:00 2001
From: Charles Kerr <charles.kerr@canonical.com>
Date: Fri, 21 Oct 2016 13:32:21 +0200
Subject: add block braces as suggested by dobey

---
 src/adbd-client.cpp |  6 ++++--
 src/greeter.cpp     |  3 ++-
 src/usb-manager.cpp | 17 +++++++++++------
 3 files changed, 17 insertions(+), 9 deletions(-)

diff --git a/src/adbd-client.cpp b/src/adbd-client.cpp
index 8b675d5..47914cb 100644
--- a/src/adbd-client.cpp
+++ b/src/adbd-client.cpp
@@ -49,8 +49,9 @@ public:
         g_cancellable_cancel(m_cancellable);
         m_pkresponse_cv.notify_one();
         m_sleep_cv.notify_one();
-        if (m_worker_thread.joinable())
+        if (m_worker_thread.joinable()) {
             m_worker_thread.join();
+        }
         g_clear_object(&m_cancellable);
     }
 
@@ -154,8 +155,9 @@ private:
                             int(response),
                             int(g_cancellable_is_cancelled(m_cancellable)));
                 }
-                if (!g_cancellable_is_cancelled(m_cancellable))
+                if (!g_cancellable_is_cancelled(m_cancellable)) {
                     send_pk_response(socket, response);
+                }
             } else if (!reqstr.empty()) {
                 g_warning("Invalid ADB request: [%s]", reqstr.c_str());
             }
diff --git a/src/greeter.cpp b/src/greeter.cpp
index 9bd5db0..3d0f347 100644
--- a/src/greeter.cpp
+++ b/src/greeter.cpp
@@ -146,8 +146,9 @@ private:
         GError* error {};
         auto v = g_dbus_connection_call_finish(G_DBUS_CONNECTION(source), res, &error);
         if (error != nullptr) {
-            if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED))
+            if (!g_error_matches(error, G_IO_ERROR, G_IO_ERROR_CANCELLED)) {
                 g_warning("Greeter: Error getting IsActive property: %s", error->message);
+            }
             g_clear_error(&error);
         } else {
             GVariant* is_active {};
diff --git a/src/usb-manager.cpp b/src/usb-manager.cpp
index 7d66e74..f83b5f1 100644
--- a/src/usb-manager.cpp
+++ b/src/usb-manager.cpp
@@ -50,10 +50,11 @@ public:
         });
 
         m_greeter->state().changed().connect([this](const Greeter::State& state) {
-            if (state == Greeter::State::INACTIVE)
+            if (state == Greeter::State::INACTIVE) {
                 maybe_snap();
-            else
+            } else {
                 stop_snap();
+            }
         });
 
         // create a new adbd client
@@ -78,8 +79,9 @@ public:
 
     ~Impl()
     {
-        if (m_request_complete_idle_tag)
+        if (m_request_complete_idle_tag) {
             g_source_remove(m_request_complete_idle_tag);
+        }
     }
 
 private:
@@ -93,12 +95,14 @@ private:
     void maybe_snap()
     {
         // only prompt if there's something to prompt about
-        if (!m_req)
+        if (!m_req) {
             return;
+        }
 
         // only prompt in an unlocked session
-        if (m_greeter->state().get() != Greeter::State::INACTIVE)
+        if (m_greeter->state().get() != Greeter::State::INACTIVE) {
             return;
+        }
 
         snap();
     }
@@ -109,8 +113,9 @@ private:
         m_snap_connections.insert((*m_snap).on_user_response().connect(
             [this](AdbdClient::PKResponse response, bool remember_choice){
 
-                if (remember_choice && (response == AdbdClient::PKResponse::ALLOW))
+                if (remember_choice && (response == AdbdClient::PKResponse::ALLOW)) {
                     write_public_key(m_req->public_key);
+                }
 
                 m_response = response;
 
-- 
cgit v1.2.3