gst-plugins-base: discoverer: fix potential tag list leaks

Tim Müller tpm at kemper.freedesktop.org
Tue Jan 10 10:46:56 PST 2012


Module: gst-plugins-base
Branch: master
Commit: 5f20af6ce537c2b079ca866ccd5dea589ea1be5f
URL:    http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=5f20af6ce537c2b079ca866ccd5dea589ea1be5f

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