Commit 06972db7 authored by Tim-Philipp Müller's avatar Tim-Philipp Müller

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
parent 8e6b876e
......@@ -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)
{
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment