Merge lp:~mterry/nux/null-ibus-config into lp:nux

[Michael Terry]

Gracefully handle ibus_bus_get_config returning NULL

ibus_bus_get_config can return NULL in two cases:
1) !ibus_bus_is_connected
2) The configuration daemon at dbus name org.freedesktop.IBus.Config is
   not available

The current nux code does not gracefully handle NULL being returned, and
crashes, as can be seen in bug 1047944.

[Tim Penhey (thumper)]

Can I get you to add a comment in the code just before the if statement saying why it may be null? Just like you did in the description. That'd help anyone coming to this code later.

Cheers,
Tim

review: Needs Fixing

[lp:~mterry/nux/null-ibus-config]

688. By Michael Terry on 2012-11-15

add some comments explaining how ibus config might be null

689. By Michael Terry on 2012-11-15

merge from trunk

[Michael Terry (mterry)]

Done. Please reconsider, thanks!

[PS Jenkins bot (ps-jenkins)]

FAILED: Continuous integration, rev:689
No commit message was specified in the merge proposal. Click on the following link and set the commit message (if you want a jenkins rebuild you need to trigger it yourself):
https://code.launchpad.net/~mterry/nux/null-ibus-config/+merge/129212/+edit-commit-message

http://jenkins.qa.ubuntu.com/job/nux-ci/9/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/nux-raring-amd64-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/nux-raring-armhf-ci/9
    SUCCESS: http://jenkins.qa.ubuntu.com/job/nux-raring-i386-ci/9

Click here to trigger a rebuild:
http://s-jenkins:8080/job/nux-ci/9/rebuild

review: Needs Fixing (continuous-integration)

[Michael Terry (mterry)]

poke

[Brandon Schaefer (brandontschaefer)]

Sorry I missed this... It now has conflicts, but it does indeed handle that bug :). So i've made a new branch just get this fix in here:

https://code.launchpad.net/~brandontschaefer/nux/lp.1047944-fix/+merge/186121

Thanks again!

Unmerged revisions

689. By Michael Terry on 2012-11-15

merge from trunk

688. By Michael Terry on 2012-11-15

add some comments explaining how ibus config might be null

687. By Michael Terry on 2012-10-11

handle NULL return from ibus_bus_get_config

=== modified file 'Nux/InputMethodIBus.cpp'
--- Nux/InputMethodIBus.cpp	2012-10-01 23:37:25 +0000
+++ Nux/InputMethodIBus.cpp	2012-11-15 23:38:21 +0000
@@ -165,8 +165,11 @@
     UpdateCursorLocation();
 
     IBusConfig* bus_conf = ibus_bus_get_config(bus_);
-    g_signal_handlers_disconnect_by_func(bus_conf, reinterpret_cast<gpointer>(OnConfigChanged_), this);
-    g_signal_connect(bus_conf, "value-changed", G_CALLBACK(OnConfigChanged_), this);
+    if (bus_conf) // may be null if not connected to bus or can't get ibus name
+    {
+      g_signal_handlers_disconnect_by_func(bus_conf, reinterpret_cast<gpointer>(OnConfigChanged_), this);
+      g_signal_connect(bus_conf, "value-changed", G_CALLBACK(OnConfigChanged_), this);
+    }
     UpdateHotkeys();
   }
 
@@ -177,7 +180,8 @@
     if (ibus_bus_is_connected(bus_))
     {
       IBusConfig* bus_conf = ibus_bus_get_config(bus_);
-      g_signal_handlers_disconnect_by_func(bus_conf, reinterpret_cast<gpointer>(OnConfigChanged_), this);
+      if (bus_conf) // may be null if not connected to bus or can't get ibus name
+        g_signal_handlers_disconnect_by_func(bus_conf, reinterpret_cast<gpointer>(OnConfigChanged_), this);
     }
 
     if (!context_)
@@ -484,6 +488,9 @@
   void IBusIMEContext::UpdateHotkeys()
   {
     IBusConfig* conf = ibus_bus_get_config(bus_);
+    if (!conf) // may be null if not connected to bus or can't get ibus name
+      return;
+
     GVariant* val = ibus_config_get_value(conf, "general", "hotkey/trigger");
     const gchar** keybindings = g_variant_get_strv(val, NULL);