From dd82370a80c25322823bade6c9c76845b51eaa71 Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Thu, 9 Jan 2014 11:57:47 +0100 Subject: [PATCH] playlist/{asx,rss,xspf}: use Expat instead of GLib to parse XML --- Makefile.am | 13 +++- configure.ac | 14 ++++ src/Expat.cxx | 76 +++++++++++++++++++ src/Expat.hxx | 63 ++++++++++++++++ src/PlaylistRegistry.cxx | 4 +- src/playlist/AsxPlaylistPlugin.cxx | 112 +++++----------------------- src/playlist/RssPlaylistPlugin.cxx | 110 +++++---------------------- src/playlist/XspfPlaylistPlugin.cxx | 90 +++++----------------- 8 files changed, 223 insertions(+), 259 deletions(-) create mode 100644 src/Expat.cxx create mode 100644 src/Expat.hxx diff --git a/Makefile.am b/Makefile.am index a63c9d1d7..7db87fd98 100644 --- a/Makefile.am +++ b/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/playlist/PlsPlaylistPlugin.hxx \ + src/Expat.cxx src/Expat.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 # diff --git a/configure.ac b/configure.ac index 58cd5af06..0af602fe4 100644 --- a/configure.ac +++ b/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) diff --git a/src/Expat.cxx b/src/Expat.cxx new file mode 100644 index 000000000..5d5a8be24 --- /dev/null +++ b/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 + +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; +} diff --git a/src/Expat.hxx b/src/Expat.hxx new file mode 100644 index 000000000..e0de30e7e --- /dev/null +++ b/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 + +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 diff --git a/src/PlaylistRegistry.cxx b/src/PlaylistRegistry.cxx index 17cad3db5..ddfd39613 100644 --- a/src/PlaylistRegistry.cxx +++ b/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 diff --git a/src/playlist/AsxPlaylistPlugin.cxx b/src/playlist/AsxPlaylistPlugin.cxx index c06200283..4e48dbf8d 100644 --- a/src/playlist/AsxPlaylistPlugin.cxx +++ b/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 - -#include - -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 * -get_attribute(const gchar **attribute_names, const gchar **attribute_values, - const gchar *name) -{ - 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) +static void XMLCALL +asx_start_element(void *user_data, const XML_Char *element_name, + const XML_Char **atts) { 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, - attribute_values, - "href"); + const char *href = + ExpatParser::GetAttributeCase(atts, "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 -asx_end_element(gcc_unused GMarkupParseContext *context, - const gchar *element_name, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +asx_end_element(void *user_data, const XML_Char *element_name) { AsxParser *parser = (AsxParser *)user_data; @@ -144,10 +120,8 @@ asx_end_element(gcc_unused GMarkupParseContext *context, } } -static void -asx_text(gcc_unused GMarkupParseContext *context, - const gchar *text, gsize text_len, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +asx_char_data(void *user_data, const XML_Char *s, int len) { 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) { - parser->tag_builder.AddItem(parser->tag_type, - text, text_len); - } + if (parser->tag_type != TAG_NUM_OF_ITEM_TYPES) + parser->tag_builder.AddItem(parser->tag_type, s, 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 = - g_markup_parse_context_new(&asx_parser, - G_MARKUP_TREAT_CDATA_AS_TEXT, - &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); + Error error; + if (!expat.Parse(is, error)) { + LogError(error); 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 = - new MemorySongEnumerator(std::move(parser.songs)); - - g_markup_parse_context_free(context); - - return playlist; + return new MemorySongEnumerator(std::move(parser.songs)); } static const char *const asx_suffixes[] = { diff --git a/src/playlist/RssPlaylistPlugin.cxx b/src/playlist/RssPlaylistPlugin.cxx index 558c74619..604f99baf 100644 --- a/src/playlist/RssPlaylistPlugin.cxx +++ b/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 - -#include - -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 * -get_attribute(const gchar **attribute_names, const gchar **attribute_values, - const gchar *name) -{ - 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) +static void XMLCALL +rss_start_element(void *user_data, const XML_Char *element_name, + const XML_Char **atts) { 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, - attribute_values, - "url"); + const char *href = + ExpatParser::GetAttributeCase(atts, "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 -rss_end_element(gcc_unused GMarkupParseContext *context, - const gchar *element_name, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +rss_end_element(void *user_data, const XML_Char *element_name) { RssParser *parser = (RssParser *)user_data; @@ -142,10 +118,8 @@ rss_end_element(gcc_unused GMarkupParseContext *context, } } -static void -rss_text(gcc_unused GMarkupParseContext *context, - const gchar *text, gsize text_len, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +rss_char_data(void *user_data, const XML_Char *s, int len) { 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, - text, text_len); + parser->tag_builder.AddItem(parser->tag_type, s, 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, - G_MARKUP_TREAT_CDATA_AS_TEXT, - &parser, nullptr); - - 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); + Error error; + if (!expat.Parse(is, error)) { + LogError(error); 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 = - new MemorySongEnumerator(std::move(parser.songs)); - - g_markup_parse_context_free(context); - - return playlist; + return new MemorySongEnumerator(std::move(parser.songs)); } static const char *const rss_suffixes[] = { diff --git a/src/playlist/XspfPlaylistPlugin.cxx b/src/playlist/XspfPlaylistPlugin.cxx index 7c20df57d..2157dd678 100644 --- a/src/playlist/XspfPlaylistPlugin.cxx +++ b/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 - #include static constexpr Domain xspf_domain("xspf"); @@ -70,12 +69,9 @@ struct XspfParser { :state(ROOT) {} }; -static void -xspf_start_element(gcc_unused GMarkupParseContext *context, - const gchar *element_name, - gcc_unused const gchar **attribute_names, - gcc_unused const gchar **attribute_values, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +xspf_start_element(void *user_data, const XML_Char *element_name, + gcc_unused const XML_Char **atts) { XspfParser *parser = (XspfParser *)user_data; @@ -124,10 +120,8 @@ xspf_start_element(gcc_unused GMarkupParseContext *context, } } -static void -xspf_end_element(gcc_unused GMarkupParseContext *context, - const gchar *element_name, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +xspf_end_element(void *user_data, const XML_Char *element_name) { XspfParser *parser = (XspfParser *)user_data; @@ -165,10 +159,8 @@ xspf_end_element(gcc_unused GMarkupParseContext *context, } } -static void -xspf_text(gcc_unused GMarkupParseContext *context, - const gchar *text, gsize text_len, - gpointer user_data, gcc_unused GError **error) +static void XMLCALL +xspf_char_data(void *user_data, const XML_Char *s, int len) { 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, - text, text_len); + parser->tag_builder.AddItem(parser->tag_type, s, 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, - G_MARKUP_TREAT_CDATA_AS_TEXT, - &parser, nullptr); - - 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); + Error error; + if (!expat.Parse(is, error)) { + LogError(error); 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 = - new MemorySongEnumerator(std::move(parser.songs)); - - g_markup_parse_context_free(context); - - return playlist; + return new MemorySongEnumerator(std::move(parser.songs)); } static const char *const xspf_suffixes[] = {