Merge branch 'v0.21.x'

src/PluginUnavailable.hxx

@@ -29,9 +29,7 @@
  */
 class PluginUnavailable : public std::runtime_error {
 public:
-	template<typename M>
-	explicit PluginUnavailable(M &&msg) noexcept
-		:std::runtime_error(std::forward<M>(msg)) {}
+	using std::runtime_error::runtime_error;
 };
 
 /**
@@ -42,9 +40,7 @@ public:
  */
 class PluginUnconfigured : public PluginUnavailable {
 public:
-	template<typename M>
-	explicit PluginUnconfigured(M &&msg) noexcept
-		:PluginUnavailable(std::forward<M>(msg)) {}
+	using PluginUnavailable::PluginUnavailable;
 };
 
 #endif

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);

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());
 

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,

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<SimpleDatabase *>(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
 

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

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;

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 (...) {

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<typename P>
-	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;

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;

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;

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;

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)

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;
 }
 
 /**

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)) {

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);

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())

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);

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;
 

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 */
 

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);
 

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();
 }

src/storage/CompositeStorage.cxx

@@ -211,6 +211,7 @@ CompositeStorage::Mount(const char *uri, std::unique_ptr<Storage> storage)
 	const std::lock_guard<Mutex> protect(mutex);
 
 	Directory &directory = root.Make(uri);
+	assert(!directory.storage);
 	directory.storage = std::move(storage);
 }
 

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<Mutex> protect(mutex);
+		return IsMounted(root, storage_uri);
+	}
+
 	void Mount(const char *uri, std::unique_ptr<Storage> 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,

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;
 }