From 0705f42cf8be9224780c09d21e5568ac9c9f086e Mon Sep 17 00:00:00 2001 From: Max Kellermann Date: Sun, 21 Feb 2016 12:53:47 +0100 Subject: [PATCH] playlist/Plugin: pass InputStreamPtr&& to open_stream() Obsolete class CloseSongEnumerator, which was a kludge. --- Makefile.am | 2 - src/input/TextInputStream.cxx | 7 ++- src/input/TextInputStream.hxx | 11 +++-- src/playlist/CloseSongEnumerator.cxx | 35 -------------- src/playlist/CloseSongEnumerator.hxx | 47 ------------------- src/playlist/PlaylistPlugin.hxx | 12 +++-- src/playlist/PlaylistRegistry.cxx | 32 +++++++------ src/playlist/PlaylistRegistry.hxx | 7 +-- src/playlist/PlaylistStream.cxx | 15 ++---- src/playlist/plugins/AsxPlaylistPlugin.cxx | 4 +- src/playlist/plugins/CuePlaylistPlugin.cxx | 9 ++-- src/playlist/plugins/ExtM3uPlaylistPlugin.cxx | 25 ++++++---- src/playlist/plugins/M3uPlaylistPlugin.cxx | 8 ++-- src/playlist/plugins/PlsPlaylistPlugin.cxx | 16 +++++-- src/playlist/plugins/RssPlaylistPlugin.cxx | 4 +- src/playlist/plugins/XspfPlaylistPlugin.cxx | 4 +- test/dump_playlist.cxx | 2 +- test/dump_text_file.cxx | 14 +++--- 18 files changed, 97 insertions(+), 157 deletions(-) delete mode 100644 src/playlist/CloseSongEnumerator.cxx delete mode 100644 src/playlist/CloseSongEnumerator.hxx diff --git a/Makefile.am b/Makefile.am index de1f813eb..96be158d0 100644 --- a/Makefile.am +++ b/Makefile.am @@ -1468,8 +1468,6 @@ endif libplaylist_plugins_a_SOURCES = \ src/playlist/PlaylistPlugin.hxx \ src/playlist/SongEnumerator.hxx \ - src/playlist/CloseSongEnumerator.cxx \ - src/playlist/CloseSongEnumerator.hxx \ src/playlist/MemorySongEnumerator.cxx \ src/playlist/MemorySongEnumerator.hxx \ src/playlist/plugins/ExtM3uPlaylistPlugin.cxx \ diff --git a/src/input/TextInputStream.cxx b/src/input/TextInputStream.cxx index 897b2b472..b82da3a38 100644 --- a/src/input/TextInputStream.cxx +++ b/src/input/TextInputStream.cxx @@ -26,6 +26,11 @@ #include +TextInputStream::TextInputStream(InputStreamPtr &&_is) + :is(std::move(_is)) {} + +TextInputStream::~TextInputStream() {} + char * TextInputStream::ReadLine() { @@ -54,7 +59,7 @@ TextInputStream::ReadLine() --dest.size; Error error; - size_t nbytes = is.LockRead(dest.data, dest.size, error); + size_t nbytes = is->LockRead(dest.data, dest.size, error); if (nbytes > 0) buffer.Append(nbytes); else if (error.IsDefined()) { diff --git a/src/input/TextInputStream.hxx b/src/input/TextInputStream.hxx index d0b5d5f40..a2d08c43a 100644 --- a/src/input/TextInputStream.hxx +++ b/src/input/TextInputStream.hxx @@ -20,12 +20,13 @@ #ifndef MPD_TEXT_INPUT_STREAM_HXX #define MPD_TEXT_INPUT_STREAM_HXX +#include "input/Ptr.hxx" #include "util/StaticFifoBuffer.hxx" class InputStream; class TextInputStream { - InputStream &is; + InputStreamPtr is; StaticFifoBuffer buffer; public: @@ -35,12 +36,16 @@ public: * * @param _is an open #InputStream object */ - explicit TextInputStream(InputStream &_is) - :is(_is) {} + explicit TextInputStream(InputStreamPtr &&_is); + ~TextInputStream(); TextInputStream(const TextInputStream &) = delete; TextInputStream& operator=(const TextInputStream &) = delete; + InputStreamPtr &&StealInputStream() { + return std::move(is); + } + /** * Reads the next line from the stream with newline character stripped. * diff --git a/src/playlist/CloseSongEnumerator.cxx b/src/playlist/CloseSongEnumerator.cxx deleted file mode 100644 index 1637cfb0b..000000000 --- a/src/playlist/CloseSongEnumerator.cxx +++ /dev/null @@ -1,35 +0,0 @@ -/* - * Copyright (C) 2003-2015 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 "CloseSongEnumerator.hxx" -#include "input/InputStream.hxx" -#include "DetachedSong.hxx" - -CloseSongEnumerator::~CloseSongEnumerator() -{ - delete other; - delete is; -} - -std::unique_ptr -CloseSongEnumerator::NextSong() -{ - return other->NextSong(); -} diff --git a/src/playlist/CloseSongEnumerator.hxx b/src/playlist/CloseSongEnumerator.hxx deleted file mode 100644 index adcd622eb..000000000 --- a/src/playlist/CloseSongEnumerator.hxx +++ /dev/null @@ -1,47 +0,0 @@ -/* - * Copyright (C) 2003-2015 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_CLOSE_SONG_ENUMERATOR_HXX -#define MPD_CLOSE_SONG_ENUMERATOR_HXX - -#include "SongEnumerator.hxx" -#include "Compiler.h" - -class InputStream; - -/** - * A #SongEnumerator wrapper that closes an #InputStream automatically - * after deleting the #SongEnumerator - */ -class CloseSongEnumerator final : public SongEnumerator { - SongEnumerator *const other; - - InputStream *const is; - -public: - gcc_nonnull_all - CloseSongEnumerator(SongEnumerator *_other, InputStream *const _is) - :other(_other), is(_is) {} - - virtual ~CloseSongEnumerator(); - - virtual std::unique_ptr NextSong() override; -}; - -#endif diff --git a/src/playlist/PlaylistPlugin.hxx b/src/playlist/PlaylistPlugin.hxx index 8d232d2cc..390ffaa1e 100644 --- a/src/playlist/PlaylistPlugin.hxx +++ b/src/playlist/PlaylistPlugin.hxx @@ -20,8 +20,9 @@ #ifndef MPD_PLAYLIST_PLUGIN_HXX #define MPD_PLAYLIST_PLUGIN_HXX +#include "input/Ptr.hxx" + struct ConfigBlock; -class InputStream; struct Tag; class Mutex; class Cond; @@ -57,8 +58,11 @@ struct playlist_plugin { * Opens the playlist in the specified input stream. It has * either matched one of the suffixes or one of the MIME * types. + * + * @parm is the input stream; the pointer will not be + * invalidated when the function returns nullptr */ - SongEnumerator *(*open_stream)(InputStream &is); + SongEnumerator *(*open_stream)(InputStreamPtr &&is); const char *const*schemes; const char *const*suffixes; @@ -101,9 +105,9 @@ playlist_plugin_open_uri(const struct playlist_plugin *plugin, const char *uri, static inline SongEnumerator * playlist_plugin_open_stream(const struct playlist_plugin *plugin, - InputStream &is) + InputStreamPtr &&is) { - return plugin->open_stream(is); + return plugin->open_stream(std::move(is)); } #endif diff --git a/src/playlist/PlaylistRegistry.cxx b/src/playlist/PlaylistRegistry.cxx index 2156414be..8e728d636 100644 --- a/src/playlist/PlaylistRegistry.cxx +++ b/src/playlist/PlaylistRegistry.cxx @@ -183,7 +183,7 @@ playlist_list_open_uri(const char *uri, Mutex &mutex, Cond &cond) } static SongEnumerator * -playlist_list_open_stream_mime2(InputStream &is, const char *mime) +playlist_list_open_stream_mime2(InputStreamPtr &&is, const char *mime) { assert(mime != nullptr); @@ -193,10 +193,10 @@ playlist_list_open_stream_mime2(InputStream &is, const char *mime) string_array_contains(plugin->mime_types, mime)) { /* rewind the stream, so each plugin gets a fresh start */ - is.Rewind(IgnoreError()); + is->Rewind(IgnoreError()); auto playlist = playlist_plugin_open_stream(plugin, - is); + std::move(is)); if (playlist != nullptr) return playlist; } @@ -206,24 +206,25 @@ playlist_list_open_stream_mime2(InputStream &is, const char *mime) } static SongEnumerator * -playlist_list_open_stream_mime(InputStream &is, const char *full_mime) +playlist_list_open_stream_mime(InputStreamPtr &&is, const char *full_mime) { assert(full_mime != nullptr); const char *semicolon = strchr(full_mime, ';'); if (semicolon == nullptr) - return playlist_list_open_stream_mime2(is, full_mime); + return playlist_list_open_stream_mime2(std::move(is), + full_mime); if (semicolon == full_mime) return nullptr; /* probe only the portion before the semicolon*/ const std::string mime(full_mime, semicolon); - return playlist_list_open_stream_mime2(is, mime.c_str()); + return playlist_list_open_stream_mime2(std::move(is), mime.c_str()); } SongEnumerator * -playlist_list_open_stream_suffix(InputStream &is, const char *suffix) +playlist_list_open_stream_suffix(InputStreamPtr &&is, const char *suffix) { assert(suffix != nullptr); @@ -233,9 +234,10 @@ playlist_list_open_stream_suffix(InputStream &is, const char *suffix) string_array_contains(plugin->suffixes, suffix)) { /* rewind the stream, so each plugin gets a fresh start */ - is.Rewind(IgnoreError()); + is->Rewind(IgnoreError()); - auto playlist = playlist_plugin_open_stream(plugin, is); + auto playlist = playlist_plugin_open_stream(plugin, + std::move(is)); if (playlist != nullptr) return playlist; } @@ -245,13 +247,14 @@ playlist_list_open_stream_suffix(InputStream &is, const char *suffix) } SongEnumerator * -playlist_list_open_stream(InputStream &is, const char *uri) +playlist_list_open_stream(InputStreamPtr &&is, const char *uri) { - assert(is.IsReady()); + assert(is->IsReady()); - const char *const mime = is.GetMimeType(); + const char *const mime = is->GetMimeType(); if (mime != nullptr) { - auto playlist = playlist_list_open_stream_mime(is, mime); + auto playlist = playlist_list_open_stream_mime(std::move(is), + mime); if (playlist != nullptr) return playlist; } @@ -261,7 +264,8 @@ playlist_list_open_stream(InputStream &is, const char *uri) ? uri_get_suffix(uri, suffix_buffer) : nullptr; if (suffix != nullptr) { - auto playlist = playlist_list_open_stream_suffix(is, suffix); + auto playlist = playlist_list_open_stream_suffix(std::move(is), + suffix); if (playlist != nullptr) return playlist; } diff --git a/src/playlist/PlaylistRegistry.hxx b/src/playlist/PlaylistRegistry.hxx index 09e842b13..37a270c2d 100644 --- a/src/playlist/PlaylistRegistry.hxx +++ b/src/playlist/PlaylistRegistry.hxx @@ -20,10 +20,11 @@ #ifndef MPD_PLAYLIST_REGISTRY_HXX #define MPD_PLAYLIST_REGISTRY_HXX +#include "input/Ptr.hxx" + class Mutex; class Cond; class SongEnumerator; -class InputStream; extern const struct playlist_plugin *const playlist_plugins[]; @@ -52,7 +53,7 @@ SongEnumerator * playlist_list_open_uri(const char *uri, Mutex &mutex, Cond &cond); SongEnumerator * -playlist_list_open_stream_suffix(InputStream &is, const char *suffix); +playlist_list_open_stream_suffix(InputStreamPtr &&is, const char *suffix); /** * Opens a playlist from an input stream. @@ -62,7 +63,7 @@ playlist_list_open_stream_suffix(InputStream &is, const char *suffix); * used to select the appropriate playlist plugin */ SongEnumerator * -playlist_list_open_stream(InputStream &is, const char *uri); +playlist_list_open_stream(InputStreamPtr &&is, const char *uri); /** * Determines if there is a playlist plugin which can handle the diff --git a/src/playlist/PlaylistStream.cxx b/src/playlist/PlaylistStream.cxx index 9663d39b6..46340253a 100644 --- a/src/playlist/PlaylistStream.cxx +++ b/src/playlist/PlaylistStream.cxx @@ -20,7 +20,6 @@ #include "config.h" #include "PlaylistStream.hxx" #include "PlaylistRegistry.hxx" -#include "CloseSongEnumerator.hxx" #include "util/UriUtil.hxx" #include "util/Error.hxx" #include "input/InputStream.hxx" @@ -50,12 +49,8 @@ try { return nullptr; } - auto playlist = playlist_list_open_stream_suffix(*is, - suffix_utf8.c_str()); - if (playlist != nullptr) - playlist = new CloseSongEnumerator(playlist, is.release()); - - return playlist; + return playlist_list_open_stream_suffix(std::move(is), + suffix_utf8.c_str()); } catch (const std::runtime_error &e) { LogError(e); return nullptr; @@ -97,11 +92,7 @@ try { return nullptr; } - playlist = playlist_list_open_stream(*is, uri); - if (playlist == nullptr) - return nullptr; - - return new CloseSongEnumerator(playlist, is.release()); + return playlist_list_open_stream(std::move(is), uri); } catch (const std::runtime_error &e) { LogError(e); return nullptr; diff --git a/src/playlist/plugins/AsxPlaylistPlugin.cxx b/src/playlist/plugins/AsxPlaylistPlugin.cxx index 1a0334237..420b1e875 100644 --- a/src/playlist/plugins/AsxPlaylistPlugin.cxx +++ b/src/playlist/plugins/AsxPlaylistPlugin.cxx @@ -144,7 +144,7 @@ asx_char_data(void *user_data, const XML_Char *s, int len) */ static SongEnumerator * -asx_open_stream(InputStream &is) +asx_open_stream(InputStreamPtr &&is) { AsxParser parser; @@ -154,7 +154,7 @@ asx_open_stream(InputStream &is) expat.SetCharacterDataHandler(asx_char_data); Error error; - if (!expat.Parse(is, error)) { + if (!expat.Parse(*is, error)) { LogError(error); return nullptr; } diff --git a/src/playlist/plugins/CuePlaylistPlugin.cxx b/src/playlist/plugins/CuePlaylistPlugin.cxx index 4aa253528..5d1bebaef 100644 --- a/src/playlist/plugins/CuePlaylistPlugin.cxx +++ b/src/playlist/plugins/CuePlaylistPlugin.cxx @@ -27,22 +27,21 @@ #include class CuePlaylist final : public SongEnumerator { - InputStream &is; TextInputStream tis; CueParser parser; public: - CuePlaylist(InputStream &_is) - :is(_is), tis(is) { + CuePlaylist(InputStreamPtr &&is) + :tis(std::move(is)) { } virtual std::unique_ptr NextSong() override; }; static SongEnumerator * -cue_playlist_open_stream(InputStream &is) +cue_playlist_open_stream(InputStreamPtr &&is) { - return new CuePlaylist(is); + return new CuePlaylist(std::move(is)); } std::unique_ptr diff --git a/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx b/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx index 53f863650..385a2968c 100644 --- a/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx +++ b/src/playlist/plugins/ExtM3uPlaylistPlugin.cxx @@ -27,6 +27,7 @@ #include "util/StringUtil.hxx" #include "util/StringCompare.hxx" #include "input/TextInputStream.hxx" +#include "input/InputStream.hxx" #include #include @@ -35,28 +36,36 @@ class ExtM3uPlaylist final : public SongEnumerator { TextInputStream tis; public: - ExtM3uPlaylist(InputStream &is) - :tis(is) { + ExtM3uPlaylist(InputStreamPtr &&is) + :tis(std::move(is)) { } - bool CheckFirstLine() { + /** + * @return nullptr if ExtM3U was recognized, or the original + * InputStream on error + */ + InputStreamPtr CheckFirstLine() { char *line = tis.ReadLine(); if (line == nullptr) - return false; + return tis.StealInputStream(); StripRight(line); - return strcmp(line, "#EXTM3U") == 0; + if (strcmp(line, "#EXTM3U") != 0) + return tis.StealInputStream(); + + return nullptr; } virtual std::unique_ptr NextSong() override; }; static SongEnumerator * -extm3u_open_stream(InputStream &is) +extm3u_open_stream(InputStreamPtr &&is) { - ExtM3uPlaylist *playlist = new ExtM3uPlaylist(is); + ExtM3uPlaylist *playlist = new ExtM3uPlaylist(std::move(is)); - if (!playlist->CheckFirstLine()) { + is = playlist->CheckFirstLine(); + if (is) { /* no EXTM3U header: fall back to the plain m3u plugin */ delete playlist; diff --git a/src/playlist/plugins/M3uPlaylistPlugin.cxx b/src/playlist/plugins/M3uPlaylistPlugin.cxx index 0b1bfda1b..6b689b9d3 100644 --- a/src/playlist/plugins/M3uPlaylistPlugin.cxx +++ b/src/playlist/plugins/M3uPlaylistPlugin.cxx @@ -29,17 +29,17 @@ class M3uPlaylist final : public SongEnumerator { TextInputStream tis; public: - M3uPlaylist(InputStream &is) - :tis(is) { + M3uPlaylist(InputStreamPtr &&is) + :tis(std::move(is)) { } virtual std::unique_ptr NextSong() override; }; static SongEnumerator * -m3u_open_stream(InputStream &is) +m3u_open_stream(InputStreamPtr &&is) { - return new M3uPlaylist(is); + return new M3uPlaylist(std::move(is)); } std::unique_ptr diff --git a/src/playlist/plugins/PlsPlaylistPlugin.cxx b/src/playlist/plugins/PlsPlaylistPlugin.cxx index 7d2579cd3..62309a68b 100644 --- a/src/playlist/plugins/PlsPlaylistPlugin.cxx +++ b/src/playlist/plugins/PlsPlaylistPlugin.cxx @@ -22,6 +22,7 @@ #include "../PlaylistPlugin.hxx" #include "../MemorySongEnumerator.hxx" #include "input/TextInputStream.hxx" +#include "input/InputStream.hxx" #include "DetachedSong.hxx" #include "tag/TagBuilder.hxx" #include "util/ASCII.hxx" @@ -142,17 +143,22 @@ ParsePls(TextInputStream &is, std::forward_list &songs) } static bool -ParsePls(InputStream &is, std::forward_list &songs) +ParsePls(InputStreamPtr &&is, std::forward_list &songs) { - TextInputStream tis(is); - return ParsePls(tis, songs); + TextInputStream tis(std::move(is)); + if (!ParsePls(tis, songs)) { + is = tis.StealInputStream(); + return false; + } + + return true; } static SongEnumerator * -pls_open_stream(InputStream &is) +pls_open_stream(InputStreamPtr &&is) { std::forward_list songs; - if (!ParsePls(is, songs)) + if (!ParsePls(std::move(is), songs)) return nullptr; return new MemorySongEnumerator(std::move(songs)); diff --git a/src/playlist/plugins/RssPlaylistPlugin.cxx b/src/playlist/plugins/RssPlaylistPlugin.cxx index a2d8e7a42..7fa8e654d 100644 --- a/src/playlist/plugins/RssPlaylistPlugin.cxx +++ b/src/playlist/plugins/RssPlaylistPlugin.cxx @@ -142,7 +142,7 @@ rss_char_data(void *user_data, const XML_Char *s, int len) */ static SongEnumerator * -rss_open_stream(InputStream &is) +rss_open_stream(InputStreamPtr &&is) { RssParser parser; @@ -152,7 +152,7 @@ rss_open_stream(InputStream &is) expat.SetCharacterDataHandler(rss_char_data); Error error; - if (!expat.Parse(is, error)) { + if (!expat.Parse(*is, error)) { LogError(error); return nullptr; } diff --git a/src/playlist/plugins/XspfPlaylistPlugin.cxx b/src/playlist/plugins/XspfPlaylistPlugin.cxx index 0b7358e15..a62857f78 100644 --- a/src/playlist/plugins/XspfPlaylistPlugin.cxx +++ b/src/playlist/plugins/XspfPlaylistPlugin.cxx @@ -189,7 +189,7 @@ xspf_char_data(void *user_data, const XML_Char *s, int len) */ static SongEnumerator * -xspf_open_stream(InputStream &is) +xspf_open_stream(InputStreamPtr &&is) { XspfParser parser; @@ -199,7 +199,7 @@ xspf_open_stream(InputStream &is) expat.SetCharacterDataHandler(xspf_char_data); Error error; - if (!expat.Parse(is, error)) { + if (!expat.Parse(*is, error)) { LogError(error); return nullptr; } diff --git a/test/dump_playlist.cxx b/test/dump_playlist.cxx index 0da744528..2477348b5 100644 --- a/test/dump_playlist.cxx +++ b/test/dump_playlist.cxx @@ -98,7 +98,7 @@ try { /* open the playlist */ - playlist = playlist_list_open_stream(*is, uri); + playlist = playlist_list_open_stream(std::move(is), uri); if (playlist == NULL) { fprintf(stderr, "Failed to open playlist\n"); return 2; diff --git a/test/dump_text_file.cxx b/test/dump_text_file.cxx index 467aeb4f5..b112a26ee 100644 --- a/test/dump_text_file.cxx +++ b/test/dump_text_file.cxx @@ -44,23 +44,23 @@ dump_text_file(TextInputStream &is) } static int -dump_input_stream(InputStream &is) +dump_input_stream(InputStreamPtr &&is) { { - TextInputStream tis(is); + TextInputStream tis(std::move(is)); dump_text_file(tis); } - is.Lock(); + is->Lock(); Error error; - if (!is.Check(error)) { + if (!is->Check(error)) { LogError(error); - is.Unlock(); + is->Unlock(); return EXIT_FAILURE; } - is.Unlock(); + is->Unlock(); return 0; } @@ -98,7 +98,7 @@ int main(int argc, char **argv) auto is = InputStream::OpenReady(argv[1], mutex, cond, error); if (is) { - ret = dump_input_stream(*is); + ret = dump_input_stream(std::move(is)); } else { if (error.IsDefined()) LogError(error);