playlist/{asx,rss,xspf}: use Expat instead of GLib to parse XML

Makefile.am

@@ -1016,11 +1016,13 @@ libplaylist_plugins_a_SOURCES = \
 	src/playlist/EmbeddedCuePlaylistPlugin.hxx \
 	src/PlaylistRegistry.cxx src/PlaylistRegistry.hxx
 libplaylist_plugins_a_CPPFLAGS = $(AM_CPPFLAGS) \
+	$(EXPAT_CFLAGS) \
 	$(YAJL_CFLAGS) \
 	$(patsubst -I%/FLAC,-I%,$(FLAC_CFLAGS))
 
 PLAYLIST_LIBS = \
 	libplaylist_plugins.a \
+	$(EXPAT_LIBS) \
 	$(FLAC_LIBS)
 
 if ENABLE_DESPOTIFY
@@ -1036,10 +1038,9 @@ libplaylist_plugins_a_SOURCES += \
 PLAYLIST_LIBS += $(YAJL_LIBS)
 endif
 
-if HAVE_GLIB
+if HAVE_EXPAT
 libplaylist_plugins_a_SOURCES += \
-	src/playlist/PlsPlaylistPlugin.cxx \
+	src/Expat.cxx src/Expat.hxx \
-	src/playlist/PlsPlaylistPlugin.hxx \
 	src/playlist/XspfPlaylistPlugin.cxx \
 	src/playlist/XspfPlaylistPlugin.hxx \
 	src/playlist/AsxPlaylistPlugin.cxx \
@@ -1048,6 +1049,12 @@ libplaylist_plugins_a_SOURCES += \
 	src/playlist/RssPlaylistPlugin.hxx
 endif
 
+if HAVE_GLIB
+libplaylist_plugins_a_SOURCES += \
+	src/playlist/PlsPlaylistPlugin.cxx \
+	src/playlist/PlsPlaylistPlugin.hxx
+endif
+
 #
 # Filter plugins
 #

configure.ac

@@ -229,6 +229,11 @@ AC_ARG_ENABLE(libmpdclient,
 		[enable support for the MPD client]),,
 	enable_libmpdclient=auto)
 
