← Back to team overview

ayatana-commits team mailing list archive

Re: [Merge] lp:~bratsche/appmenu-gtk/rebuild-ids-destruction into lp:appmenu-gtk

 

Review: Approve
  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);
> +    }
>  }
> 



-- 
https://code.launchpad.net/~bratsche/appmenu-gtk/rebuild-ids-destruction/+merge/34271
Your team ayatana-commits is subscribed to branch lp:appmenu-gtk.



References