From 35a232105ee9b52e08dc41587d165f424e00cd5e Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Wed, 4 Nov 2020 20:39:06 +0100
Subject: [PATCH] util/UriExtract: uri_get_suffix() returns std::string_view

No need to copy it to a buffer.
---
 src/TagStream.cxx                   |  9 ++++-----
 src/command/FingerprintCommands.cxx | 29 +++++++++++++--------------
 src/db/update/Archive.cxx           |  2 +-
 src/db/update/Container.cxx         |  2 +-
 src/db/update/Playlist.cxx          |  2 +-
 src/db/update/UpdateSong.cxx        |  4 ++--
 src/db/update/Walk.cxx              |  4 ++--
 src/db/update/Walk.hxx              | 12 +++++------
 src/decoder/Thread.cxx              | 23 +++++++++++----------
 src/playlist/PlaylistRegistry.cxx   | 22 ++++++++++----------
 src/util/UriExtract.cxx             | 26 +++++-------------------
 src/util/UriExtract.hxx             | 13 +-----------
 test/ContainerScan.cxx              |  8 +++-----
 test/TestUriExtract.cxx             | 31 ++++++++++-------------------
 14 files changed, 72 insertions(+), 115 deletions(-)

diff --git a/src/TagStream.cxx b/src/TagStream.cxx
index 0a1f0b744..8013161d0 100644
--- a/src/TagStream.cxx
+++ b/src/TagStream.cxx
@@ -36,10 +36,10 @@
 gcc_pure
 static bool
 CheckDecoderPlugin(const DecoderPlugin &plugin,
-		   const char *suffix, const char *mime) noexcept
+		   std::string_view suffix, const char *mime) noexcept
 {
 	return (mime != nullptr && plugin.SupportsMimeType(mime)) ||
-		(suffix != nullptr && plugin.SupportsSuffix(suffix));
+		(!suffix.empty() && plugin.SupportsSuffix(suffix));
 }
 
 bool
