Exercise: simplify the definition of IconvProcessor

It is no longer needed to create fake copy constructors and assignment and to
deal with deletion by hand, thanks to unique_ptr, the inference of move
constructor and assignment operator, and the compatibility of standard
containers with movable objects.

src/support/unicode.cpp

@@ -49,75 +49,37 @@ namespace lyx {
 	char const * ucs4_codeset = "UCS-4LE";
 #endif
 
-static const iconv_t invalid_cd = (iconv_t)(-1);
 
-
+struct IconvProcessor::Handler {
-class IconvProcessor::Impl
+	// assumes cd is valid
-{
+	Handler(iconv_t const cd) : cd(cd) {}
-public:
+	~Handler() {
-	// noncopyable because iconv_close() is called in destructor
+		if (iconv_close(cd) == -1)
-	Impl(Impl const &) = delete;
-	Impl & operator=(Impl const &) = delete;
-
-	Impl(string const & to, string const & from)
-		: cd(invalid_cd), tocode_(to), fromcode_(from)
-	{}
-
-	~Impl()
-	{
-		if (cd != invalid_cd && iconv_close(cd) == -1)
 			LYXERR0("Error returned from iconv_close(" << errno << ')');
 	}
-
+	iconv_t const cd;
-	iconv_t cd;
-	string tocode_;
-	string fromcode_;
 };
 
 
-IconvProcessor::IconvProcessor(char const * tocode, char const * fromcode)
+IconvProcessor::IconvProcessor(string tocode, string fromcode)
-	: pimpl_(new IconvProcessor::Impl(tocode, fromcode))
+	: tocode_(tocode), fromcode_(fromcode)
-{
+{}
-}
-
-
-IconvProcessor::IconvProcessor(IconvProcessor const & other)
-	: pimpl_(new IconvProcessor::Impl(other.pimpl_->tocode_, other.pimpl_->fromcode_))
-{
-}
-
-
-IconvProcessor::~IconvProcessor()
-{
-	delete pimpl_;
-}
-
-
-IconvProcessor & IconvProcessor::operator=(IconvProcessor const & other)
-{
-	if (&other != this) {
-		delete pimpl_;
-		pimpl_ = new Impl(other.pimpl_->tocode_, other.pimpl_->fromcode_);
-	}
-	return *this;
-}
 
 
 bool IconvProcessor::init()
 {
-	if (pimpl_->cd != invalid_cd)
+	if (h_)
 		return true;
-
+	iconv_t cd = iconv_open(tocode_.c_str(), fromcode_.c_str());
-	pimpl_->cd = iconv_open(pimpl_->tocode_.c_str(), pimpl_->fromcode_.c_str());
+	if (cd != (iconv_t)(-1)) {
-	if (pimpl_->cd != invalid_cd)
+		h_ = make_unique<Handler>(cd);
 		return true;
-
+	}
 	lyxerr << "Error returned from iconv_open" << endl;
 	switch (errno) {
 	case EINVAL:
-			lyxerr << "EINVAL The conversion from " << pimpl_->fromcode_
+		lyxerr << "EINVAL The conversion from " << fromcode_ << " to "
-				<< " to " << pimpl_->tocode_
+		       << tocode_ << " is not supported by the implementation."
-				<< " is not supported by the implementation."
 		       << endl;
 		break;
 	default:
@@ -134,21 +96,19 @@ int IconvProcessor::convert(char const * buf, size_t buflen,
 	if (buflen == 0)
 		return 0;
 
-	if (pimpl_->cd == invalid_cd) {
+	if (!h_ && !init())
-		if (!init())
 		return -1;
-	}
 
 	char ICONV_CONST * inbuf = const_cast<char ICONV_CONST *>(buf);
 	size_t inbytesleft = buflen;
 	size_t outbytesleft = maxoutsize;
 
-	int res = iconv(pimpl_->cd, &inbuf, &inbytesleft, &outbuf, &outbytesleft);
+	int res = iconv(h_->cd, &inbuf, &inbytesleft, &outbuf, &outbytesleft);
 
 	// flush out remaining data. This is needed because iconv sometimes
 	// holds back chars in the stream, waiting for a combination character
 	// (see e.g. http://sources.redhat.com/bugzilla/show_bug.cgi?id=1124)
-	iconv(pimpl_->cd, NULL, NULL, &outbuf, &outbytesleft);
+	iconv(h_->cd, NULL, NULL, &outbuf, &outbytesleft);
 
 	//lyxerr << dec;
 	//lyxerr << "Inbytesleft: " << inbytesleft << endl;
@@ -167,8 +127,8 @@ int IconvProcessor::convert(char const * buf, size_t buflen,
 		case EILSEQ:
 			lyxerr << "EILSEQ An invalid multibyte sequence"
 				<< " has been encountered in the input.\n"
-				<< "When converting from " << pimpl_->fromcode_
+				<< "When converting from " << fromcode_
-				<< " to " << pimpl_->tocode_ << ".\n";
+				<< " to " << tocode_ << ".\n";
 			lyxerr << "Input:" << hex;
 			for (size_t i = 0; i < buflen; ++i) {
 				// char may be signed, avoid output of
@@ -182,8 +142,8 @@ int IconvProcessor::convert(char const * buf, size_t buflen,
 		case EINVAL:
 			lyxerr << "EINVAL An incomplete multibyte sequence"
 				<< " has been encountered in the input.\n"
-				<< "When converting from " << pimpl_->fromcode_
+				<< "When converting from " << fromcode_
-				<< " to " << pimpl_->tocode_ << ".\n";
+				<< " to " << tocode_ << ".\n";
 			lyxerr << "Input:" << hex;
 			for (size_t i = 0; i < buflen; ++i) {
 				// char may be signed, avoid output of
@@ -199,27 +159,11 @@ int IconvProcessor::convert(char const * buf, size_t buflen,
 			break;
 	}
 	// We got an error so we close down the conversion engine
-	if (iconv_close(pimpl_->cd) == -1) {
+	h_.reset();
-		lyxerr << "Error returned from iconv_close("
-			<< errno << ")" << endl;
-	}
-	pimpl_->cd = invalid_cd;
 	return -1;
 }
 
 
-std::string IconvProcessor::from() const
-{
-	return pimpl_->fromcode_;
-}
-
-
-std::string IconvProcessor::to() const
-{
-	return pimpl_->tocode_;
-}
-
-
 namespace {
 
 
@@ -310,6 +254,23 @@ IconvProcessor & ucs4ToUtf8()
 	return *processor.localData();
 }
 
+namespace {
+
+IconvProcessor & getProc(map<string, IconvProcessor> & processors,
+                         string const & encoding, bool to)
+{
+	string const & fromcode = to ? ucs4_codeset : encoding;
+	string const & tocode = to ? encoding : ucs4_codeset;
+	map<string, IconvProcessor>::iterator const it = processors.find(encoding);
+	if (it == processors.end()) {
+		IconvProcessor p(fromcode, tocode);
+		return processors.insert(make_pair(encoding, move(p))).first->second;
+	} else
+		return it->second;
+}
+
+} //anon namespace
+
 
 vector<char>
 ucs4_to_utf8(char_type c)
@@ -342,11 +303,8 @@ eightbit_to_ucs4(char const * s, size_t ls, string const & encoding)
 	if (!static_processors.hasLocalData())
 		static_processors.setLocalData(new map<string, IconvProcessor>);
 	map<string, IconvProcessor> & processors = *static_processors.localData();
-	if (processors.find(encoding) == processors.end()) {
+	IconvProcessor & processor = getProc(processors, encoding, true);
-		IconvProcessor processor(ucs4_codeset, encoding.c_str());
+	return iconv_convert<char_type>(processor, s, ls);
-		processors.insert(make_pair(encoding, processor));
-	}
-	return iconv_convert<char_type>(processors[encoding], s, ls);
 }
 
 
@@ -362,30 +320,21 @@ map<string, IconvProcessor> & ucs4To8bitProcessors()
 
 }
 
-
 vector<char>
 ucs4_to_eightbit(char_type const * ucs4str, size_t ls, string const & encoding)
 {
 	map<string, IconvProcessor> & processors(ucs4To8bitProcessors());
-	if (processors.find(encoding) == processors.end()) {
+	IconvProcessor & processor = getProc(processors, encoding, false);
-		IconvProcessor processor(encoding.c_str(), ucs4_codeset);
+	return iconv_convert<char>(processor, ucs4str, ls);
-		processors.insert(make_pair(encoding, processor));
-	}
-	return iconv_convert<char>(processors[encoding], ucs4str, ls);
 }
 
 
 char ucs4_to_eightbit(char_type ucs4, string const & encoding)
 {
 	map<string, IconvProcessor> & processors(ucs4To8bitProcessors());
-	map<string, IconvProcessor>::iterator it = processors.find(encoding);
+	IconvProcessor & processor = getProc(processors, encoding, false);
-	if (it == processors.end()) {
-		IconvProcessor processor(encoding.c_str(), ucs4_codeset);
-		it = processors.insert(make_pair(encoding, processor)).first;
-	}
-
 	char out;
-	int const bytes = it->second.convert((char *)(&ucs4), 4, &out, 1);
+	int const bytes = processor.convert((char *)(&ucs4), 4, &out, 1);
 	if (bytes > 0)
 		return out;
 	return 0;
@@ -399,14 +348,9 @@ void ucs4_to_multibytes(char_type ucs4, vector<char> & out,
 	if (!static_processors.hasLocalData())
 		static_processors.setLocalData(new map<string, IconvProcessor>);
 	map<string, IconvProcessor> & processors = *static_processors.localData();
-	map<string, IconvProcessor>::iterator it = processors.find(encoding);
+	IconvProcessor & processor = getProc(processors, encoding, false);
-	if (it == processors.end()) {
-		IconvProcessor processor(encoding.c_str(), ucs4_codeset);
-		it = processors.insert(make_pair(encoding, processor)).first;
-	}
-
 	out.resize(4);
-	int bytes = it->second.convert((char *)(&ucs4), 4, &out[0], 4);
+	int bytes = processor.convert((char *)(&ucs4), 4, &out[0], 4);
 	if (bytes > 0)
 		out.resize(bytes);
 	else

src/support/unicode.h

@@ -1,3 +1,4 @@
+// -*- C++ -*-
 /**
  * \file unicode.h
  * This file is part of LyX, the document processor.
@@ -14,8 +15,10 @@
 #define LYX_SUPPORT_UNICODE_H
 
 #include "support/strfwd.h"
+#include "support/unique_ptr.h"
 
 #include <cstddef>
+#include <string>
 #include <vector>
 
 
@@ -44,32 +47,23 @@ namespace lyx {
  */
 class IconvProcessor
 {
+	/// open iconv.
+	/// \return true if the processor is ready to use.
+	bool init();
+	std::string const tocode_;
+	std::string const fromcode_;
+	struct Handler;
+	unique_ptr<Handler> h_;
 public:
-	IconvProcessor(char const * tocode = "", char const * fromcode = "");
+	IconvProcessor(std::string tocode, std::string fromcode);
-	/// copy constructor needed because of pimpl_
-	IconvProcessor(IconvProcessor const &);
-	/// assignment operator needed because of pimpl_
-	IconvProcessor & operator=(IconvProcessor const &);
-	/// destructor
-	~IconvProcessor();
-
 	/// convert any data from \c fromcode to \c tocode unicode format.
 	/// \return the number of bytes of the converted output buffer.
 	int convert(char const * in_buffer, size_t in_size,
 		char * out_buffer, size_t max_out_size);
-
-	/// source encoding
-	std::string from() const;
 	/// target encoding
-	std::string to() const;
+	std::string to() const { return tocode_; }
-
+	// required by g++ 4.7
-private:
+	IconvProcessor(IconvProcessor &&) = default;
-	/// open iconv.
-	/// \return true if the processor is ready to use.
-	bool init();
-	/// hide internals
-	class Impl;
-	Impl * pimpl_;
 };
 
 /// Get the global IconvProcessor instance of the current thread for