[gnome-software] Prevent memory corruption when doing gs_plugin_loader_dedupe() more than once

[Richard Hughes <rhughes src gnome org>]

commit 155b2623fb9acff21914dc99cf0bb3926d1b9a83
Author: Richard Hughes <richard hughsie com>
Date:   Tue Oct 8 18:02:04 2013 +0100

    Prevent memory corruption when doing gs_plugin_loader_dedupe() more than once
    
    We were not checking if the GsApp was already in the cache and matching exactly,
    which caused memory corruption. Also refactor the method to make things clearer.

 src/gs-plugin-loader.c |   83 +++++++++++++++++++++++++-----------------------
 src/gs-self-test.c     |    3 ++
 2 files changed, 46 insertions(+), 40 deletions(-)
---
diff --git a/src/gs-plugin-loader.c b/src/gs-plugin-loader.c
index facbb6a..b94dc38 100644
--- a/src/gs-plugin-loader.c
+++ b/src/gs-plugin-loader.c
@@ -101,51 +101,54 @@ gs_plugin_loader_dedupe (GsPluginLoader *plugin_loader, GsApp *app)
 
        /* already exists */
        new_app = g_hash_table_lookup (priv->app_cache, gs_app_get_id (app));
-       if (new_app != NULL) {
-               /* an [updatable] installable package is more information than
-                * just the fact that something is installed */
-               if (gs_app_get_state (app) == GS_APP_STATE_UPDATABLE &&
-                   gs_app_get_state (new_app) == GS_APP_STATE_INSTALLED) {
-                       /* we have to do the little dance to appease the
-                        * angry gnome controlling the state-machine */
-                       gs_app_set_state (new_app, GS_APP_STATE_UNKNOWN);
-                       gs_app_set_state (new_app, GS_APP_STATE_UPDATABLE);
-               }
-
-               /* save any properties we already know */
-               if (gs_app_get_source (app) != NULL)
-                       gs_app_set_source (new_app, gs_app_get_source (app));
-               if (gs_app_get_project_group (app) != NULL)
-                       gs_app_set_project_group (new_app, gs_app_get_project_group (app));
-               if (gs_app_get_name (app) != NULL)
-                       gs_app_set_name (new_app, gs_app_get_name (app));
-               if (gs_app_get_summary (app) != NULL)
-                       gs_app_set_summary (new_app, gs_app_get_summary (app));
-               if (gs_app_get_description (app) != NULL)
-                       gs_app_set_description (new_app, gs_app_get_description (app));
-               if (gs_app_get_update_details (app) != NULL)
-                       gs_app_set_update_details (new_app, gs_app_get_update_details (app));
-               if (gs_app_get_update_version (app) != NULL)
-                       gs_app_set_update_version (new_app, gs_app_get_update_version (app));
-               if (gs_app_get_pixbuf (app) != NULL)
-                       gs_app_set_pixbuf (new_app, gs_app_get_pixbuf (app));
-
-               /* this looks a little odd to unref the method parameter,
-                * but it allows us to do:
-                * app = gs_plugin_loader_dedupe (cache, app);
-                */
-               g_object_unref (app);
-               g_object_ref (new_app);
+       if (new_app == app) {
+               new_app = app;
                goto out;
        }
 
        /* insert new entry */
-       g_hash_table_insert (priv->app_cache,
-                            g_strdup (gs_app_get_id (app)),
-                            g_object_ref (app));
+       if (new_app == NULL) {
+               new_app = app;
+               g_hash_table_insert (priv->app_cache,
+                                    g_strdup (gs_app_get_id (app)),
+                                    g_object_ref (app));
+               goto out;
+       }
+
+       /* an [updatable] installable package is more information than
+        * just the fact that something is installed */
+       if (gs_app_get_state (app) == GS_APP_STATE_UPDATABLE &&
+           gs_app_get_state (new_app) == GS_APP_STATE_INSTALLED) {
+               /* we have to do the little dance to appease the
+                * angry gnome controlling the state-machine */
+               gs_app_set_state (new_app, GS_APP_STATE_UNKNOWN);
+               gs_app_set_state (new_app, GS_APP_STATE_UPDATABLE);
+       }
 
-       /* no ref */
-       new_app = app;
+       /* save any properties we already know */
+       if (gs_app_get_source (app) != NULL)
+               gs_app_set_source (new_app, gs_app_get_source (app));
+       if (gs_app_get_project_group (app) != NULL)
+               gs_app_set_project_group (new_app, gs_app_get_project_group (app));
+       if (gs_app_get_name (app) != NULL)
+               gs_app_set_name (new_app, gs_app_get_name (app));
+       if (gs_app_get_summary (app) != NULL)
+               gs_app_set_summary (new_app, gs_app_get_summary (app));
+       if (gs_app_get_description (app) != NULL)
+               gs_app_set_description (new_app, gs_app_get_description (app));
+       if (gs_app_get_update_details (app) != NULL)
+               gs_app_set_update_details (new_app, gs_app_get_update_details (app));
+       if (gs_app_get_update_version (app) != NULL)
+               gs_app_set_update_version (new_app, gs_app_get_update_version (app));
+       if (gs_app_get_pixbuf (app) != NULL)
+               gs_app_set_pixbuf (new_app, gs_app_get_pixbuf (app));
+
+       /* this looks a little odd to unref the method parameter,
+        * but it allows us to do:
+        * app = gs_plugin_loader_dedupe (cache, app);
+        */
+       g_object_unref (app);
+       g_object_ref (new_app);
 out:
        g_mutex_unlock (&plugin_loader->priv->app_cache_mutex);
        return new_app;
diff --git a/src/gs-self-test.c b/src/gs-self-test.c
index a8c9e1a..ed76a30 100644
--- a/src/gs-self-test.c
+++ b/src/gs-self-test.c
@@ -142,6 +142,9 @@ gs_plugin_loader_dedupe_func (void)
        app2 = gs_plugin_loader_dedupe (loader, app2);
        g_assert_cmpstr (gs_app_get_id (app2), ==, "app1");
        g_assert_cmpstr (gs_app_get_description (app2), ==, "description");
+       app2 = gs_plugin_loader_dedupe (loader, app2);
+       g_assert_cmpstr (gs_app_get_id (app2), ==, "app1");
+       g_assert_cmpstr (gs_app_get_description (app2), ==, "description");
 
        g_object_unref (app1);
        g_object_unref (app2);