Am Mittwoch, 15. November 2006 18:22 schrieb Abdelrazak Younes:
> Georg Baum wrote:
> > Kornel Benko wrote:
> > 
> >> Am Mittwoch, 15. November 2006 17:09 schrieb Georg Baum:
> >>> From this I suspect that your iconv can't handle latin9. If that is 
true
> >>> the attached patch (which should go in anyway) should help, and you
> >>> should also see an error message on the console from iconv_open.
> >> It still crashes.
> >>
> >> Program received signal SIGSEGV, Segmentation fault.
> >> [Switching to Thread 1087032608 (LWP 28978)]
> >> 0x40babf4d in __gconv_close () from /lib/tls/libc.so.6
> >> (gdb) bt
> >> #0  0x40babf4d in __gconv_close () from /lib/tls/libc.so.6
> >> #1  0x40bab5dc in iconv_close () from /lib/tls/libc.so.6
> >> #2  0x08524d86 in ~IconvProcessor (this=0xbfffb3e4) at unicode.C:56
> >> #3  0x08526d1c in ~pair (this=0xbfffb3e0) at unicode.C:286
> >> #4  0x08525d13 in lyx::ucs4_to_eightbit (ucs4str=0x8ee953c, ls=56,
> >> [EMAIL PROTECTED]) at unicode.C:298
> > 
> > I see now: This is the problem. Inserting the new IconvProcessor into 
the
> > map calls the copy constructor -> iconv_close gets called twice for the
> > same conversion descriptor.
> > That means that IconvProcessor must not be copied. Abdel, do you have 
any
> > idea how to solve that?
> 
> Maybe just remove the iconv_close in the destructor? This is not 
> strictly needed I think and I put that just for clarity.

It should be called to free up system resources. I found a very elegant way 
to solve the 

> > We could implement our own copy constructor and
> > assignment operator that opens a new decsripor, but I am not sure I 
like
> > that. Basically we should be able to inherit from boost:noncopyable and
> > never copy an IconvProcessor, but then we can not use a map.
> 
> Whatever container we use the destructor will always be called on 
> insertion, I think...

Yes.

> If the iconv_close is really needed, we need just to do that explicitly 
> when we need it instead of doing that in the destructor.

I was finally able to reproduce Kornels problems with a document with three 
encodings (only two did not shopw any problem here). The problem is more 
fundamental: pimpl_ is deleted twice (because the compiler generated copy 
constructor does not call new). So in addition to the problem I wanted to 
solve with my first patch we have another one that occurs if iconv_close 
is not called.

The attached patch solves the destruction problem, too. It works fine for 
me, and I also know why.


José, if you did not already roll the alpha release I'd suggest to include 
it. It is safe enough IMO, and documents with other inputenc than utf8 are 
very likely to crash LyX without the patch. On Mac it seems that the 
probability is 100%.


Georg
Index: src/support/unicode.C
===================================================================
--- src/support/unicode.C	(Revision 15936)
+++ src/support/unicode.C	(Arbeitskopie)
@@ -39,6 +39,15 @@ static const iconv_t invalid_cd = (iconv
 
 struct IconvProcessor::Private {
 	Private(): cd(invalid_cd) {}
+	~Private()
+	{
+		if (cd != invalid_cd) {
+			if (iconv_close(cd) == -1) {
+				lyxerr << "Error returned from iconv_close("
+				       << errno << ")" << endl;
+			}
+		}
+	}
 	iconv_t cd;
 };
 
@@ -50,16 +59,27 @@ IconvProcessor::IconvProcessor(char cons
 }
 
 
-IconvProcessor::~IconvProcessor()
+IconvProcessor::IconvProcessor(IconvProcessor const & other)
+	: tocode_(other.tocode_), fromcode_(other.fromcode_),
+	  pimpl_(new IconvProcessor::Private)
 {
-	if (iconv_close(pimpl_->cd) == -1) {
-		lyxerr << "Error returned from iconv_close("
-			<< errno << ")" << endl;
-	}
-	delete pimpl_;
 }
 
 
+IconvProcessor & IconvProcessor::operator=(IconvProcessor const & other)
+{
+	if (&other == this)
+		return *this;
+	tocode_ = other.tocode_;
+	fromcode_ = other.fromcode_;
+	pimpl_.reset(new Private);
+	return *this;
+}
+
+
+IconvProcessor::~IconvProcessor() {}
+
+
 bool IconvProcessor::init()
 {
 	if (pimpl_->cd != invalid_cd)
Index: src/support/unicode.h
===================================================================
--- src/support/unicode.h	(Revision 15936)
+++ src/support/unicode.h	(Arbeitskopie)
@@ -15,6 +15,8 @@
 
 #include "support/types.h"
 
+#include <boost/scoped_ptr.hpp>
+
 #include <string>
 #include <vector>
 
@@ -27,6 +29,12 @@ public:
 	IconvProcessor(
 		char const * tocode = "",
 		char const * fromcode = "");
+	/// copy constructor needed because of pimpl_
+	IconvProcessor(IconvProcessor const &);
+	/// assignment operator needed because of pimpl_
+	IconvProcessor & operator=(IconvProcessor const &);
+	/// destructor (needs to be implemented in the .C file because the
+	/// boost::scoped_ptr destructor needs a fully defined type
 	~IconvProcessor();
 
 	/// convert any data from \c fromcode to \c tocode unicode format.
@@ -45,7 +53,7 @@ private:
 	std::string fromcode_;
 
 	struct Private;
-	Private * pimpl_;
+	boost::scoped_ptr<Private> pimpl_;
 };
 
 // utf8_to_ucs4

Reply via email to