From 06972db7b6a55f951833327a659afc76bef6b5c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim-Philipp=20M=C3=BCller?= Date: Mon, 21 Feb 2011 00:55:49 +0000 Subject: [PATCH] icydemux: fix tag list handling issues that might have caused crashes Fix slightly confused tag handling in some places: make it clear when we're taking ownership of a tag list and when not. For example, gst_icydemux_tag_found() was taking ownership when the source pad existed, but otherwise not (leak). Also, gst_event_parse_tag() does not return a newly-allocated taglist, but a tag list that belongs to the tag event, so don't give ownership of it away. While we're at it, some minor clean-ups: don't re-invent g_strndup() and simplify gst_icydemux_parse_and_send_tags() a bit, and don't leak the tag list in case no valid tags where found. https://bugzilla.gnome.org/show_bug.cgi?id=641330 --- gst/icydemux/gsticydemux.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-) diff --git a/gst/icydemux/gsticydemux.c b/gst/icydemux/gsticydemux.c index 5965500560..25829598fa 100644 --- a/gst/icydemux/gsticydemux.c +++ b/gst/icydemux/gsticydemux.c @@ -301,6 +301,7 @@ gst_icydemux_unicodify (const gchar * str) return gst_tag_freeform_string_to_utf8 (str, -1, env_vars); } +/* takes ownership of tag list */ static gboolean gst_icydemux_tag_found (GstICYDemux * icydemux, GstTagList * tags) { @@ -309,10 +310,13 @@ gst_icydemux_tag_found (GstICYDemux * icydemux, GstTagList * tags) return gst_icydemux_send_tag_event (icydemux, tags); /* if we haven't a source pad yet, cache the tags */ - if (!icydemux->cached_tags) - icydemux->cached_tags = gst_tag_list_new (); - - gst_tag_list_insert (icydemux->cached_tags, tags, GST_TAG_MERGE_REPLACE_ALL); + if (!icydemux->cached_tags) { + icydemux->cached_tags = tags; + } else { + gst_tag_list_insert (icydemux->cached_tags, tags, + GST_TAG_MERGE_REPLACE_ALL); + gst_tag_list_free (tags); + } return TRUE; } @@ -320,12 +324,11 @@ gst_icydemux_tag_found (GstICYDemux * icydemux, GstTagList * tags) static void gst_icydemux_parse_and_send_tags (GstICYDemux * icydemux) { - GstTagList *tags = gst_tag_list_new (); + GstTagList *tags; const guint8 *data; int length, i; gchar *buffer; gchar **strings; - gboolean found_tag = FALSE; length = gst_adapter_available (icydemux->meta_adapter); @@ -333,10 +336,9 @@ gst_icydemux_parse_and_send_tags (GstICYDemux * icydemux) /* Now, copy this to a buffer where we can NULL-terminate it to make things * a bit easier, then do that parsing. */ - buffer = g_malloc (length + 1); - memcpy (buffer, data, length); - buffer[length] = 0; + buffer = g_strndup ((const gchar *) data, length); + tags = gst_tag_list_new (); strings = g_strsplit (buffer, "';", 0); for (i = 0; strings[i]; i++) { @@ -347,7 +349,6 @@ gst_icydemux_parse_and_send_tags (GstICYDemux * icydemux) gst_tag_list_add (tags, GST_TAG_MERGE_REPLACE, GST_TAG_TITLE, title, NULL); g_free (title); - found_tag = TRUE; } } else if (!g_ascii_strncasecmp (strings[i], "StreamUrl=", 10)) { char *url = gst_icydemux_unicodify (strings[i] + 11); @@ -356,7 +357,6 @@ gst_icydemux_parse_and_send_tags (GstICYDemux * icydemux) gst_tag_list_add (tags, GST_TAG_MERGE_REPLACE, GST_TAG_HOMEPAGE, url, NULL); g_free (url); - found_tag = TRUE; } } } @@ -365,8 +365,10 @@ gst_icydemux_parse_and_send_tags (GstICYDemux * icydemux) g_free (buffer); gst_adapter_clear (icydemux->meta_adapter); - if (found_tag) + if (!gst_tag_list_is_empty (tags)) gst_icydemux_tag_found (icydemux, tags); + else + gst_tag_list_free (tags); } static gboolean @@ -379,7 +381,7 @@ gst_icydemux_handle_event (GstPad * pad, GstEvent * event) GstTagList *tags; gst_event_parse_tag (event, &tags); - result = gst_icydemux_tag_found (icydemux, tags); + result = gst_icydemux_tag_found (icydemux, gst_tag_list_copy (tags)); gst_event_unref (event); return result; } @@ -617,6 +619,7 @@ gst_icydemux_change_state (GstElement * element, GstStateChange transition) return ret; } +/* takes ownership of tag list */ static gboolean gst_icydemux_send_tag_event (GstICYDemux * icydemux, GstTagList * tags) {