[0.10] gst-plugins-base: discoverer: fix potential tag list leaks

Tim Müller tpm at kemper.freedesktop.org
Thu Jan 12 09:48:35 PST 2012


Module: gst-plugins-base
Branch: 0.10
Commit: 636a458e960349f93b2bca357b3115ed94241bbe
URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=636a458e960349f93b2bca357b3115ed94241bbe

Author: Tim-Philipp Müller <tim.muller at collabora.co.uk>
Date:   Tue Jan 10 18:07:19 2012 +0000

discoverer: fix potential tag list leaks

Not that I have ever seen these in practice, but if they
can't happen we may just as well just assign the new tag
list. Merge properly to be on the safe side, and also
avoid a useless tag list copy in the normal case where
there is no tag list yet.

---

 gst-libs/gst/pbutils/gstdiscoverer.c |   40 +++++++++++++++++++++------------
 1 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/gst-libs/gst/pbutils/gstdiscoverer.c b/gst-libs/gst/pbutils/gstdiscoverer.c
index a7c4968..6291cd7 100644
--- a/gst-libs/gst/pbutils/gstdiscoverer.c
+++ b/gst-libs/gst/pbutils/gstdiscoverer.c
@@ -607,6 +607,23 @@ collect_stream_information (GstDiscoverer * dc, PrivateStream * ps, guint idx)
   return st;
 }
 
+/* takes ownership of new_tags, may replace *taglist with a new one */
+static void
+gst_discoverer_merge_and_replace_tags (GstTagList ** taglist,
+    GstTagList * new_tags)
+{
+  if (new_tags == NULL)
+    return;
+
+  if (*taglist == NULL) {
+    *taglist = new_tags;
+    return;
+  }
+
+  gst_tag_list_insert (*taglist, new_tags, GST_TAG_MERGE_REPLACE);
+  gst_tag_list_free (new_tags);
+}
+
 /* Parses a set of caps and tags in st and populates a GstDiscovererStreamInfo
  * structure (parent, if !NULL, otherwise it allocates one)
  */
@@ -664,10 +681,8 @@ collect_information (GstDiscoverer * dc, const GstStructure * st,
         info->max_bitrate = utmp;
 
       /* FIXME: Is it worth it to remove the tags we've parsed? */
-      info->parent.tags = gst_tag_list_merge (info->parent.tags,
-          (GstTagList *) tags_st, GST_TAG_MERGE_REPLACE);
-
-      gst_structure_free (tags_st);
+      gst_discoverer_merge_and_replace_tags (&info->parent.tags,
+          (GstTagList *) tags_st);
     }
 
     if (!info->language && ((GstDiscovererStreamInfo *) info)->tags) {
@@ -728,9 +743,8 @@ collect_information (GstDiscoverer * dc, const GstStructure * st,
         info->max_bitrate = utmp;
 
       /* FIXME: Is it worth it to remove the tags we've parsed? */
-      info->parent.tags = gst_tag_list_merge (info->parent.tags,
-          (GstTagList *) tags_st, GST_TAG_MERGE_REPLACE);
-      gst_structure_free (tags_st);
+      gst_discoverer_merge_and_replace_tags (&info->parent.tags,
+          (GstTagList *) tags_st);
     }
 
     return (GstDiscovererStreamInfo *) info;
@@ -757,10 +771,8 @@ collect_information (GstDiscoverer * dc, const GstStructure * st,
         info->language = g_strdup (language);
 
       /* FIXME: Is it worth it to remove the tags we've parsed? */
-      info->parent.tags = gst_tag_list_merge (info->parent.tags,
-          (GstTagList *) tags_st, GST_TAG_MERGE_REPLACE);
-      gst_structure_free (tags_st);
-
+      gst_discoverer_merge_and_replace_tags (&info->parent.tags,
+          (GstTagList *) tags_st);
     }
 
     if (!info->language && ((GstDiscovererStreamInfo *) info)->tags) {
@@ -787,9 +799,8 @@ collect_information (GstDiscoverer * dc, const GstStructure * st,
 
     if (gst_structure_id_get (st, _TAGS_QUARK,
             GST_TYPE_STRUCTURE, &tags_st, NULL)) {
-      info->tags = gst_tag_list_merge (info->tags, (GstTagList *) tags_st,
-          GST_TAG_MERGE_REPLACE);
-      gst_structure_free (tags_st);
+      gst_discoverer_merge_and_replace_tags (&info->tags,
+          (GstTagList *) tags_st);
     }
 
     return info;
@@ -1197,7 +1208,6 @@ handle_current_sync (GstDiscoverer * dc)
       done = handle_message (dc, msg);
       gst_message_unref (msg);
     }
-
   } while (!done && (g_timer_elapsed (timer, NULL) < deadline));
 
   /* return result */



More information about the gstreamer-commits mailing list