Code review comment for lp:~bratsche/appmenu-gtk/rebuild-ids-destruction

  review approve

This looks fine, but I'm curious:

I'm guessing the bug was caused by rebuild_ids not existing somewhere,
do you think we should still check for it's existence where it's used?

On Wed, 2010-09-01 at 03:24 +0000, Cody Russell wrote:
> Cody Russell has proposed merging lp:~bratsche/appmenu-gtk/rebuild-ids-destruction into lp:appmenu-gtk.
>
> Requested reviews:
> Canonical Desktop Experience Team (canonical-dx-team)
> Related bugs:
> #624607 gnome-display-properties crashed with SIGFPE in g_hash_table_lookup()
> https://bugs.launchpad.net/bugs/624607
>
> differences between files attachment (review-diff.txt)
> === modified file 'src/bridge.c'
> --- src/bridge.c 2010-08-30 21:31:56 +0000
> +++ src/bridge.c 2010-09-01 03:24:45 +0000
> @@ -970,10 +970,6 @@
> data->widget);
>
> g_hash_table_remove (rebuild_ids, data->widget);
> - if (g_hash_table_size (rebuild_ids) == 0)
> - {
> - g_hash_table_destroy (rebuild_ids);
> - }
> }
>
> g_free (data);
> @@ -1296,4 +1292,8 @@
> G_MODULE_EXPORT void
> menu_proxy_module_unload (UbuntuMenuProxyModule *module)
> {
> + if (rebuild_ids)
> + {
> + g_hash_table_destroy (rebuild_ids);
> + }
> }
>