From 12e75a523acde305cd0b9affb9f5cc6298ada3b8 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@musicpd.org>
Date: Fri, 31 May 2019 19:01:22 +0200
Subject: [PATCH] archive/ArchiveLookup: remove "suffix" output parameter

Let the caller do this.  Our GetSuffix() function was broken anyway.
---
 src/archive/ArchiveLookup.cxx            | 17 +----------------
 src/archive/ArchiveLookup.hxx            |  7 +++----
 src/input/plugins/ArchiveInputPlugin.cxx |  6 ++++--
 test/test_archive.cxx                    | 16 +++++++---------
 4 files changed, 15 insertions(+), 31 deletions(-)

diff --git a/src/archive/ArchiveLookup.cxx b/src/archive/ArchiveLookup.cxx
index 49f975aa6..44c0cfe30 100644
--- a/src/archive/ArchiveLookup.cxx
+++ b/src/archive/ArchiveLookup.cxx
@@ -36,21 +36,9 @@ FindSlash(char *p, size_t i) noexcept
 	return nullptr;
 }
 
-gcc_pure
-static const char *
-FindSuffix(const char *p, const char *i) noexcept
-{
-	for (; i > p; --i) {
-		if (*i == '.')
-			return i + 1;
-	}
-
-	return nullptr;
-}
-
 bool
 archive_lookup(char *pathname, const char **archive,
-	       const char **inpath, const char **suffix)
+	       const char **inpath)
 {
 	size_t idx = strlen(pathname);
 
@@ -70,9 +58,6 @@ archive_lookup(char *pathname, const char **archive,
 				//so the upper should be file
 				*archive = pathname;
 				*inpath = slash + 1;
-
-				//try to get suffix
-				*suffix = FindSuffix(pathname, slash - 1);
 				return true;
 			} else {
 				FormatError(archive_domain,
diff --git a/src/archive/ArchiveLookup.hxx b/src/archive/ArchiveLookup.hxx
index c217d56f2..6f42ff7f5 100644
--- a/src/archive/ArchiveLookup.hxx
+++ b/src/archive/ArchiveLookup.hxx
@@ -24,24 +24,23 @@
  *
  * archive_lookup is used to determine if part of pathname refers to an regular
  * file (archive). If so then its also used to split pathname into archive file
- * and path used to locate file in archive. It also returns suffix of the file.
+ * and path used to locate file in archive.
  * How it works:
  * We do stat of the parent of input pathname as long as we find an regular file
  * Normally this should never happen. When routine returns true pathname modified
- * and split into archive, inpath and suffix. Otherwise nothing happens
+ * and split into archive and inpath. Otherwise nothing happens
  *
  * For example:
  *
  * /music/path/Talco.zip/Talco - Combat Circus/12 - A la pachenka.mp3
  * is split into archive:	/music/path/Talco.zip
  * inarchive pathname:		Talco - Combat Circus/12 - A la pachenka.mp3
- * and suffix:			 zip
  *
  * Throws on error.
  */
 bool
 archive_lookup(char *pathname, const char **archive,
-	       const char **inpath, const char **suffix);
+	       const char **inpath);
 
 #endif
 
diff --git a/src/input/plugins/ArchiveInputPlugin.cxx b/src/input/plugins/ArchiveInputPlugin.cxx
index 0f3954755..cd918ef37 100644
--- a/src/input/plugins/ArchiveInputPlugin.cxx
+++ b/src/input/plugins/ArchiveInputPlugin.cxx
@@ -44,9 +44,9 @@ OpenArchiveInputStream(Path path, Mutex &mutex)
 	};
 
 	// archive_lookup will modify pname when true is returned
-	const char *archive, *filename, *suffix;
+	const char *archive, *filename;
 	try {
-		if (!archive_lookup(pname, &archive, &filename, &suffix)) {
+		if (!archive_lookup(pname, &archive, &filename)) {
 			FormatDebug(archive_domain,
 				    "not an archive, lookup %s failed", pname);
 			return nullptr;
@@ -57,6 +57,8 @@ OpenArchiveInputStream(Path path, Mutex &mutex)
 		return nullptr;
 	}
 
+	const char *suffix = Path::FromFS(archive).GetSuffix();
+
 	//check which archive plugin to use (by ext)
 	arplug = archive_plugin_from_suffix(suffix);
 	if (!arplug) {
diff --git a/test/test_archive.cxx b/test/test_archive.cxx
index 7b92cea4b..ebab890ea 100644
--- a/test/test_archive.cxx
+++ b/test/test_archive.cxx
@@ -8,41 +8,39 @@
 
 TEST(ArchiveTest, Lookup)
 {
-	const char *archive, *inpath, *suffix;
+	const char *archive, *inpath;
 
 	char *path = strdup("");
-	EXPECT_THROW(archive_lookup(path, &archive, &inpath, &suffix),
+	EXPECT_THROW(archive_lookup(path, &archive, &inpath),
 		     std::system_error);
 	free(path);
 
 	path = strdup(".");
-	EXPECT_FALSE(archive_lookup(path, &archive, &inpath, &suffix));
+	EXPECT_FALSE(archive_lookup(path, &archive, &inpath));
 	free(path);
 
 	path = strdup("config.h");
-	EXPECT_FALSE(archive_lookup(path, &archive, &inpath, &suffix));
+	EXPECT_FALSE(archive_lookup(path, &archive, &inpath));
 	free(path);
 
 	path = strdup("src/foo/bar");
-	EXPECT_THROW(archive_lookup(path, &archive, &inpath, &suffix),
+	EXPECT_THROW(archive_lookup(path, &archive, &inpath),
 		     std::system_error);
 	free(path);
 
 	fclose(fopen("dummy", "w"));
 
 	path = strdup("dummy/foo/bar");
-	EXPECT_TRUE(archive_lookup(path, &archive, &inpath, &suffix));
+	EXPECT_TRUE(archive_lookup(path, &archive, &inpath));
 	EXPECT_EQ((const char *)path, archive);
 	EXPECT_STREQ(archive, "dummy");
 	EXPECT_STREQ(inpath, "foo/bar");
-	EXPECT_EQ((const char *)nullptr, suffix);
 	free(path);
 
 	path = strdup("config.h/foo/bar");
-	EXPECT_TRUE(archive_lookup(path, &archive, &inpath, &suffix));
+	EXPECT_TRUE(archive_lookup(path, &archive, &inpath));
 	EXPECT_EQ((const char *)path, archive);
 	EXPECT_STREQ(archive, "config.h");
 	EXPECT_STREQ(inpath, "foo/bar");
-	EXPECT_STREQ(suffix, "h");
 	free(path);
 }