From 01b68db30e6b4e9e036e0ae1adcdc2a35d102e43 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Tue, 12 Apr 2016 22:18:36 +0200
Subject: [PATCH] lib/icu/Converter: Create() throws exception on error

---
 Makefile.am               |  8 ++++----
 src/Main.cxx              |  5 +----
 src/fs/Charset.cxx        | 10 ++++------
 src/fs/Charset.hxx        |  7 +++++--
 src/fs/Config.cxx         | 10 ++++------
 src/fs/Config.hxx         |  6 ++++--
 src/lib/icu/Converter.cxx | 25 +++++++++++--------------
 src/lib/icu/Converter.hxx |  7 ++++---
 test/TestIcu.cxx          | 12 ++++++++----
 9 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/Makefile.am b/Makefile.am
index 6a3eb6075..1b7236b1e 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -1657,11 +1657,11 @@ test_DumpDatabase_LDADD = \
 	$(DB_LIBS) \
 	$(TAG_LIBS) \
 	libconf.a \
-	libutil.a \
 	libevent.a \
 	$(FS_LIBS) \
 	libsystem.a \
-	$(ICU_LDADD)
+	$(ICU_LDADD) \
+	libutil.a
 test_DumpDatabase_SOURCES = test/DumpDatabase.cxx \
 	src/protocol/Ack.cxx \
 	src/Log.cxx src/LogBackend.cxx \
@@ -2002,8 +2002,8 @@ test_run_convert_LDADD = \
 	libconf.a \
 	$(FS_LIBS) \
 	libsystem.a \
-	libutil.a \
-	$(ICU_LDADD)
+	$(ICU_LDADD) \
+	libutil.a
 
 test_run_output_LDADD = $(MPD_LIBS) \
 	$(PCM_LIBS) \
