Code review comment for lp:~ted/indicator-appmenu/lp934429

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.