Merge lp:~ted/indicator-appmenu/lp934429 into lp:indicator-appmenu/0.4

[Ted Gould]

Ensuring we always have values for GVariant

[Lars Karlitski (larsu)]

I'm unsure that this fixes the bug. The stacktrace shows g_variant_build_add_value crashing because receiving a NULL value, which could be

  app,
  db,
  dbus_address,
  dbus_path or
  dbus_id'

This patch only does NULL checks for 'display', 'app_icon' and 'item_icon'.

Am I missing something? (Disclaimer: I haven't reproduced the bug or tested this patch)

Btw, this should be fixed in glib, so that passing a NULL value will at least throw a fatal error.

review: Needs Information

[Ted Gould (ted)]

On Tue, 2012-02-21 at 08:25 +0000, Lars Uebernickel wrote:
> I'm unsure that this fixes the bug. The stacktrace shows
> g_variant_build_add_value crashing because receiving a NULL value, which
> could be
>
> app,
> db,
> dbus_address,
> dbus_path or
> dbus_id'
>
> This patch only does NULL checks for 'display', 'app_icon' and 'item_icon'.
>
> Am I missing something? (Disclaimer: I haven't reproduced the bug or tested
> this patch)

Yes :-) The line number in the stack trace. The dbus paths can't be
NULL because the other code depends on them to find the menus. So
they're already guaranteed to have a value.

> Btw, this should be fixed in glib, so that passing a NULL value will
> at least throw a fatal error.

I agree.

[Lars Karlitski (larsu)]

*whistles*

review: Approve

=== modified file 'src/hud-search.c'
--- src/hud-search.c	2012-02-15 15:53:59 +0000
+++ src/hud-search.c	2012-02-20 18:21:17 +0000
@@ -648,6 +648,12 @@
 {
 	HudSearchSuggest * suggest = g_new0(HudSearchSuggest, 1);
 
+	/* Ensure that the values are at least set to the NULL string for
+	   DBus even if we don't have values for them */
+	if (display == NULL) display = "";
+	if (app_icon == NULL) app_icon = "";
+	if (item_icon == NULL) item_icon = "";
+
 	suggest->display = g_strdup(display);
 	suggest->app_icon = g_strdup(app_icon);
 	suggest->item_icon = g_strdup(item_icon);