[libdmapsharing] Fix some memory management

[W. Michael Petullo <wmpetullo src gnome org>]

commit dfdc3adb23c40cf91e8094bfcc190333d52b65d3
Author: W. Michael Petullo <mike flyn org>
Date:   Sat May 11 18:10:47 2019 -0400

    Fix some memory management
    
    Signed-off-by: W. Michael Petullo <mike flyn org>

 libdmapsharing/dmap-av-share.c       |  3 ---
 libdmapsharing/dmap-share.c          | 39 +++++++++++++++----------------
 libdmapsharing/test-dmap-av-record.c | 29 ++++++++++++++---------
 libdmapsharing/test-dmap-db.c        | 27 +++++++++++++++-------
 tests/test-dmap-server.c             | 45 +++++++++++++++++++++++++++---------
 5 files changed, 90 insertions(+), 53 deletions(-)
---
diff --git a/libdmapsharing/dmap-av-share.c b/libdmapsharing/dmap-av-share.c
index b24ab23..99534ca 100644
--- a/libdmapsharing/dmap-av-share.c
+++ b/libdmapsharing/dmap-av-share.c
@@ -102,9 +102,6 @@ dmap_av_share_new (const char *name,
                    DmapContainerDb * container_db,
                    gchar * transcode_mimetype)
 {
-       g_object_ref (db);
-       g_object_ref (container_db);
-
        return DMAP_AV_SHARE (g_object_new (DMAP_TYPE_AV_SHARE,
                                           "name", name,
                                           "password", password,
diff --git a/libdmapsharing/dmap-share.c b/libdmapsharing/dmap-share.c
index 8055184..769e107 100644
--- a/libdmapsharing/dmap-share.c
+++ b/libdmapsharing/dmap-share.c
@@ -240,8 +240,6 @@ dmap_share_serve (DmapShare *share, GError **error)
        gboolean ret;
        GError *error2 = NULL;
 
-       share->priv->server = soup_server_new (NULL, NULL);
-
        password_required = (share->priv->auth_method != DMAP_SHARE_AUTH_METHOD_NONE);
 
        if (password_required) {
@@ -315,11 +313,6 @@ dmap_share_serve (DmapShare *share, GError **error)
 
        g_debug ("Started DMAP server on port %u", share->priv->port);
 
-       /* using direct since there is no g_uint_hash or g_uint_equal */
-       share->priv->session_ids =
-               g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL,
-                                      g_free);
-
        share->priv->server_active = TRUE;
 
        ok = TRUE;
@@ -339,13 +332,10 @@ _server_stop (DmapShare * share)
 
        if (share->priv->server) {
                soup_server_disconnect (share->priv->server);
-               g_object_unref (share->priv->server);
-               share->priv->server = NULL;
        }
 
        if (share->priv->session_ids) {
-               g_hash_table_destroy (share->priv->session_ids);
-               share->priv->session_ids = NULL;
+               g_hash_table_remove_all (share->priv->session_ids);
        }
 
        share->priv->server_active = FALSE;
@@ -569,10 +559,18 @@ _dispose (GObject * object)
 {
        DmapShare *share = DMAP_SHARE (object);
 
-       g_clear_object (&share->priv->db);
-       g_clear_object (&share->priv->container_db);
+       if (share->priv->published) {
+               _publish_stop (share);
+       }
+
+       if (share->priv->server_active) {
+               _server_stop (share);
+       }
+
        g_clear_object (&share->priv->publisher);
        g_clear_object (&share->priv->server);
+       g_clear_object (&share->priv->db);
+       g_clear_object (&share->priv->container_db);
 
        G_OBJECT_CLASS (dmap_share_parent_class)->dispose (object);
 }
@@ -584,13 +582,8 @@ _finalize (GObject * object)
 
        g_debug ("Finalizing DmapShare");
 
-       if (share->priv->published) {
-               _publish_stop (share);
-       }
-
-       if (share->priv->server_active) {
-               _server_stop (share);
-       }
+       g_hash_table_destroy (share->priv->session_ids);
+       share->priv->session_ids = NULL;
 
        g_free (share->priv->name);
        g_free (share->priv->password);
@@ -738,6 +731,12 @@ dmap_share_init (DmapShare * share)
        share->priv->revision_number = 5;
        share->priv->auth_method = DMAP_SHARE_AUTH_METHOD_NONE;
        share->priv->publisher = dmap_mdns_publisher_new ();
+       share->priv->server = soup_server_new (NULL, NULL);
+
+       /* using direct since there is no g_uint_hash or g_uint_equal */
+       share->priv->session_ids =
+               g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL,
+                                      g_free);
 
        g_signal_connect_object (share->priv->publisher,
                                 "published",
diff --git a/libdmapsharing/test-dmap-av-record.c b/libdmapsharing/test-dmap-av-record.c
index f9afc0a..5ca72fa 100644
--- a/libdmapsharing/test-dmap-av-record.c
+++ b/libdmapsharing/test-dmap-av-record.c
@@ -274,6 +274,7 @@ test_dmap_av_record_init (TestDmapAvRecord *record)
        record->priv = TEST_DMAP_AV_RECORD_GET_PRIVATE (record);
 }
 
+static void test_dmap_av_record_dispose  (GObject *object);
 static void test_dmap_av_record_finalize (GObject *object);
 
 static void
@@ -283,6 +284,7 @@ test_dmap_av_record_class_init (TestDmapAvRecordClass *klass)
 
        gobject_class->set_property = test_dmap_av_record_set_property;
         gobject_class->get_property = test_dmap_av_record_get_property;
+        gobject_class->dispose      = test_dmap_av_record_dispose;
         gobject_class->finalize     = test_dmap_av_record_finalize;
 
         g_object_class_override_property (gobject_class, PROP_LOCATION, "location");
@@ -339,6 +341,19 @@ G_DEFINE_TYPE_WITH_CODE (TestDmapAvRecord, test_dmap_av_record, G_TYPE_OBJECT,
                          G_IMPLEMENT_INTERFACE (DMAP_TYPE_RECORD, _dmap_record_iface_init)
                          G_ADD_PRIVATE (TestDmapAvRecord))
 
+static void
+test_dmap_av_record_dispose (GObject *object)
+{
+       TestDmapAvRecord *record = TEST_DMAP_AV_RECORD (object);
+
+       if (record->priv->hash) {
+               g_array_unref(record->priv->hash);
+       }
+       record->priv->hash = NULL;
+
+       G_OBJECT_CLASS (test_dmap_av_record_parent_class)->dispose (object);
+}
+
 static void
 test_dmap_av_record_finalize (GObject *object)
 {
@@ -354,10 +369,6 @@ test_dmap_av_record_finalize (GObject *object)
        g_free (record->priv->sort_artist);
        g_free (record->priv->genre);
 
-       if (record->priv->hash) {
-               g_array_unref(record->priv->hash);
-       }
-
        G_OBJECT_CLASS (test_dmap_av_record_parent_class)->finalize (object);
 }
 
@@ -374,19 +385,14 @@ TestDmapAvRecord *test_dmap_av_record_new (void)
        g_free (dir);
 
        record->priv->title = g_strdup ("Unknown");
-
        record->priv->album = g_strdup ("Unknown");
-
+       record->priv->sort_album = g_strdup ("Unknown");
        record->priv->artist = g_strdup ("Unknown");
-
+       record->priv->sort_artist = g_strdup ("Unknown");
        record->priv->bitrate = 128;
-
        record->priv->firstseen = 1;
-
        record->priv->mtime = 1;
-
        record->priv->disc = 1;
-
        record->priv->genre = g_strdup ("Unknown");
 
        ext = strrchr (record->priv->location, '.');
@@ -396,6 +402,7 @@ TestDmapAvRecord *test_dmap_av_record_new (void)
                ext++;
        }
        record->priv->format = g_strdup (ext);
+       record->priv->real_format = g_strdup (ext);
 
        record->priv->filesize = 33729;
 
diff --git a/libdmapsharing/test-dmap-db.c b/libdmapsharing/test-dmap-db.c
index 07fe760..2586c4b 100644
--- a/libdmapsharing/test-dmap-db.c
+++ b/libdmapsharing/test-dmap-db.c
@@ -32,8 +32,7 @@ test_dmap_db_lookup_by_id (const DmapDb *db, guint id)
 {
        DmapRecord *record;
        record = g_hash_table_lookup (TEST_DMAP_DB (db)->priv->db, GUINT_TO_POINTER (id));
-       g_object_ref (record);
-       return record;
+       return g_object_ref(record);
 }
 
 static void
@@ -64,7 +63,7 @@ static void
 test_dmap_db_init (TestDmapDb *db)
 {
        db->priv = TEST_DMAP_DB_GET_PRIVATE (db);
-       db->priv->db = g_hash_table_new (g_direct_hash, g_direct_equal);
+       db->priv->db = g_hash_table_new_full (g_direct_hash, g_direct_equal, NULL, g_object_unref);
 
        /* Media ID's start at max and go down.
         * Container ID's start at 1 and go up.
@@ -72,11 +71,6 @@ test_dmap_db_init (TestDmapDb *db)
        db->priv->nextid = G_MAXINT;
 }
 
-static void
-test_dmap_db_class_init (TestDmapDbClass *klass)
-{
-}
-
 static void
 _dmap_db_iface_init (gpointer iface, gpointer data)
 {
@@ -95,6 +89,23 @@ G_DEFINE_TYPE_WITH_CODE (TestDmapDb, test_dmap_db, G_TYPE_OBJECT,
                                                 _dmap_db_iface_init)
                          G_ADD_PRIVATE (TestDmapDb))
 
+static void
+_finalize(GObject *object)
+{
+       TestDmapDb *db = TEST_DMAP_DB(object);
+       g_hash_table_destroy(db->priv->db);
+
+       G_OBJECT_CLASS (test_dmap_db_parent_class)->finalize (object);
+}
+
+static void
+test_dmap_db_class_init (TestDmapDbClass *klass)
+{
+       GObjectClass *gobject_class = G_OBJECT_CLASS (klass);
+
+       gobject_class->finalize = _finalize;
+}
+
 TestDmapDb *
 test_dmap_db_new (void)
 {
diff --git a/tests/test-dmap-server.c b/tests/test-dmap-server.c
index 36e2bb0..54636fb 100644
--- a/tests/test-dmap-server.c
+++ b/tests/test-dmap-server.c
@@ -57,13 +57,20 @@ error_cb(DmapShare *share, GError *error, gpointer user_data)
        g_error("%s", error->message);
 }
 
-static void
-create_share (guint conn_type)
+static gboolean
+_quit(gpointer user_data)
+{
+       g_main_loop_quit(user_data);
+       return FALSE;
+}
+
+static DmapShare *
+create_share (guint conn_type, GMainLoop *loop)
 {
        char *name = dmap_sharing_default_share_name ();
        DmapContainerRecord *dmap_container_record = \
                DMAP_CONTAINER_RECORD (test_dmap_container_record_new ());
-       DmapContainerDb *dmap_container_db = \
+       DmapContainerDb *container_db = \
                DMAP_CONTAINER_DB (test_dmap_container_db_new
                                        (dmap_container_record));
        DmapRecordFactory *factory;
@@ -73,11 +80,15 @@ create_share (guint conn_type)
        gboolean ok;
        DmapDb *db;
 
-       if (conn_type == DAAP) { 
+       switch (conn_type) {
+       default:
+               g_idle_add(_quit, loop);
+       case DAAP:
                factory = DMAP_RECORD_FACTORY (test_dmap_av_record_factory_new ());
-
-       } else {
+               break;
+       case DPAP:
                factory = DMAP_RECORD_FACTORY (test_dmap_image_record_factory_new ());
+               break;
        }
 
        record = DMAP_RECORD (dmap_record_factory_create (factory, NULL, NULL));
@@ -87,18 +98,22 @@ create_share (guint conn_type)
 
        g_warning ("initialize DAAP sharing");
 
-       if (conn_type == DAAP) {
+       switch (conn_type) {
+       default:
+       case DAAP:
                share = DMAP_SHARE (dmap_av_share_new (name,
                                                        NULL,
                                                        db,
-                                                       dmap_container_db,
+                                                       container_db,
                                                       NULL));
-       } else {
+               break;
+       case DPAP:
                share = DMAP_SHARE (dmap_image_share_new (name,
                                                           NULL,
                                                           db,
-                                                          dmap_container_db,
+                                                          container_db,
                                                           NULL));
+               break;
        }
 
        g_assert (NULL != share);
@@ -115,12 +130,18 @@ create_share (guint conn_type)
                g_error("Error publishing server: %s", error->message);
        }
 
+       g_object_unref (factory);
+       g_object_unref (container_db);
+       g_object_unref (db);
        g_free (name);
+
+       return share;
 }
 
 int
 main (int argc, char *argv[])
 {
+       DmapShare *share = NULL;
        guint conn_type = DAAP;
        static GMainLoop *loop;
 
@@ -129,9 +150,11 @@ main (int argc, char *argv[])
 
        loop = g_main_loop_new (NULL, FALSE);
 
-       create_share (conn_type);
+       share = create_share (conn_type, loop);
 
        g_main_loop_run (loop);
 
+       g_object_unref(share);
+
        exit(EXIT_SUCCESS);
 }