diff --git a/NEWS b/NEWS index 3fad71c81..43c7fcbb1 100644 --- a/NEWS +++ b/NEWS @@ -43,15 +43,24 @@ ver 0.22 (not yet released) ver 0.21.25 (not yet released) * protocol: - fix crash when using "rangeid" while playing +* database + - simple: automatically scan new mounts +* storage + - fix disappearing mounts after mounting twice + - udisks: fix reading ".mpdignore" * input - file: detect premature end of file - smbclient: don't send credentials to MPD clients * decoder - opus: apply pre-skip and end trimming - opus: fix memory leak + - opus: fix crash bug + - vorbis: fix crash bug * output - osx: improve sample rate selection - osx: fix noise while stopping +* neighbor + - upnp: fix crash during shutdown * Windows/Android: - fix Boost detection after breaking change in Meson 0.54 diff --git a/doc/plugins.rst b/doc/plugins.rst index db700bd69..9c1fd3d31 100644 --- a/doc/plugins.rst +++ b/doc/plugins.rst @@ -60,25 +60,25 @@ The default plugin which gives :program:`MPD` access to local files. It is used curl ---- -A WebDAV client using libcurl. It is used when :code:`music_directory` contains a http:// or https:// URI, for example :samp:`https://the.server/dav/`. +A WebDAV client using libcurl. It is used when :code:`music_directory` +contains a ``http://`` or ``https://`` URI, for example +:samp:`https://the.server/dav/`. smbclient --------- -Load music files from a SMB/CIFS server. It is used when :code:`music_directory` contains a smb:// URI, for example :samp:`smb://myfileserver/Music`. +Load music files from a SMB/CIFS server. It is used when +:code:`music_directory` contains a ``smb://`` URI, for example +:samp:`smb://myfileserver/Music`. nfs --- -Load music files from a NFS server. It is used when :code:`music_directory` contains a nfs:// URI according to RFC2224, for example :samp:`nfs://servername/path`. +Load music files from a NFS server. It is used when +:code:`music_directory` contains a ``nfs://`` URI according to +RFC2224, for example :samp:`nfs://servername/path`. -This plugin uses libnfs, which supports only NFS version 3. Since :program:`MPD` is not allowed to bind to "privileged ports", the NFS server needs to enable the "insecure" setting; example :file:`/etc/exports`: - -.. code-block:: none - - /srv/mp3 192.168.1.55(ro,insecure) - -Don't fear: "insecure" does not mean that your NFS server is insecure. A few decades ago, people thought the concept of "privileged ports" would make network services "secure", which was a fallacy. The absence of this obsolete "security" measure means little. +See :ref:`input_nfs` for more information. udisks ------ @@ -186,7 +186,10 @@ curl Opens remote files or streams over HTTP using libcurl. -Note that unless overridden by the below settings (e.g. by setting them to a blank value), general curl configuration from environment variables such as http_proxy or specified in :file:`~/.curlrc` will be in effect. +Note that unless overridden by the below settings (e.g. by setting +them to a blank value), general curl configuration from environment +variables such as ``http_proxy`` or specified in :file:`~/.curlrc` +will be in effect. .. list-table:: :widths: 20 80 @@ -206,7 +209,9 @@ Note that unless overridden by the below settings (e.g. by setting them to a bla ffmpeg ------ -Access to various network protocols implemented by the FFmpeg library: gopher://, rtp://, rtsp://, rtmp://, rtmpt://, rtmps:// +Access to various network protocols implemented by the FFmpeg library: +``gopher://``, ``rtp://``, ``rtsp://``, ``rtmp://``, ``rtmpt://``, +``rtmps://`` file ---- @@ -218,30 +223,51 @@ mms Plays streams with the MMS protocol using `libmms `_. +.. _input_nfs: + nfs --- -Allows :program:`MPD` to access files on NFSv3 servers without actually mounting them (i.e. in userspace, without help from the kernel's VFS layer). All URIs with the nfs:// scheme are used according to RFC2224. Example: +Allows :program:`MPD` to access files on NFS servers without actually +mounting them (i.e. with :program:`libnfs` in userspace, without help +from the kernel's VFS layer). All URIs with the ``nfs://`` scheme are +used according to RFC2224. Example: .. code-block:: none mpc add nfs://servername/path/filename.ogg -Note that this usually requires enabling the "insecure" flag in the server's /etc/exports file, because :program:`MPD` cannot bind to so-called "privileged" ports. Don't fear: this will not make your file server insecure; the flag was named in a time long ago when privileged ports were thought to be meaningful for security. By today's standards, NFSv3 is not secure at all, and if you believe it is, you're already doomed. +This plugin uses :program:`libnfs`, which supports only NFS version 3. +Since :program:`MPD` is not allowed to bind to so-called "privileged +ports", the NFS server needs to enable the ``insecure`` setting; +example :file:`/etc/exports`: + +.. code-block:: none + + /srv/mp3 192.168.1.55(ro,insecure) + +Don't fear: this will not make your file server insecure; the flag was +named a time long ago when privileged ports were thought to be +meaningful for security. By today's standards, NFSv3 is not secure at +all, and if you believe it is, you're already doomed. smbclient --------- -Allows :program:`MPD` to access files on SMB/CIFS servers (e.g. Samba or Microsoft Windows). All URIs with the smb:// scheme are used. Example: +Allows :program:`MPD` to access files on SMB/CIFS servers (e.g. Samba +or Microsoft Windows). All URIs with the ``smb://`` scheme are +used. Example: .. code-block:: none mpc add smb://servername/sharename/filename.ogg + mpc add smb://username:password@servername/sharename/filename.ogg qobuz ----- -Play songs from the commercial streaming service Qobuz. It plays URLs in the form qobuz://track/ID, e.g.: +Play songs from the commercial streaming service Qobuz. It plays URLs +in the form ``qobuz://track/ID``, e.g.: .. code-block:: none @@ -267,7 +293,9 @@ Play songs from the commercial streaming service Qobuz. It plays URLs in the for tidal ----- -Play songs from the commercial streaming service `Tidal `_. It plays URLs in the form tidal://track/ID, e.g.: +Play songs from the commercial streaming service `Tidal +`_. It plays URLs in the form ``tidal://track/ID``, +e.g.: .. warning:: diff --git a/src/PluginUnavailable.hxx b/src/PluginUnavailable.hxx index f47826689..a29361ad4 100644 --- a/src/PluginUnavailable.hxx +++ b/src/PluginUnavailable.hxx @@ -29,9 +29,7 @@ */ class PluginUnavailable : public std::runtime_error { public: - template - explicit PluginUnavailable(M &&msg) noexcept - :std::runtime_error(std::forward(msg)) {} + using std::runtime_error::runtime_error; }; /** @@ -42,9 +40,7 @@ public: */ class PluginUnconfigured : public PluginUnavailable { public: - template - explicit PluginUnconfigured(M &&msg) noexcept - :PluginUnavailable(std::forward(msg)) {} + using PluginUnavailable::PluginUnavailable; }; #endif diff --git a/src/SongUpdate.cxx b/src/SongUpdate.cxx index 30cd699e0..c5701d9aa 100644 --- a/src/SongUpdate.cxx +++ b/src/SongUpdate.cxx @@ -69,17 +69,22 @@ Song::UpdateFile(Storage &storage) TagBuilder tag_builder; auto new_audio_format = AudioFormat::Undefined(); - const auto path_fs = storage.MapFS(relative_uri.c_str()); - if (path_fs.IsNull()) { - const auto absolute_uri = - storage.MapUTF8(relative_uri.c_str()); - if (!tag_stream_scan(absolute_uri.c_str(), tag_builder, - &new_audio_format)) - return false; - } else { - if (!ScanFileTagsWithGeneric(path_fs, tag_builder, + try { + const auto path_fs = storage.MapFS(relative_uri.c_str()); + if (path_fs.IsNull()) { + const auto absolute_uri = + storage.MapUTF8(relative_uri.c_str()); + if (!tag_stream_scan(absolute_uri.c_str(), tag_builder, &new_audio_format)) - return false; + return false; + } else { + if (!ScanFileTagsWithGeneric(path_fs, tag_builder, + &new_audio_format)) + return false; + } + } catch (...) { + // TODO: log or propagate I/O errors? + return false; } mtime = info.mtime; @@ -139,8 +144,14 @@ DetachedSong::LoadFile(Path path) return false; TagBuilder tag_builder; - if (!ScanFileTagsWithGeneric(path, tag_builder)) + + try { + if (!ScanFileTagsWithGeneric(path, tag_builder)) + return false; + } catch (...) { + // TODO: log or propagate I/O errors? return false; + } mtime = fi.GetModificationTime(); tag_builder.Commit(tag); @@ -157,8 +168,14 @@ DetachedSong::Update() return LoadFile(path_fs); } else if (IsRemote()) { TagBuilder tag_builder; - if (!tag_stream_scan(uri.c_str(), tag_builder)) + + try { + if (!tag_stream_scan(uri.c_str(), tag_builder)) + return false; + } catch (...) { + // TODO: log or propagate I/O errors? return false; + } mtime = std::chrono::system_clock::time_point::min(); tag_builder.Commit(tag); diff --git a/src/TagStream.cxx b/src/TagStream.cxx index 4010318e1..0a1f0b744 100644 --- a/src/TagStream.cxx +++ b/src/TagStream.cxx @@ -43,7 +43,7 @@ CheckDecoderPlugin(const DecoderPlugin &plugin, } bool -tag_stream_scan(InputStream &is, TagHandler &handler) noexcept +tag_stream_scan(InputStream &is, TagHandler &handler) { assert(is.IsReady()); @@ -81,7 +81,7 @@ tag_stream_scan(const char *uri, TagHandler &handler) bool tag_stream_scan(InputStream &is, TagBuilder &builder, - AudioFormat *audio_format) noexcept + AudioFormat *audio_format) { assert(is.IsReady()); diff --git a/src/TagStream.hxx b/src/TagStream.hxx index f7c54e082..9a61719b4 100644 --- a/src/TagStream.hxx +++ b/src/TagStream.hxx @@ -29,14 +29,16 @@ class TagBuilder; * Scan the tags of an #InputStream. Invokes matching decoder * plugins, but does not invoke the special "APE" and "ID3" scanners. * + * Throws on I/O error. + * * @return true if the file was recognized (even if no metadata was * found) */ bool -tag_stream_scan(InputStream &is, TagHandler &handler) noexcept; +tag_stream_scan(InputStream &is, TagHandler &handler); /** - * Throws on error. + * Throws on I/O error. */ bool tag_stream_scan(const char *uri, TagHandler &handler); @@ -46,15 +48,17 @@ tag_stream_scan(const char *uri, TagHandler &handler); * plugins, and falls back to generic scanners (APE and ID3) if no * tags were found (but the file was recognized). * + * Throws on I/O error. + * * @return true if the file was recognized (even if no metadata was * found) */ bool tag_stream_scan(InputStream &is, TagBuilder &builder, - AudioFormat *audio_format=nullptr) noexcept; + AudioFormat *audio_format=nullptr); /** - * Throws on error. + * Throws on I/O error. */ bool tag_stream_scan(const char *uri, TagBuilder &builder, diff --git a/src/command/StorageCommands.cxx b/src/command/StorageCommands.cxx index 5dbff101f..8b5693103 100644 --- a/src/command/StorageCommands.cxx +++ b/src/command/StorageCommands.cxx @@ -195,6 +195,16 @@ handle_mount(Client &client, Request args, Response &r) return CommandResult::ERROR; } + if (composite.IsMountPoint(local_uri)) { + r.Error(ACK_ERROR_ARG, "Mount point busy"); + return CommandResult::ERROR; + } + + if (composite.IsMounted(remote_uri)) { + r.Error(ACK_ERROR_ARG, "This storage is already mounted"); + return CommandResult::ERROR; + } + auto &event_loop = instance.io_thread.GetEventLoop(); auto storage = CreateStorageURI(event_loop, remote_uri); if (storage == nullptr) { @@ -207,8 +217,10 @@ handle_mount(Client &client, Request args, Response &r) #ifdef ENABLE_DATABASE if (auto *db = dynamic_cast(instance.GetDatabase())) { + bool need_update; + try { - db->Mount(local_uri, remote_uri); + need_update = !db->Mount(local_uri, remote_uri); } catch (...) { composite.Unmount(local_uri); throw; @@ -217,6 +229,12 @@ handle_mount(Client &client, Request args, Response &r) // TODO: call Instance::OnDatabaseModified()? // TODO: trigger database update? instance.EmitIdle(IDLE_DATABASE); + + if (need_update) { + UpdateService *update = client.GetInstance().update; + if (update != nullptr) + update->Enqueue(local_uri, false); + } } #endif diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.cxx b/src/db/plugins/simple/SimpleDatabasePlugin.cxx index c0dff9cc2..3cea58a57 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.cxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.cxx @@ -427,7 +427,7 @@ IsUnsafeChar(char ch) return !IsSafeChar(ch); } -void +bool SimpleDatabase::Mount(const char *local_uri, const char *storage_uri) { if (cache_path.IsNull()) @@ -446,9 +446,11 @@ SimpleDatabase::Mount(const char *local_uri, const char *storage_uri) compress); db->Open(); - // TODO: update the new database instance? + bool exists = db->FileExists(); Mount(local_uri, std::move(db)); + + return exists; } inline DatabasePtr diff --git a/src/db/plugins/simple/SimpleDatabasePlugin.hxx b/src/db/plugins/simple/SimpleDatabasePlugin.hxx index ac4f067d1..efcf87927 100644 --- a/src/db/plugins/simple/SimpleDatabasePlugin.hxx +++ b/src/db/plugins/simple/SimpleDatabasePlugin.hxx @@ -103,9 +103,11 @@ public: /** * Throws #std::runtime_error on error. + * + * @return false if the mounted database needs to be updated */ gcc_nonnull_all - void Mount(const char *local_uri, const char *storage_uri); + bool Mount(const char *local_uri, const char *storage_uri); gcc_nonnull_all bool Unmount(const char *uri) noexcept; diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx index 1cb1538dc..7526c6b6f 100644 --- a/src/db/update/Walk.cxx +++ b/src/db/update/Walk.cxx @@ -322,8 +322,8 @@ UpdateWalk::UpdateDirectory(Directory &directory, try { Mutex mutex; - auto is = InputStream::OpenReady(PathTraitsUTF8::Build(storage.MapUTF8(directory.GetPath()), - ".mpdignore").c_str(), + auto is = InputStream::OpenReady(storage.MapUTF8(PathTraitsUTF8::Build(directory.GetPath(), + ".mpdignore")).c_str(), mutex); child_exclude_list.Load(std::move(is)); } catch (...) { diff --git a/src/decoder/DecoderPlugin.hxx b/src/decoder/DecoderPlugin.hxx index 61f959c79..da4409e47 100644 --- a/src/decoder/DecoderPlugin.hxx +++ b/src/decoder/DecoderPlugin.hxx @@ -67,18 +67,22 @@ struct DecoderPlugin { void (*file_decode)(DecoderClient &client, Path path_fs) = nullptr; /** - * Scan metadata of a file. + * Scan metadata of a file. + * + * Throws on I/O error. * - * @return false if the operation has failed + * @return false if the file was not recognized */ - bool (*scan_file)(Path path_fs, TagHandler &handler) noexcept = nullptr; + bool (*scan_file)(Path path_fs, TagHandler &handler) = nullptr; /** - * Scan metadata of a file. + * Scan metadata of a stream. + * + * Throws on I/O error. * - * @return false if the operation has failed + * @return false if the stream was not recognized */ - bool (*scan_stream)(InputStream &is, TagHandler &handler) noexcept = nullptr; + bool (*scan_stream)(InputStream &is, TagHandler &handler) = nullptr; /** * @brief Return a "virtual" filename for subtracks in @@ -99,14 +103,14 @@ struct DecoderPlugin { void (*_file_decode)(DecoderClient &client, Path path_fs), bool (*_scan_file)(Path path_fs, - TagHandler &handler) noexcept) noexcept + TagHandler &handler)) noexcept :name(_name), file_decode(_file_decode), scan_file(_scan_file) {} constexpr DecoderPlugin(const char *_name, void (*_stream_decode)(DecoderClient &client, InputStream &is), - bool (*_scan_stream)(InputStream &is, TagHandler &handler) noexcept) noexcept + bool (*_scan_stream)(InputStream &is, TagHandler &handler)) noexcept :name(_name), stream_decode(_stream_decode), scan_stream(_scan_stream) {} @@ -114,11 +118,11 @@ struct DecoderPlugin { constexpr DecoderPlugin(const char *_name, void (*_stream_decode)(DecoderClient &client, InputStream &is), - bool (*_scan_stream)(InputStream &is, TagHandler &handler) noexcept, + bool (*_scan_stream)(InputStream &is, TagHandler &handler), void (*_file_decode)(DecoderClient &client, Path path_fs), bool (*_scan_file)(Path path_fs, - TagHandler &handler) noexcept) noexcept + TagHandler &handler)) noexcept :name(_name), stream_decode(_stream_decode), file_decode(_file_decode), @@ -191,7 +195,7 @@ struct DecoderPlugin { * Read the tag of a file. */ template - bool ScanFile(P path_fs, TagHandler &handler) const noexcept { + bool ScanFile(P path_fs, TagHandler &handler) const { return scan_file != nullptr ? scan_file(path_fs, handler) : false; @@ -200,7 +204,7 @@ struct DecoderPlugin { /** * Read the tag of a stream. */ - bool ScanStream(InputStream &is, TagHandler &handler) const noexcept { + bool ScanStream(InputStream &is, TagHandler &handler) const { return scan_stream != nullptr ? scan_stream(is, handler) : false; diff --git a/src/decoder/plugins/AudiofileDecoderPlugin.cxx b/src/decoder/plugins/AudiofileDecoderPlugin.cxx index eff898c3b..ce78c693c 100644 --- a/src/decoder/plugins/AudiofileDecoderPlugin.cxx +++ b/src/decoder/plugins/AudiofileDecoderPlugin.cxx @@ -241,7 +241,7 @@ audiofile_stream_decode(DecoderClient &client, InputStream &is) } static bool -audiofile_scan_stream(InputStream &is, TagHandler &handler) noexcept +audiofile_scan_stream(InputStream &is, TagHandler &handler) { if (!is.IsSeekable() || !is.KnownSize()) return false; diff --git a/src/decoder/plugins/DsdiffDecoderPlugin.cxx b/src/decoder/plugins/DsdiffDecoderPlugin.cxx index 5758e05ad..1fb88da16 100644 --- a/src/decoder/plugins/DsdiffDecoderPlugin.cxx +++ b/src/decoder/plugins/DsdiffDecoderPlugin.cxx @@ -448,7 +448,7 @@ dsdiff_stream_decode(DecoderClient &client, InputStream &is) } static bool -dsdiff_scan_stream(InputStream &is, TagHandler &handler) noexcept +dsdiff_scan_stream(InputStream &is, TagHandler &handler) { DsdiffMetaData metadata; DsdiffChunkHeader chunk_header; diff --git a/src/decoder/plugins/DsfDecoderPlugin.cxx b/src/decoder/plugins/DsfDecoderPlugin.cxx index 1d8e943c3..de627a137 100644 --- a/src/decoder/plugins/DsfDecoderPlugin.cxx +++ b/src/decoder/plugins/DsfDecoderPlugin.cxx @@ -325,7 +325,7 @@ dsf_stream_decode(DecoderClient &client, InputStream &is) } static bool -dsf_scan_stream(InputStream &is, TagHandler &handler) noexcept +dsf_scan_stream(InputStream &is, TagHandler &handler) { /* check DSF metadata */ DsfMetaData metadata; diff --git a/src/decoder/plugins/FaadDecoderPlugin.cxx b/src/decoder/plugins/FaadDecoderPlugin.cxx index a3884adec..e9b0feac9 100644 --- a/src/decoder/plugins/FaadDecoderPlugin.cxx +++ b/src/decoder/plugins/FaadDecoderPlugin.cxx @@ -411,7 +411,7 @@ faad_stream_decode(DecoderClient &client, InputStream &is) } static bool -faad_scan_stream(InputStream &is, TagHandler &handler) noexcept +faad_scan_stream(InputStream &is, TagHandler &handler) { auto result = faad_get_file_time(is); if (!result.first) diff --git a/src/decoder/plugins/FfmpegDecoderPlugin.cxx b/src/decoder/plugins/FfmpegDecoderPlugin.cxx index 971241826..ceb0653c2 100644 --- a/src/decoder/plugins/FfmpegDecoderPlugin.cxx +++ b/src/decoder/plugins/FfmpegDecoderPlugin.cxx @@ -591,8 +591,7 @@ ffmpeg_decode(DecoderClient &client, InputStream &input) } static bool -FfmpegScanStream(AVFormatContext &format_context, - TagHandler &handler) noexcept +FfmpegScanStream(AVFormatContext &format_context, TagHandler &handler) { const int find_result = avformat_find_stream_info(&format_context, nullptr); @@ -625,16 +624,14 @@ FfmpegScanStream(AVFormatContext &format_context, } static bool -ffmpeg_scan_stream(InputStream &is, TagHandler &handler) noexcept -try { +ffmpeg_scan_stream(InputStream &is, TagHandler &handler) +{ AvioStream stream(nullptr, is); if (!stream.Open()) return false; auto f = FfmpegOpenInput(stream.io, is.GetURI(), nullptr); return FfmpegScanStream(*f, handler); -} catch (...) { - return false; } /** diff --git a/src/decoder/plugins/FlacDecoderPlugin.cxx b/src/decoder/plugins/FlacDecoderPlugin.cxx index e1c06c5a6..856169450 100644 --- a/src/decoder/plugins/FlacDecoderPlugin.cxx +++ b/src/decoder/plugins/FlacDecoderPlugin.cxx @@ -69,7 +69,7 @@ flac_write_cb(const FLAC__StreamDecoder *dec, const FLAC__Frame *frame, } static bool -flac_scan_file(Path path_fs, TagHandler &handler) noexcept +flac_scan_file(Path path_fs, TagHandler &handler) { FlacMetadataChain chain; if (!chain.Read(NarrowPath(path_fs))) { @@ -84,7 +84,7 @@ flac_scan_file(Path path_fs, TagHandler &handler) noexcept } static bool -flac_scan_stream(InputStream &is, TagHandler &handler) noexcept +flac_scan_stream(InputStream &is, TagHandler &handler) { FlacMetadataChain chain; if (!chain.Read(is)) { @@ -313,7 +313,7 @@ oggflac_init([[maybe_unused]] const ConfigBlock &block) } static bool -oggflac_scan_file(Path path_fs, TagHandler &handler) noexcept +oggflac_scan_file(Path path_fs, TagHandler &handler) { FlacMetadataChain chain; if (!chain.ReadOgg(NarrowPath(path_fs))) { @@ -328,7 +328,7 @@ oggflac_scan_file(Path path_fs, TagHandler &handler) noexcept } static bool -oggflac_scan_stream(InputStream &is, TagHandler &handler) noexcept +oggflac_scan_stream(InputStream &is, TagHandler &handler) { FlacMetadataChain chain; if (!chain.ReadOgg(is)) { diff --git a/src/decoder/plugins/MadDecoderPlugin.cxx b/src/decoder/plugins/MadDecoderPlugin.cxx index 95dc6f7ce..608b03250 100644 --- a/src/decoder/plugins/MadDecoderPlugin.cxx +++ b/src/decoder/plugins/MadDecoderPlugin.cxx @@ -1013,7 +1013,7 @@ MadDecoder::RunScan(TagHandler &handler) noexcept } static bool -mad_decoder_scan_stream(InputStream &is, TagHandler &handler) noexcept +mad_decoder_scan_stream(InputStream &is, TagHandler &handler) { MadDecoder data(nullptr, is); return data.RunScan(handler); diff --git a/src/decoder/plugins/MpcdecDecoderPlugin.cxx b/src/decoder/plugins/MpcdecDecoderPlugin.cxx index e83925574..2ecbe4a48 100644 --- a/src/decoder/plugins/MpcdecDecoderPlugin.cxx +++ b/src/decoder/plugins/MpcdecDecoderPlugin.cxx @@ -273,7 +273,7 @@ mpcdec_get_file_duration(InputStream &is) } static bool -mpcdec_scan_stream(InputStream &is, TagHandler &handler) noexcept +mpcdec_scan_stream(InputStream &is, TagHandler &handler) { const auto duration = mpcdec_get_file_duration(is); if (duration.IsNegative()) diff --git a/src/decoder/plugins/OpusDecoderPlugin.cxx b/src/decoder/plugins/OpusDecoderPlugin.cxx index 126409098..ba5a2bff4 100644 --- a/src/decoder/plugins/OpusDecoderPlugin.cxx +++ b/src/decoder/plugins/OpusDecoderPlugin.cxx @@ -423,8 +423,8 @@ VisitOpusDuration(InputStream &is, OggSyncState &sync, OggStreamState &stream, } } -bool -mpd_opus_scan_stream(InputStream &is, TagHandler &handler) noexcept +static bool +mpd_opus_scan_stream(InputStream &is, TagHandler &handler) { InputStreamReader reader(is); OggSyncState oy(reader); diff --git a/src/decoder/plugins/SndfileDecoderPlugin.cxx b/src/decoder/plugins/SndfileDecoderPlugin.cxx index 803dd259b..bde242c87 100644 --- a/src/decoder/plugins/SndfileDecoderPlugin.cxx +++ b/src/decoder/plugins/SndfileDecoderPlugin.cxx @@ -268,7 +268,7 @@ static constexpr struct { }; static bool -sndfile_scan_stream(InputStream &is, TagHandler &handler) noexcept +sndfile_scan_stream(InputStream &is, TagHandler &handler) { SF_INFO info; diff --git a/src/decoder/plugins/VorbisDecoderPlugin.cxx b/src/decoder/plugins/VorbisDecoderPlugin.cxx index 13361c2ad..cadef0ca2 100644 --- a/src/decoder/plugins/VorbisDecoderPlugin.cxx +++ b/src/decoder/plugins/VorbisDecoderPlugin.cxx @@ -370,7 +370,7 @@ VisitVorbisDuration(InputStream &is, } static bool -vorbis_scan_stream(InputStream &is, TagHandler &handler) noexcept +vorbis_scan_stream(InputStream &is, TagHandler &handler) { /* initialize libogg */ diff --git a/src/decoder/plugins/WavpackDecoderPlugin.cxx b/src/decoder/plugins/WavpackDecoderPlugin.cxx index 983c4e0f7..a3b6f0991 100644 --- a/src/decoder/plugins/WavpackDecoderPlugin.cxx +++ b/src/decoder/plugins/WavpackDecoderPlugin.cxx @@ -612,7 +612,7 @@ wavpack_scan_file(Path path_fs, TagHandler &handler) noexcept } static bool -wavpack_scan_stream(InputStream &is, TagHandler &handler) noexcept +wavpack_scan_stream(InputStream &is, TagHandler &handler) { WavpackInput isp(nullptr, is); diff --git a/src/neighbor/plugins/UdisksNeighborPlugin.cxx b/src/neighbor/plugins/UdisksNeighborPlugin.cxx index 4ff406f2c..687fff0e2 100644 --- a/src/neighbor/plugins/UdisksNeighborPlugin.cxx +++ b/src/neighbor/plugins/UdisksNeighborPlugin.cxx @@ -45,6 +45,11 @@ ToNeighborInfo(const UDisks2::Object &o) noexcept return {o.GetUri(), o.path}; } +static constexpr char udisks_neighbor_match[] = + "type='signal',sender='" UDISKS2_INTERFACE "'," + "interface='" DBUS_OM_INTERFACE "'," + "path='" UDISKS2_PATH "'"; + class UdisksNeighborExplorer final : public NeighborExplorer { @@ -106,26 +111,37 @@ UdisksNeighborExplorer::DoOpen() auto &connection = GetConnection(); + /* this ugly try/catch cascade is only here because this + method has no RAII for this method - TODO: improve this */ try { Error error; - dbus_bus_add_match(connection, - "type='signal',sender='" UDISKS2_INTERFACE "'," - "interface='" DBUS_OM_INTERFACE "'," - "path='" UDISKS2_PATH "'", - error); + dbus_bus_add_match(connection, udisks_neighbor_match, error); error.CheckThrow("DBus AddMatch error"); - dbus_connection_add_filter(connection, - HandleMessage, this, - nullptr); + try { + dbus_connection_add_filter(connection, + HandleMessage, this, + nullptr); - auto msg = Message::NewMethodCall(UDISKS2_INTERFACE, - UDISKS2_PATH, - DBUS_OM_INTERFACE, - "GetManagedObjects"); - list_request.Send(connection, *msg.Get(), - std::bind(&UdisksNeighborExplorer::OnListNotify, - this, std::placeholders::_1)); + try { + auto msg = Message::NewMethodCall(UDISKS2_INTERFACE, + UDISKS2_PATH, + DBUS_OM_INTERFACE, + "GetManagedObjects"); + list_request.Send(connection, *msg.Get(), + std::bind(&UdisksNeighborExplorer::OnListNotify, + this, std::placeholders::_1)); + } catch (...) { + dbus_connection_remove_filter(connection, + HandleMessage, + this); + throw; + } + } catch (...) { + dbus_bus_remove_match(connection, + udisks_neighbor_match, nullptr); + throw; + } } catch (...) { dbus_glue.Destruct(); throw; @@ -145,8 +161,10 @@ UdisksNeighborExplorer::DoClose() noexcept list_request.Cancel(); } - // TODO: remove_match - // TODO: remove_filter + auto &connection = GetConnection(); + + dbus_connection_remove_filter(connection, HandleMessage, this); + dbus_bus_remove_match(connection, udisks_neighbor_match, nullptr); dbus_glue.Destruct(); } diff --git a/src/storage/CompositeStorage.cxx b/src/storage/CompositeStorage.cxx index 479927f65..f93452b70 100644 --- a/src/storage/CompositeStorage.cxx +++ b/src/storage/CompositeStorage.cxx @@ -211,6 +211,7 @@ CompositeStorage::Mount(const char *uri, std::unique_ptr storage) const std::lock_guard protect(mutex); Directory &directory = root.Make(uri); + assert(!directory.storage); directory.storage = std::move(storage); } diff --git a/src/storage/CompositeStorage.hxx b/src/storage/CompositeStorage.hxx index 3ed9ab730..432aecbd1 100644 --- a/src/storage/CompositeStorage.hxx +++ b/src/storage/CompositeStorage.hxx @@ -100,6 +100,15 @@ public: gcc_pure gcc_nonnull_all Storage *GetMount(std::string_view uri) noexcept; + /** + * Is the given URI a mount point, i.e. is something already + * mounted on this path? + */ + gcc_pure gcc_nonnull_all + bool IsMountPoint(const char *uri) noexcept { + return GetMount(uri) != nullptr; + } + /** * Call the given function for each mounted storage, including * the root storage. Passes mount point URI and the a const @@ -112,6 +121,15 @@ public: VisitMounts(uri, root, t); } + /** + * Is a storage with the given URI already mounted? + */ + gcc_pure gcc_nonnull_all + bool IsMounted(const char *storage_uri) const noexcept { + const std::lock_guard protect(mutex); + return IsMounted(root, storage_uri); + } + void Mount(const char *uri, std::unique_ptr storage); bool Unmount(const char *uri); @@ -146,6 +164,22 @@ private: } } + gcc_pure gcc_nonnull_all + static bool IsMounted(const Directory &directory, + const char *storage_uri) noexcept { + if (directory.storage) { + const auto uri = directory.storage->MapUTF8(""); + if (uri == storage_uri) + return true; + } + + for (const auto &i : directory.children) + if (IsMounted(i.second, storage_uri)) + return true; + + return false; + } + /** * Follow the given URI path, and find the outermost directory * which is a #Storage mount point. If there are no mounts, diff --git a/src/storage/StorageState.cxx b/src/storage/StorageState.cxx index 8d4445dc8..9d2f0d10b 100644 --- a/src/storage/StorageState.cxx +++ b/src/storage/StorageState.cxx @@ -106,6 +106,17 @@ storage_state_restore(const char *line, TextFile &file, Instance &instance) FormatDebug(storage_domain, "Restoring mount %s => %s", uri.c_str(), url.c_str()); + auto &composite_storage = *(CompositeStorage *)instance.storage; + if (composite_storage.IsMountPoint(uri.c_str())) { + LogError(storage_domain, "Mount point busy"); + return true; + } + + if (composite_storage.IsMounted(url.c_str())) { + LogError(storage_domain, "This storage is already mounted"); + return true; + } + auto &event_loop = instance.io_thread.GetEventLoop(); auto storage = CreateStorageURI(event_loop, url.c_str()); if (storage == nullptr) { @@ -124,8 +135,7 @@ storage_state_restore(const char *line, TextFile &file, Instance &instance) } } - ((CompositeStorage*)instance.storage)->Mount(uri.c_str(), - std::move(storage)); + composite_storage.Mount(uri.c_str(), std::move(storage)); return true; }