+AC_ARG_ENABLE(expat,
+	AS_HELP_STRING([--enable-expat],
+		[enable the expat XML parser]),,
+	enable_expat=auto)
+
 AC_ARG_ENABLE(adplug,
 	AS_HELP_STRING([--enable-adplug],
 		[enable the AdPlug decoder plugin (default: auto)]),,
@@ -639,6 +644,15 @@ fi
 
 AM_CONDITIONAL(HAVE_LIBMPDCLIENT, test x$enable_libmpdclient = xyes)
 
+dnl -------------------------------- expat --------------------------------
+MPD_AUTO_PKG(expat, EXPAT, [expat],
+	[expat XML parser], [expat not found])
+if test x$enable_expat = xyes; then
+	AC_DEFINE(HAVE_EXPAT, 1, [Define to use the expat XML parser])
+fi
+
+AM_CONDITIONAL(HAVE_EXPAT, test x$enable_expat = xyes)
+
 dnl --------------------------------- inotify ---------------------------------
 AC_CHECK_FUNCS(inotify_init inotify_init1)
 

src/Expat.cxx

@@ -0,0 +1,76 @@
+/*
+ * Copyright (C) 2003-2014 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#include "config.h"
+#include "Expat.hxx"
+#include "InputStream.hxx"
+#include "util/ASCII.hxx"
+#include "util/Error.hxx"
+#include "util/Domain.hxx"
+
+#include <string.h>
+
+static constexpr Domain expat_domain("expat");
+
+void
+ExpatParser::SetError(Error &error)
+{
+	XML_Error code = XML_GetErrorCode(parser);
+	error.Format(expat_domain, int(code), "XML parser failed: %s",
+		     XML_ErrorString(code));
+}
+
+bool
+ExpatParser::Parse(InputStream &is, Error &error)
+{
+	assert(is.ready);
+
+	while (true) {
+		char buffer[4096];
+		size_t nbytes = is.LockRead(buffer, sizeof(buffer), error);
+		if (nbytes == 0)
+			break;
+
+		if (XML_Parse(parser, buffer, nbytes, false) != XML_STATUS_OK) {
+			SetError(error);
+			return false;
+		}
+	}
+
+	if (error.IsDefined())
+		return false;
+
+	if (XML_Parse(parser, "", 0, true) != XML_STATUS_OK) {
+		SetError(error);
+		return false;
+	}
+
+	return true;
+}
+
+const char *
+ExpatParser::GetAttributeCase(const XML_Char **atts,
+			      const char *name)
+{
+	for (unsigned i = 0; atts[i] != nullptr; ++i)
+		if (StringEqualsCaseASCII(atts[i], name))
+			return atts[i] + strlen(name) + 1;
+
+	return nullptr;
+}

src/Expat.hxx

@@ -0,0 +1,63 @@
+/*
+ * Copyright (C) 2003-2014 The Music Player Daemon Project
+ * http://www.musicpd.org
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License along
+ * with this program; if not, write to the Free Software Foundation, Inc.,
+ * 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA.
+ */
+
+#ifndef MPD_EXPAT_HXX
+#define MPD_EXPAT_HXX
+
+#include "check.h"
+#include "Compiler.h"
+
+#include <expat.h>
+
+struct InputStream;
+class Error;
+
+class ExpatParser final {
+	const XML_Parser parser;
+
+public:
+	ExpatParser(void *userData)
+		:parser(XML_ParserCreate(nullptr)) {
+		XML_SetUserData(parser, userData);
+	}
+
+	~ExpatParser() {
+		XML_ParserFree(parser);
+	}
+
+	void SetElementHandler(XML_StartElementHandler start,
+			       XML_EndElementHandler end) {
+		XML_SetElementHandler(parser, start, end);
+	}
+
+	void SetCharacterDataHandler(XML_CharacterDataHandler charhndl) {
+		XML_SetCharacterDataHandler(parser, charhndl);
+	}
+
+	bool Parse(InputStream &is, Error &error);
+
+	gcc_pure
+	static const char *GetAttributeCase(const XML_Char **atts,
+					    const char *name);
+
+private:
+	void SetError(Error &error);
+};
+
+#endif

src/PlaylistRegistry.cxx

@@ -46,10 +46,12 @@
 const struct playlist_plugin *const playlist_plugins[] = {
 	&extm3u_playlist_plugin,
 	&m3u_playlist_plugin,
-	&xspf_playlist_plugin,
 	&pls_playlist_plugin,
+#ifdef HAVE_EXPAT
+	&xspf_playlist_plugin,
 	&asx_playlist_plugin,
 	&rss_playlist_plugin,
+#endif
 #ifdef ENABLE_DESPOTIFY
 	&despotify_playlist_plugin,
 #endif

src/playlist/AsxPlaylistPlugin.cxx

@@ -21,20 +21,13 @@
 #include "AsxPlaylistPlugin.hxx"
 #include "PlaylistPlugin.hxx"
 #include "MemorySongEnumerator.hxx"
-#include "InputStream.hxx"
 #include "Song.hxx"
 #include "tag/TagBuilder.hxx"
 #include "util/ASCII.hxx"
 #include "util/Error.hxx"
-#include "util/Domain.hxx"
+#include "Expat.hxx"
 #include "Log.hxx"
 
-#include <glib.h>
-
-#include <string.h>
-
-static constexpr Domain asx_domain("asx");
-
 /**
  * This is the state object for the GLib XML parser.
  */
@@ -71,23 +64,9 @@ struct AsxParser {
 
 };
 
-static const gchar *
+static void XMLCALL
-get_attribute(const gchar **attribute_names, const gchar **attribute_values,
+asx_start_element(void *user_data, const XML_Char *element_name,
-	      const gchar *name)
+		  const XML_Char **atts)
-{
-	for (unsigned i = 0; attribute_names[i] != nullptr; ++i)
-		if (StringEqualsCaseASCII(attribute_names[i], name))
-			return attribute_values[i];
-
-	return nullptr;
-}
-
-static void
-asx_start_element(gcc_unused GMarkupParseContext *context,
-		  const gchar *element_name,
-		  const gchar **attribute_names,
-		  const gchar **attribute_values,
-		  gpointer user_data, gcc_unused GError **error)
 {
 	AsxParser *parser = (AsxParser *)user_data;
 
@@ -103,9 +82,8 @@ asx_start_element(gcc_unused GMarkupParseContext *context,
 
 	case AsxParser::ENTRY:
 		if (StringEqualsCaseASCII(element_name, "ref")) {
-			const gchar *href = get_attribute(attribute_names,
+			const char *href =
-							  attribute_values,
+				ExpatParser::GetAttributeCase(atts, "href");
-							  "href");
 			if (href != nullptr)
 				parser->location = href;
 		} else if (StringEqualsCaseASCII(element_name, "author"))
@@ -119,10 +97,8 @@ asx_start_element(gcc_unused GMarkupParseContext *context,
 	}
 }
 
-static void
+static void XMLCALL
-asx_end_element(gcc_unused GMarkupParseContext *context,
+asx_end_element(void *user_data, const XML_Char *element_name)
-		const gchar *element_name,
-		gpointer user_data, gcc_unused GError **error)
 {
 	AsxParser *parser = (AsxParser *)user_data;
 
@@ -144,10 +120,8 @@ asx_end_element(gcc_unused GMarkupParseContext *context,
 	}
 }
 
-static void
+static void XMLCALL
-asx_text(gcc_unused GMarkupParseContext *context,
+asx_char_data(void *user_data, const XML_Char *s, int len)
-	 const gchar *text, gsize text_len,
-	 gpointer user_data, gcc_unused GError **error)
 {
 	AsxParser *parser = (AsxParser *)user_data;
 
@@ -156,23 +130,13 @@ asx_text(gcc_unused GMarkupParseContext *context,
 		break;
 
 	case AsxParser::ENTRY:
-		if (parser->tag_type != TAG_NUM_OF_ITEM_TYPES) {
+		if (parser->tag_type != TAG_NUM_OF_ITEM_TYPES)
-			parser->tag_builder.AddItem(parser->tag_type,
+			parser->tag_builder.AddItem(parser->tag_type, s, len);
-						    text, text_len);
-		}
 
 		break;
 	}
 }
 
-static const GMarkupParser asx_parser = {
-	asx_start_element,
-	asx_end_element,
-	asx_text,
-	nullptr,
-	nullptr,
-};
-
 /*
  * The playlist object
  *
@@ -182,57 +146,21 @@ static SongEnumerator *
 asx_open_stream(InputStream &is)
 {
 	AsxParser parser;
-	bool success;
-	Error error2;
-	GError *error = nullptr;
 
-	/* parse the ASX XML file */
+	{
+		ExpatParser expat(&parser);
+		expat.SetElementHandler(asx_start_element, asx_end_element);
+		expat.SetCharacterDataHandler(asx_char_data);
 
-	GMarkupParseContext *context =
+		Error error;
-		g_markup_parse_context_new(&asx_parser,
+		if (!expat.Parse(is, error)) {
-					   G_MARKUP_TREAT_CDATA_AS_TEXT,
+			LogError(error);
-					   &parser, nullptr);
-
-	while (true) {
-	char buffer[1024];
-		size_t nbytes = is.LockRead(buffer, sizeof(buffer), error2);
-		if (nbytes == 0) {
-			if (error2.IsDefined()) {
-				g_markup_parse_context_free(context);
-				LogError(error2);
-				return nullptr;
-			}
-
-			break;
-		}
-
-		success = g_markup_parse_context_parse(context, buffer, nbytes,
-						       &error);
-		if (!success) {
-			FormatErrno(asx_domain,
-				    "XML parser failed: %s", error->message);
-			g_error_free(error);
-			g_markup_parse_context_free(context);
 			return nullptr;
 		}
 	}
 
-	success = g_markup_parse_context_end_parse(context, &error);
-	if (!success) {
-		FormatErrno(asx_domain,
-			    "XML parser failed: %s", error->message);
-		g_error_free(error);
-		g_markup_parse_context_free(context);
-		return nullptr;
-	}
-
 	parser.songs.reverse();
-	MemorySongEnumerator *playlist =
+	return new MemorySongEnumerator(std::move(parser.songs));
-		new MemorySongEnumerator(std::move(parser.songs));
-
-	g_markup_parse_context_free(context);
-
-	return playlist;
 }
 
 static const char *const asx_suffixes[] = {

src/playlist/RssPlaylistPlugin.cxx

@@ -21,20 +21,13 @@
 #include "RssPlaylistPlugin.hxx"
 #include "PlaylistPlugin.hxx"
 #include "MemorySongEnumerator.hxx"
-#include "InputStream.hxx"
 #include "Song.hxx"
 #include "tag/TagBuilder.hxx"
 #include "util/ASCII.hxx"
 #include "util/Error.hxx"
-#include "util/Domain.hxx"
+#include "Expat.hxx"
 #include "Log.hxx"
 
-#include <glib.h>
-
-#include <string.h>
-
-static constexpr Domain rss_domain("rss");
-
 /**
  * This is the state object for the GLib XML parser.
  */
@@ -71,23 +64,9 @@ struct RssParser {
 		:state(ROOT) {}
 };
 
-static const gchar *
+static void XMLCALL
-get_attribute(const gchar **attribute_names, const gchar **attribute_values,
+rss_start_element(void *user_data, const XML_Char *element_name,
-	      const gchar *name)
+		  const XML_Char **atts)
-{
-	for (unsigned i = 0; attribute_names[i] != nullptr; ++i)
-		if (StringEqualsCaseASCII(attribute_names[i], name))
-			return attribute_values[i];
-
-	return nullptr;
-}
-
-static void
-rss_start_element(gcc_unused GMarkupParseContext *context,
-		  const gchar *element_name,
-		  const gchar **attribute_names,
-		  const gchar **attribute_values,
-		  gpointer user_data, gcc_unused GError **error)
 {
 	RssParser *parser = (RssParser *)user_data;
 
@@ -103,9 +82,8 @@ rss_start_element(gcc_unused GMarkupParseContext *context,
 
 	case RssParser::ITEM:
 		if (StringEqualsCaseASCII(element_name, "enclosure")) {
-			const gchar *href = get_attribute(attribute_names,
+			const char *href =
-							  attribute_values,
+				ExpatParser::GetAttributeCase(atts, "url");
-							  "url");
 			if (href != nullptr)
 				parser->location = href;
 		} else if (StringEqualsCaseASCII(element_name, "title"))
@@ -117,10 +95,8 @@ rss_start_element(gcc_unused GMarkupParseContext *context,
 	}
 }
 
-static void
+static void XMLCALL
-rss_end_element(gcc_unused GMarkupParseContext *context,
+rss_end_element(void *user_data, const XML_Char *element_name)
-		const gchar *element_name,
-		gpointer user_data, gcc_unused GError **error)
 {
 	RssParser *parser = (RssParser *)user_data;
 
@@ -142,10 +118,8 @@ rss_end_element(gcc_unused GMarkupParseContext *context,
 	}
 }
 
-static void
+static void XMLCALL
-rss_text(gcc_unused GMarkupParseContext *context,
+rss_char_data(void *user_data, const XML_Char *s, int len)
-	 const gchar *text, gsize text_len,
-	 gpointer user_data, gcc_unused GError **error)
 {
 	RssParser *parser = (RssParser *)user_data;
 
@@ -155,21 +129,12 @@ rss_text(gcc_unused GMarkupParseContext *context,
 
 	case RssParser::ITEM:
 		if (parser->tag_type != TAG_NUM_OF_ITEM_TYPES)
-			parser->tag_builder.AddItem(parser->tag_type,
+			parser->tag_builder.AddItem(parser->tag_type, s, len);
-						    text, text_len);
 
 		break;
 	}
 }
 
-static const GMarkupParser rss_parser = {
-	rss_start_element,
-	rss_end_element,
-	rss_text,
-	nullptr,
-	nullptr,
-};
-
 /*
  * The playlist object
  *
@@ -179,58 +144,21 @@ static SongEnumerator *
 rss_open_stream(InputStream &is)
 {
 	RssParser parser;
-	GMarkupParseContext *context;
-	char buffer[1024];
-	size_t nbytes;
-	bool success;
-	Error error2;
-	GError *error = nullptr;
 
-	/* parse the RSS XML file */
+	{
+		ExpatParser expat(&parser);
+		expat.SetElementHandler(rss_start_element, rss_end_element);
+		expat.SetCharacterDataHandler(rss_char_data);
 
-	context = g_markup_parse_context_new(&rss_parser,
+		Error error;
-					     G_MARKUP_TREAT_CDATA_AS_TEXT,
+		if (!expat.Parse(is, error)) {
-					     &parser, nullptr);
+			LogError(error);
-
-	while (true) {
-		nbytes = is.LockRead(buffer, sizeof(buffer), error2);
-		if (nbytes == 0) {
-			if (error2.IsDefined()) {
-				g_markup_parse_context_free(context);
-				LogError(error2);
-				return nullptr;
-			}
-
-			break;
-		}
-
-		success = g_markup_parse_context_parse(context, buffer, nbytes,
-						       &error);
-		if (!success) {
-			FormatError(rss_domain,
-				    "XML parser failed: %s", error->message);
-			g_error_free(error);
-			g_markup_parse_context_free(context);
 			return nullptr;
 		}
 	}
 
-	success = g_markup_parse_context_end_parse(context, &error);
-	if (!success) {
-		FormatError(rss_domain,
-			    "XML parser failed: %s", error->message);
-		g_error_free(error);
-		g_markup_parse_context_free(context);
-		return nullptr;
-	}
-
 	parser.songs.reverse();
-	MemorySongEnumerator *playlist =
+	return new MemorySongEnumerator(std::move(parser.songs));
-		new MemorySongEnumerator(std::move(parser.songs));
-
-	g_markup_parse_context_free(context);
-
-	return playlist;
 }
 
 static const char *const rss_suffixes[] = {

src/playlist/XspfPlaylistPlugin.cxx

@@ -26,10 +26,9 @@
 #include "tag/TagBuilder.hxx"
 #include "util/Error.hxx"
 #include "util/Domain.hxx"
+#include "Expat.hxx"
 #include "Log.hxx"
 
-#include <glib.h>
-
 #include <string.h>
 
 static constexpr Domain xspf_domain("xspf");
@@ -70,12 +69,9 @@ struct XspfParser {
 		:state(ROOT) {}
 };
 
-static void
+static void XMLCALL
-xspf_start_element(gcc_unused GMarkupParseContext *context,
+xspf_start_element(void *user_data, const XML_Char *element_name,
-		   const gchar *element_name,
+		   gcc_unused const XML_Char **atts)
-		   gcc_unused const gchar **attribute_names,
-		   gcc_unused const gchar **attribute_values,
-		   gpointer user_data, gcc_unused GError **error)
 {
 	XspfParser *parser = (XspfParser *)user_data;
 
@@ -124,10 +120,8 @@ xspf_start_element(gcc_unused GMarkupParseContext *context,
 	}
 }
 
-static void
+static void XMLCALL
-xspf_end_element(gcc_unused GMarkupParseContext *context,
+xspf_end_element(void *user_data, const XML_Char *element_name)
-		 const gchar *element_name,
-		 gpointer user_data, gcc_unused GError **error)
 {
 	XspfParser *parser = (XspfParser *)user_data;
 
@@ -165,10 +159,8 @@ xspf_end_element(gcc_unused GMarkupParseContext *context,
 	}
 }
 
-static void
+static void XMLCALL
-xspf_text(gcc_unused GMarkupParseContext *context,
+xspf_char_data(void *user_data, const XML_Char *s, int len)
-	  const gchar *text, gsize text_len,
-	  gpointer user_data, gcc_unused GError **error)
 {
 	XspfParser *parser = (XspfParser *)user_data;
 
@@ -181,26 +173,17 @@ xspf_text(gcc_unused GMarkupParseContext *context,
 	case XspfParser::TRACK:
 		if (!parser->location.empty() &&
 		    parser->tag_type != TAG_NUM_OF_ITEM_TYPES)
-			parser->tag_builder.AddItem(parser->tag_type,
+			parser->tag_builder.AddItem(parser->tag_type, s, len);
-						    text, text_len);
 
 		break;
 
 	case XspfParser::LOCATION:
-		parser->location.assign(text, text_len);
+		parser->location.assign(s, len);
 
 		break;
 	}
 }
 
-static const GMarkupParser xspf_parser = {
-	xspf_start_element,
-	xspf_end_element,
-	xspf_text,
-	nullptr,
-	nullptr,
-};
-
 /*
  * The playlist object
  *
@@ -210,58 +193,21 @@ static SongEnumerator *
 xspf_open_stream(InputStream &is)
 {
 	XspfParser parser;
-	GMarkupParseContext *context;
-	char buffer[1024];
-	size_t nbytes;
-	bool success;
-	Error error2;
-	GError *error = nullptr;
 
-	/* parse the XSPF XML file */
+	{
+		ExpatParser expat(&parser);
+		expat.SetElementHandler(xspf_start_element, xspf_end_element);
+		expat.SetCharacterDataHandler(xspf_char_data);
 
-	context = g_markup_parse_context_new(&xspf_parser,
+		Error error;
-					     G_MARKUP_TREAT_CDATA_AS_TEXT,
+		if (!expat.Parse(is, error)) {
-					     &parser, nullptr);
+			LogError(error);
-
-	while (true) {
-		nbytes = is.LockRead(buffer, sizeof(buffer), error2);
-		if (nbytes == 0) {
-			if (error2.IsDefined()) {
-				g_markup_parse_context_free(context);
-				LogError(error2);
-				return nullptr;
-			}
-
-			break;
-		}
-
-		success = g_markup_parse_context_parse(context, buffer, nbytes,
-						       &error);
-		if (!success) {
-			FormatError(xspf_domain,
-				    "XML parser failed: %s", error->message);
-			g_error_free(error);
-			g_markup_parse_context_free(context);
 			return nullptr;
 		}
 	}
 
-	success = g_markup_parse_context_end_parse(context, &error);
-	if (!success) {
-		FormatError(xspf_domain,
-			    "XML parser failed: %s", error->message);
-		g_error_free(error);
-		g_markup_parse_context_free(context);
-		return nullptr;
-	}
-
 	parser.songs.reverse();
-	MemorySongEnumerator *playlist =
+	return new MemorySongEnumerator(std::move(parser.songs));
-		new MemorySongEnumerator(std::move(parser.songs));
-
-	g_markup_parse_context_free(context);
-
-	return playlist;
 }
 
 static const char *const xspf_suffixes[] = {