diff --git a/src/Main.cxx b/src/Main.cxx
index 5f91a19b7..acb4b6b67 100644
--- a/src/Main.cxx
+++ b/src/Main.cxx
@@ -510,10 +510,7 @@ static int mpd_main_after_fork(struct options options)
 try {
 	Error error;
 
-	if (!ConfigureFS(error)) {
-		LogError(error);
-		return EXIT_FAILURE;
-	}
+	ConfigureFS();
 
 	if (!glue_mapper_init(error)) {
 		LogError(error);
diff --git a/src/fs/Charset.cxx b/src/fs/Charset.cxx
index f4e9d65d3..c44238faa 100644
--- a/src/fs/Charset.cxx
+++ b/src/fs/Charset.cxx
@@ -40,19 +40,17 @@ static std::string fs_charset;
 
 static IcuConverter *fs_converter;
 
-bool
-SetFSCharset(const char *charset, Error &error)
+void
+SetFSCharset(const char *charset)
 {
 	assert(charset != nullptr);
 	assert(fs_converter == nullptr);
 
-	fs_converter = IcuConverter::Create(charset, error);
-	if (fs_converter == nullptr)
-		return false;
+	fs_converter = IcuConverter::Create(charset);
+	assert(fs_converter != nullptr);
 
 	FormatDebug(path_domain,
 		    "SetFSCharset: fs charset is: %s", fs_charset.c_str());
-	return true;
 }
 
 #endif
diff --git a/src/fs/Charset.hxx b/src/fs/Charset.hxx
index 721a41cd4..b7497ca32 100644
--- a/src/fs/Charset.hxx
+++ b/src/fs/Charset.hxx
@@ -37,8 +37,11 @@ gcc_const
 const char *
 GetFSCharset();
 
-bool
-SetFSCharset(const char *charset, Error &error);
+/**
+ * Throws std::runtime_error on error.
+ */
+void
+SetFSCharset(const char *charset);
 
 void
 DeinitFSCharset();
diff --git a/src/fs/Config.cxx b/src/fs/Config.cxx
index 87dee9657..e51da06b0 100644
--- a/src/fs/Config.cxx
+++ b/src/fs/Config.cxx
@@ -22,15 +22,13 @@
 #include "Charset.hxx"
 #include "config/ConfigGlobal.hxx"
 
-bool
-ConfigureFS(Error &error)
+void
+ConfigureFS()
 {
 #ifdef HAVE_FS_CHARSET
 	const char *charset = config_get_string(ConfigOption::FS_CHARSET);
-	return charset == nullptr || SetFSCharset(charset, error);
-#else
-	(void)error;
-	return true;
+	if (charset != nullptr)
+		SetFSCharset(charset);
 #endif
 }
 
diff --git a/src/fs/Config.hxx b/src/fs/Config.hxx
index 14b2c5580..78cda056e 100644
--- a/src/fs/Config.hxx
+++ b/src/fs/Config.hxx
@@ -26,9 +26,11 @@ class Error;
 
 /**
  * Performs global one-time initialization of this class.
+ *
+ * Throws std::runtime_error on error.
  */
-bool
-ConfigureFS(Error &error);
+void
+ConfigureFS();
 
 void
 DeinitFS();
diff --git a/src/lib/icu/Converter.cxx b/src/lib/icu/Converter.cxx
index 819a1c9e3..cb818d375 100644
--- a/src/lib/icu/Converter.cxx
+++ b/src/lib/icu/Converter.cxx
@@ -19,12 +19,13 @@
 
 #include "config.h"
 #include "Converter.hxx"
-#include "Error.hxx"
-#include "util/Error.hxx"
 #include "util/Macros.hxx"
 #include "util/AllocatedString.hxx"
 #include "util/AllocatedArray.hxx"
 #include "util/ConstBuffer.hxx"
+#include "util/FormatString.hxx"
+
+#include <stdexcept>
 
 #include <string.h>
 
@@ -32,8 +33,7 @@
 #include "Util.hxx"
 #include <unicode/ucnv.h>
 #elif defined(HAVE_ICONV)
-#include "util/Domain.hxx"
-static constexpr Domain iconv_domain("iconv");
+#include "system/Error.hxx"
 #endif
 
 #ifdef HAVE_ICU
@@ -48,30 +48,27 @@ IcuConverter::~IcuConverter()
 #ifdef HAVE_ICU_CONVERTER
 
 IcuConverter *
-IcuConverter::Create(const char *charset, Error &error)
+IcuConverter::Create(const char *charset)
 {
 #ifdef HAVE_ICU
 	UErrorCode code = U_ZERO_ERROR;
 	UConverter *converter = ucnv_open(charset, &code);
-	if (converter == nullptr) {
-		error.Format(icu_domain, int(code),
-			     "Failed to initialize charset '%s': %s",
-			     charset, u_errorName(code));
-		return nullptr;
-	}
+	if (converter == nullptr)
+		throw std::runtime_error(FormatString("Failed to initialize charset '%s': %s",
+						      charset, u_errorName(code)).c_str());
 
 	return new IcuConverter(converter);
 #elif defined(HAVE_ICONV)
 	iconv_t to = iconv_open("utf-8", charset);
 	iconv_t from = iconv_open(charset, "utf-8");
 	if (to == (iconv_t)-1 || from == (iconv_t)-1) {
-		error.FormatErrno("Failed to initialize charset '%s'",
-				  charset);
+		int e = errno;
 		if (to != (iconv_t)-1)
 			iconv_close(to);
 		if (from != (iconv_t)-1)
 			iconv_close(from);
-		return nullptr;
+		throw FormatErrno(e, "Failed to initialize charset '%s'",
+				  charset);
 	}
 
 	return new IcuConverter(to, from);
diff --git a/src/lib/icu/Converter.hxx b/src/lib/icu/Converter.hxx
index bbe25ead6..59b1e5b5a 100644
--- a/src/lib/icu/Converter.hxx
+++ b/src/lib/icu/Converter.hxx
@@ -33,8 +33,6 @@
 
 #ifdef HAVE_ICU_CONVERTER
 
-class Error;
-
 #ifdef HAVE_ICU
 struct UConverter;
 #endif
@@ -73,7 +71,10 @@ public:
 	}
 #endif
 
-	static IcuConverter *Create(const char *charset, Error &error);
+	/**
+	 * Throws std::runtime_error on error.
+	 */
+	static IcuConverter *Create(const char *charset);
 
 	/**
 	 * Convert the string to UTF-8.
diff --git a/test/TestIcu.cxx b/test/TestIcu.cxx
index 484af4f22..c4a015c4e 100644
--- a/test/TestIcu.cxx
+++ b/test/TestIcu.cxx
@@ -13,6 +13,8 @@
 #include <cppunit/ui/text/TestRunner.h>
 #include <cppunit/extensions/HelperMacros.h>
 
+#include <stdexcept>
+
 #include <string.h>
 #include <stdlib.h>
 
@@ -39,14 +41,16 @@ class TestIcuConverter : public CppUnit::TestFixture {
 
 public:
 	void TestInvalidCharset() {
-		CPPUNIT_ASSERT_EQUAL((IcuConverter *)nullptr,
-				     IcuConverter::Create("doesntexist",
-							  IgnoreError()));
+		try {
+			IcuConverter::Create("doesntexist");
+			CPPUNIT_FAIL("Exception expected");
+		} catch (const std::runtime_error &) {
+		}
 	}
 
 	void TestLatin1() {
 		IcuConverter *const converter =
-			IcuConverter::Create("iso-8859-1", IgnoreError());
+			IcuConverter::Create("iso-8859-1");
 		CPPUNIT_ASSERT(converter != nullptr);
 
 		for (const auto i : invalid_utf8) {