@@ -47,11 +47,10 @@ tag_stream_scan(InputStream &is, TagHandler &handler)
 {
 	assert(is.IsReady());
 
-	UriSuffixBuffer suffix_buffer;
-	const char *const suffix = uri_get_suffix(is.GetURI(), suffix_buffer);
+	const auto suffix = uri_get_suffix(is.GetURI());
 	const char *mime = is.GetMimeType();
 
-	if (suffix == nullptr && mime == nullptr)
+	if (suffix.empty() && mime == nullptr)
 		return false;
 
 	std::string mime_base;
diff --git a/src/command/FingerprintCommands.cxx b/src/command/FingerprintCommands.cxx
index d75b9f5be..3e8fa5893 100644
--- a/src/command/FingerprintCommands.cxx
+++ b/src/command/FingerprintCommands.cxx
@@ -72,12 +72,12 @@ protected:
 
 private:
 	void DecodeStream(InputStream &is, const DecoderPlugin &plugin);
-	bool DecodeStream(InputStream &is, const char *suffix,
+	bool DecodeStream(InputStream &is, std::string_view suffix,
 			  const DecoderPlugin &plugin);
 	void DecodeStream(InputStream &is);
-	bool DecodeContainer(const char *suffix, const DecoderPlugin &plugin);
-	bool DecodeContainer(const char *suffix);
-	bool DecodeFile(const char *suffix, InputStream &is,
+	bool DecodeContainer(std::string_view suffix, const DecoderPlugin &plugin);
+	bool DecodeContainer(std::string_view suffix);
+	bool DecodeFile(std::string_view suffix, InputStream &is,
 			const DecoderPlugin &plugin);
 	void DecodeFile();
 
@@ -130,17 +130,17 @@ decoder_check_plugin_mime(const DecoderPlugin &plugin,
 gcc_pure
 static bool
 decoder_check_plugin_suffix(const DecoderPlugin &plugin,
-			    const char *suffix) noexcept
+			    std::string_view suffix) noexcept
 {
 	assert(plugin.stream_decode != nullptr);
 
-	return suffix != nullptr && plugin.SupportsSuffix(suffix);
+	return !suffix.empty() && plugin.SupportsSuffix(suffix);
 }
 
 gcc_pure
 static bool
 decoder_check_plugin(const DecoderPlugin &plugin, const InputStream &is,
-		     const char *suffix) noexcept
+		     std::string_view suffix) noexcept
 {
 	return plugin.stream_decode != nullptr &&
 		(decoder_check_plugin_mime(plugin, is) ||
@@ -149,7 +149,7 @@ decoder_check_plugin(const DecoderPlugin &plugin, const InputStream &is,
 
 inline bool
 GetChromaprintCommand::DecodeStream(InputStream &is,
-				    const char *suffix,
+				    std::string_view suffix,
 				    const DecoderPlugin &plugin)
 {
 	if (!decoder_check_plugin(plugin, is, suffix))
@@ -164,8 +164,7 @@ GetChromaprintCommand::DecodeStream(InputStream &is,
 inline void
 GetChromaprintCommand::DecodeStream(InputStream &is)
 {
-	UriSuffixBuffer suffix_buffer;
-	const char *const suffix = uri_get_suffix(uri.c_str(), suffix_buffer);
+	const auto suffix = uri_get_suffix(uri.c_str());
 
 	decoder_plugins_try([this, &is, suffix](const DecoderPlugin &plugin){
 		return DecodeStream(is, suffix, plugin);
@@ -173,7 +172,7 @@ GetChromaprintCommand::DecodeStream(InputStream &is)
 }
 
 inline bool
-GetChromaprintCommand::DecodeContainer(const char *suffix,
+GetChromaprintCommand::DecodeContainer(std::string_view suffix,
 				       const DecoderPlugin &plugin)
 {
 	if (plugin.container_scan == nullptr ||
@@ -188,7 +187,7 @@ GetChromaprintCommand::DecodeContainer(const char *suffix,
 }
 
 inline bool
-GetChromaprintCommand::DecodeContainer(const char *suffix)
+GetChromaprintCommand::DecodeContainer(std::string_view suffix)
 {
 	return decoder_plugins_try([this, suffix](const DecoderPlugin &plugin){
 		return DecodeContainer(suffix, plugin);
@@ -196,7 +195,7 @@ GetChromaprintCommand::DecodeContainer(const char *suffix)
 }
 
 inline bool
-GetChromaprintCommand::DecodeFile(const char *suffix, InputStream &is,
+GetChromaprintCommand::DecodeFile(std::string_view suffix, InputStream &is,
 				  const DecoderPlugin &plugin)
 {
 	if (!plugin.SupportsSuffix(suffix))
@@ -223,8 +222,8 @@ GetChromaprintCommand::DecodeFile(const char *suffix, InputStream &is,
 inline void
 GetChromaprintCommand::DecodeFile()
 {
-	const char *suffix = uri_get_suffix(uri.c_str());
-	if (suffix == nullptr)
+	const auto suffix = uri_get_suffix(uri.c_str());
+	if (suffix.empty())
 		return;
 
 	InputStreamPtr input_stream;
diff --git a/src/db/update/Archive.cxx b/src/db/update/Archive.cxx
index 9b9531ec4..f1708dc96 100644
--- a/src/db/update/Archive.cxx
+++ b/src/db/update/Archive.cxx
@@ -157,7 +157,7 @@ UpdateWalk::UpdateArchiveFile(Directory &parent, std::string_view name,
 
 bool
 UpdateWalk::UpdateArchiveFile(Directory &directory,
-			      std::string_view name, const char *suffix,
+			      std::string_view name, std::string_view suffix,
 			      const StorageFileInfo &info) noexcept
 {
 	const ArchivePlugin *plugin = archive_plugin_from_suffix(suffix);
diff --git a/src/db/update/Container.cxx b/src/db/update/Container.cxx
index 410ddee62..e8b6eead8 100644
--- a/src/db/update/Container.cxx
+++ b/src/db/update/Container.cxx
@@ -32,7 +32,7 @@
 
 bool
 UpdateWalk::UpdateContainerFile(Directory &directory,
-				std::string_view name, const char *suffix,
+				std::string_view name, std::string_view suffix,
 				const StorageFileInfo &info) noexcept
 {
 	const DecoderPlugin *_plugin = decoder_plugins_find([suffix](const DecoderPlugin &plugin){
diff --git a/src/db/update/Playlist.cxx b/src/db/update/Playlist.cxx
index 46286bb90..46b28c767 100644
--- a/src/db/update/Playlist.cxx
+++ b/src/db/update/Playlist.cxx
@@ -88,7 +88,7 @@ UpdateWalk::UpdatePlaylistFile(Directory &parent, std::string_view name,
 
 bool
 UpdateWalk::UpdatePlaylistFile(Directory &directory,
-			       std::string_view name, const char *suffix,
+			       std::string_view name, std::string_view suffix,
 			       const StorageFileInfo &info) noexcept
 {
 	const auto *const plugin = FindPlaylistPluginBySuffix(suffix);
diff --git a/src/db/update/UpdateSong.cxx b/src/db/update/UpdateSong.cxx
index f373cb2c7..8af1c8889 100644
--- a/src/db/update/UpdateSong.cxx
+++ b/src/db/update/UpdateSong.cxx
@@ -31,7 +31,7 @@
 
 inline void
 UpdateWalk::UpdateSongFile2(Directory &directory,
-			    const char *name, const char *suffix,
+			    const char *name, std::string_view suffix,
 			    const StorageFileInfo &info) noexcept
 try {
 	Song *song;
@@ -98,7 +98,7 @@ try {
 
 bool
 UpdateWalk::UpdateSongFile(Directory &directory,
-			   const char *name, const char *suffix,
+			   const char *name, std::string_view suffix,
 			   const StorageFileInfo &info) noexcept
 {
 	if (!decoder_plugins_supports_suffix(suffix))
diff --git a/src/db/update/Walk.cxx b/src/db/update/Walk.cxx
index 3e0677e91..34955a207 100644
--- a/src/db/update/Walk.cxx
+++ b/src/db/update/Walk.cxx
@@ -189,8 +189,8 @@ UpdateWalk::UpdateRegularFile(Directory &directory,
 			      const char *name,
 			      const StorageFileInfo &info) noexcept
 {
-	const char *suffix = uri_get_suffix(name);
-	if (suffix == nullptr)
+	const auto suffix = uri_get_suffix(name);
+	if (suffix.empty())
 		return false;
 
 	return UpdateSongFile(directory, name, suffix, info) ||
diff --git a/src/db/update/Walk.hxx b/src/db/update/Walk.hxx
index dfc3e4ed9..2c275254f 100644
--- a/src/db/update/Walk.hxx
+++ b/src/db/update/Walk.hxx
@@ -86,15 +86,15 @@ private:
 	void PurgeDeletedFromDirectory(Directory &directory) noexcept;
 
 	void UpdateSongFile2(Directory &directory,
-			     const char *name, const char *suffix,
+			     const char *name, std::string_view suffix,
 			     const StorageFileInfo &info) noexcept;
 
 	bool UpdateSongFile(Directory &directory,
-			    const char *name, const char *suffix,
+			    const char *name, std::string_view suffix,
 			    const StorageFileInfo &info) noexcept;
 
 	bool UpdateContainerFile(Directory &directory,
-				 std::string_view name, const char *suffix,
+				 std::string_view name, std::string_view suffix,
 				 const StorageFileInfo &info) noexcept;
 
 
@@ -103,7 +103,7 @@ private:
 			       const char *name) noexcept;
 
 	bool UpdateArchiveFile(Directory &directory,
-			       std::string_view name, const char *suffix,
+			       std::string_view name, std::string_view suffix,
 			       const StorageFileInfo &info) noexcept;
 
 	void UpdateArchiveFile(Directory &directory, std::string_view name,
@@ -114,7 +114,7 @@ private:
 #else
 	bool UpdateArchiveFile([[maybe_unused]] Directory &directory,
 			       [[maybe_unused]] const char *name,
-			       [[maybe_unused]] const char *suffix,
+			       [[maybe_unused]] std::string_view suffix,
 			       [[maybe_unused]] const StorageFileInfo &info) noexcept {
 		return false;
 	}
@@ -125,7 +125,7 @@ private:
 				const PlaylistPlugin &plugin) noexcept;
 
 	bool UpdatePlaylistFile(Directory &directory,
-				std::string_view name, const char *suffix,
+				std::string_view name, std::string_view suffix,
 				const StorageFileInfo &info) noexcept;
 
 	bool UpdateRegularFile(Directory &directory,
diff --git a/src/decoder/Thread.cxx b/src/decoder/Thread.cxx
index 00113063d..ee948a40f 100644
--- a/src/decoder/Thread.cxx
+++ b/src/decoder/Thread.cxx
@@ -178,17 +178,17 @@ decoder_check_plugin_mime(const DecoderPlugin &plugin,
 gcc_pure
 static bool
 decoder_check_plugin_suffix(const DecoderPlugin &plugin,
-			    const char *suffix) noexcept
+			    std::string_view suffix) noexcept
 {
 	assert(plugin.stream_decode != nullptr);
 
-	return suffix != nullptr && plugin.SupportsSuffix(suffix);
+	return !suffix.empty() && plugin.SupportsSuffix(suffix);
 }
 
 gcc_pure
 static bool
 decoder_check_plugin(const DecoderPlugin &plugin, const InputStream &is,
-		     const char *suffix) noexcept
+		     std::string_view suffix) noexcept
 {
 	return plugin.stream_decode != nullptr &&
 		(decoder_check_plugin_mime(plugin, is) ||
@@ -198,7 +198,7 @@ decoder_check_plugin(const DecoderPlugin &plugin, const InputStream &is,
 static bool
 decoder_run_stream_plugin(DecoderBridge &bridge, InputStream &is,
 			  std::unique_lock<Mutex> &lock,
-			  const char *suffix,
+			  std::string_view suffix,
 			  const DecoderPlugin &plugin,
 			  bool &tried_r)
 {
@@ -216,8 +216,7 @@ decoder_run_stream_locked(DecoderBridge &bridge, InputStream &is,
 			  std::unique_lock<Mutex> &lock,
 			  const char *uri, bool &tried_r)
 {
-	UriSuffixBuffer suffix_buffer;
-	const char *const suffix = uri_get_suffix(uri, suffix_buffer);
+	const auto suffix = uri_get_suffix(uri);
 
 	const auto f = [&,suffix](const auto &plugin)
 		{ return decoder_run_stream_plugin(bridge, is, lock, suffix, plugin, tried_r); };
@@ -326,7 +325,7 @@ decoder_run_stream(DecoderBridge &bridge, const char *uri)
  * DecoderControl::mutex is not locked by caller.
  */
 static bool
-TryDecoderFile(DecoderBridge &bridge, Path path_fs, const char *suffix,
+TryDecoderFile(DecoderBridge &bridge, Path path_fs, std::string_view suffix,
 	       InputStream &input_stream,
 	       const DecoderPlugin &plugin)
 {
@@ -354,7 +353,8 @@ TryDecoderFile(DecoderBridge &bridge, Path path_fs, const char *suffix,
  * DecoderControl::mutex is not locked by caller.
  */
 static bool
-TryContainerDecoder(DecoderBridge &bridge, Path path_fs, const char *suffix,
+TryContainerDecoder(DecoderBridge &bridge, Path path_fs,
+		    std::string_view suffix,
 		    const DecoderPlugin &plugin)
 {
 	if (plugin.container_scan == nullptr ||
@@ -375,7 +375,8 @@ TryContainerDecoder(DecoderBridge &bridge, Path path_fs, const char *suffix,
  * DecoderControl::mutex is not locked by caller.
  */
 static bool
-TryContainerDecoder(DecoderBridge &bridge, Path path_fs, const char *suffix)
+TryContainerDecoder(DecoderBridge &bridge, Path path_fs,
+		    std::string_view suffix)
 {
 	return decoder_plugins_try([&bridge, path_fs,
 				    suffix](const DecoderPlugin &plugin){
@@ -394,8 +395,8 @@ TryContainerDecoder(DecoderBridge &bridge, Path path_fs, const char *suffix)
 static bool
 decoder_run_file(DecoderBridge &bridge, const char *uri_utf8, Path path_fs)
 {
-	const char *suffix = uri_get_suffix(uri_utf8);
-	if (suffix == nullptr)
+	const auto suffix = uri_get_suffix(uri_utf8);
+	if (suffix.empty())
 		return false;
 
 	InputStreamPtr input_stream;
diff --git a/src/playlist/PlaylistRegistry.cxx b/src/playlist/PlaylistRegistry.cxx
index 54540b5e4..166ff136d 100644
--- a/src/playlist/PlaylistRegistry.cxx
+++ b/src/playlist/PlaylistRegistry.cxx
@@ -157,9 +157,8 @@ playlist_list_open_uri_suffix(const char *uri, Mutex &mutex,
 {
 	assert(uri != nullptr);
 
-	UriSuffixBuffer suffix_buffer;
-	const char *const suffix = uri_get_suffix(uri, suffix_buffer);
-	if (suffix == nullptr)
+	const auto suffix = uri_get_suffix(uri);
+	if (suffix.empty())
 		return nullptr;
 
 	for (unsigned i = 0; playlist_plugins[i] != nullptr; ++i) {
@@ -272,15 +271,14 @@ playlist_list_open_stream(InputStreamPtr &&is, const char *uri)
 			return playlist;
 	}
 
-	UriSuffixBuffer suffix_buffer;
-	const char *suffix = uri != nullptr
-		? uri_get_suffix(uri, suffix_buffer)
-		: nullptr;
-	if (suffix != nullptr) {
-		auto playlist = playlist_list_open_stream_suffix(std::move(is),
-								 suffix);
-		if (playlist != nullptr)
-			return playlist;
+	if (uri != nullptr) {
+		const auto suffix = uri_get_suffix(uri);
+		if (!suffix.empty()) {
+			auto playlist = playlist_list_open_stream_suffix(std::move(is),
+									 suffix);
+			if (playlist != nullptr)
+				return playlist;
+		}
 	}
 
 	return nullptr;
diff --git a/src/util/UriExtract.cxx b/src/util/UriExtract.cxx
index 053c20cc7..31215e1c9 100644
--- a/src/util/UriExtract.cxx
+++ b/src/util/UriExtract.cxx
@@ -121,37 +121,21 @@ uri_get_path(std::string_view uri) noexcept
 }
 
 /* suffixes should be ascii only characters */
-const char *
+std::string_view
 uri_get_suffix(const char *uri) noexcept
 {
 	const char *suffix = std::strrchr(uri, '.');
 	if (suffix == nullptr || suffix == uri ||
 	    suffix[-1] == '/' || suffix[-1] == '\\')
-		return nullptr;
+		return {};
 
 	++suffix;
 
 	if (strpbrk(suffix, "/\\") != nullptr)
-		return nullptr;
+		return {};
 
-	return suffix;
-}
-
-const char *
-uri_get_suffix(const char *uri, UriSuffixBuffer &buffer) noexcept
-{
-	const char *suffix = uri_get_suffix(uri);
-	if (suffix == nullptr)
-		return nullptr;
-
-	const char *q = std::strchr(suffix, '?');
-	if (q != nullptr && size_t(q - suffix) < sizeof(buffer.data)) {
-		memcpy(buffer.data, suffix, q - suffix);
-		buffer.data[q - suffix] = 0;
-		suffix = buffer.data;
-	}
-
-	return suffix;
+	/* remove the query string */
+	return StringView(suffix).Split('?').first;
 }
 
 const char *
diff --git a/src/util/UriExtract.hxx b/src/util/UriExtract.hxx
index 05e288b30..12d1a9df4 100644
--- a/src/util/UriExtract.hxx
+++ b/src/util/UriExtract.hxx
@@ -62,20 +62,9 @@ std::string_view
 uri_get_path(std::string_view uri) noexcept;
 
 gcc_pure
-const char *
+std::string_view
 uri_get_suffix(const char *uri) noexcept;
 
-struct UriSuffixBuffer {
-	char data[8];
-};
-
-/**
- * Returns the file name suffix, ignoring the query string.
- */
-gcc_pure
-const char *
-uri_get_suffix(const char *uri, UriSuffixBuffer &buffer) noexcept;
-
 /**
  * Returns the URI fragment, i.e. the portion after the '#', but
  * without the '#'.  If there is no '#', this function returns
diff --git a/test/ContainerScan.cxx b/test/ContainerScan.cxx
index 886adb0ba..84300a775 100644
--- a/test/ContainerScan.cxx
+++ b/test/ContainerScan.cxx
@@ -37,7 +37,7 @@
 #include <stdio.h>
 
 static const DecoderPlugin *
-FindContainerDecoderPlugin(const char *suffix)
+FindContainerDecoderPlugin(std::string_view suffix)
 {
 	return decoder_plugins_find([suffix](const DecoderPlugin &plugin){
 			return plugin.container_scan != nullptr &&
@@ -48,10 +48,8 @@ FindContainerDecoderPlugin(const char *suffix)
 static const DecoderPlugin *
 FindContainerDecoderPlugin(Path path)
 {
-	UriSuffixBuffer suffix_buffer;
-	const char *const suffix = uri_get_suffix(path.ToUTF8Throw().c_str(),
-						  suffix_buffer);
-	if (suffix == nullptr)
+	const auto suffix = uri_get_suffix(path.ToUTF8Throw().c_str());
+	if (suffix.empty())
 		return nullptr;
 
 	return FindContainerDecoderPlugin(suffix);
diff --git a/test/TestUriExtract.cxx b/test/TestUriExtract.cxx
index b5e4c8656..d7d636b6a 100644
--- a/test/TestUriExtract.cxx
+++ b/test/TestUriExtract.cxx
@@ -6,28 +6,17 @@
 
 #include <gtest/gtest.h>
 
+using std::string_view_literals::operator""sv;
+
 TEST(UriExtract, Suffix)
 {
-	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo/bar"));
-	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo.jpg/bar"));
-	EXPECT_STREQ(uri_get_suffix("/foo/bar.jpg"), "jpg");
-	EXPECT_STREQ(uri_get_suffix("/foo.png/bar.jpg"), "jpg");
-	EXPECT_EQ((const char *)nullptr, uri_get_suffix(".jpg"));
-	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo/.jpg"));
+	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo/bar").data());
+	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo.jpg/bar").data());
+	EXPECT_EQ(uri_get_suffix("/foo/bar.jpg"), "jpg"sv);
+	EXPECT_EQ(uri_get_suffix("/foo.png/bar.jpg"), "jpg"sv);
+	EXPECT_EQ((const char *)nullptr, uri_get_suffix(".jpg").data());
+	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo/.jpg").data());
 
-	/* the first overload does not eliminate the query
-	   string */
-	EXPECT_STREQ(uri_get_suffix("/foo/bar.jpg?query_string"),
-		     "jpg?query_string");
-
-	/* ... but the second one does */
-	UriSuffixBuffer buffer;
-	EXPECT_STREQ(uri_get_suffix("/foo/bar.jpg?query_string", buffer),
-		     "jpg");
-
-	/* repeat some of the above tests with the second overload */
-	EXPECT_EQ((const char *)nullptr, uri_get_suffix("/foo/bar", buffer));
-	EXPECT_EQ((const char *)nullptr,
-		  uri_get_suffix("/foo.jpg/bar", buffer));
-	EXPECT_STREQ(uri_get_suffix("/foo/bar.jpg", buffer), "jpg");
+	/* eliminate the query string */
+	EXPECT_EQ(uri_get_suffix("/foo/bar.jpg?query_string"), "jpg"sv);
 }