[Help-smalltalk] [PATCH] iconv: Handle iconv_open failures and substitute encoding names

[Holger Hans Peter Freyther]

On AMD64/FreeBSD 9.1 the GNU libiconv library does not know about
'utf8' and iconv_open returned an invalid handle address. The code
attempts to detect cases of the handle being equal to (iconv_t) -1
but this didn't work on AMD64 systems. Add a C-Function to query
the value of (iconv_t) -1 to make the code work for various pointer
sizes.

Grease is using 'utf8' as codec name but GNU libiconv does not have
support to match this to 'UTF-8'. Add some support code to replace
the codec name before calling iconv_open.

Introduce a >>#isValid that checks the iconvHandle for isNil or
the address for being an invalid handle. This should fix a potential
crash when saving an invalid handle and calling release on it.

2013-05-20  Holger Hans Peter Freyther  <address@hidden>

        * Sets.st: Add >>#isValid, >>#codecReplace: and >>#iconvInvalidHandle.
        * iconv.c: Add iconvInvalid C-Function.
        * iconvtests.st: Add test for 'utf8' replacement.
---
 packages/iconv/ChangeLog     |    6 ++++++
 packages/iconv/Sets.st       |   41 ++++++++++++++++++++++++++++++++++++-----
 packages/iconv/iconv.c       |    8 +++++++-
 packages/iconv/iconvtests.st |   14 +++++++++++++-
 4 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/packages/iconv/ChangeLog b/packages/iconv/ChangeLog
index 2d74b98..4ede4da 100644
--- a/packages/iconv/ChangeLog
+++ b/packages/iconv/ChangeLog
@@ -1,3 +1,9 @@
+2013-05-20  Holger Hans Peter Freyther  <address@hidden>
+
+       * Sets.st: Add >>#isValid, >>#codecReplace: and >>#iconvInvalidHandle.
+       * iconv.c: Add iconvInvalid C-Function.
+       * iconvtests.st: Add test for 'utf8' replacement.
+
 2010-12-04  Paolo Bonzini  <address@hidden>
 
        * package.xml: Remove now superfluous <file> tags.
diff --git a/packages/iconv/Sets.st b/packages/iconv/Sets.st
index 30b28a7..f6a0d89 100644
--- a/packages/iconv/Sets.st
+++ b/packages/iconv/Sets.st
@@ -7,7 +7,7 @@
 
 "======================================================================
 |
-| Copyright 2001, 2002, 2005, 2006, 2007, 2008 Free Software Foundation, Inc.
+| Copyright 2001, 2002, 2005, 2006, 2007, 2008, 2013 Free Software Foundation, 
Inc.
 | Written by Paolo Bonzini.
 |
 | This file is part of the GNU Smalltalk class library.
@@ -928,6 +928,19 @@ Iconv is skipped altogether and only Smalltalk converters 
are used.'>
            ifTrue: [self allInstancesDo: [:each | each release]]
     ]
 
+    codecReplace: aName [
+       "libiconv/glibc behave differently in terms of supporting
+       UTF-8/utf8. This routine is patching it up."
+       <category:'private'>
+       ^aName = 'utf8'
+           ifTrue: ['UTF-8'] ifFalse: [aName].
+    ]
+
+    iconvInvalidHandle [
+       <category: 'C call-outs'>
+       <cCall: 'iconv_invalid' returning: #cObject args: #()>
+    ]
+
     iconvOpen: to from: from [
        <category: 'C call-outs'>
        <cCall: 'iconv_open' returning: #cObject args: #(#string #string)>
@@ -1000,6 +1013,16 @@ Iconv is skipped altogether and only Smalltalk 
converters are used.'>
        ^n
     ]
 
+    isValid [
+       <category: 'accessing'>
+
+       iconvHandle isNil
+           ifTrue: [^false].
+       iconvHandle address = self iconvInvalidHandle
+           ifTrue: [^false].
+       ^true.
+    ]
+
     release [
        <category: 'private - living across snapshots'>
        self
@@ -1009,17 +1032,25 @@ Iconv is skipped altogether and only Smalltalk 
converters are used.'>
 
     finalize [
        <category: 'private - living across snapshots'>
-       iconvHandle isNil ifTrue: [^self].
+       self isValid ifFalse: [
+           iconvHandle := nil.
+           ^self].
        self iconvClose: iconvHandle.
        iconvHandle := nil
     ]
 
     iconvOpen [
+       | convTo convFrom |
        <category: 'private - living across snapshots'>
+
+       "Replace utf8 to UTF-8 or such"
+       convTo := self codecReplace: to.
+       convFrom := self codecReplace: from.
+
        iconvHandle isNil ifFalse: [self release].
-       iconvHandle := self iconvOpen: to from: from.
-       iconvHandle address = 4294967295 
-           ifTrue: [^InvalidCharsetError signal: 
+       iconvHandle := self iconvOpen: convTo from: convFrom.
+       self isValid
+           ifFalse: [^InvalidCharsetError signal:
                        {from.
                        to}].
        self addToBeFinalized.
diff --git a/packages/iconv/iconv.c b/packages/iconv/iconv.c
index 80c97c6..ba2c38a 100644
--- a/packages/iconv/iconv.c
+++ b/packages/iconv/iconv.c
@@ -7,7 +7,7 @@
 
 /***********************************************************************
  *
- * Copyright 2001, 2002, 2004, 2005, 2006 Free Software Foundation, Inc.
+ * Copyright 2001, 2002, 2004, 2005, 2006, 2013 Free Software Foundation, Inc.
  * Written by Paolo Bonzini.
  *
  * This file is part of GNU Smalltalk.
@@ -92,10 +92,16 @@ iconvWrapper (iconv_t handle, OOP readBufferOOP, int 
readPos,
   return (save_errno != EILSEQ);
 }
 
+iconv_t iconvInvalid(void)
+{
+  return (iconv_t) -1;
+}
+
 void
 gst_initModule (VMProxy * proxy)
 {
   vmProxy = proxy;
+  vmProxy->defineCFunc ("iconv_invalid", iconvInvalid);
   vmProxy->defineCFunc ("iconv_open", iconv_open);
   vmProxy->defineCFunc ("iconv_close", iconv_close);
   vmProxy->defineCFunc ("iconvWrapper", iconvWrapper);
diff --git a/packages/iconv/iconvtests.st b/packages/iconv/iconvtests.st
index 92fa2a3..90629c2 100644
--- a/packages/iconv/iconvtests.st
+++ b/packages/iconv/iconvtests.st
@@ -7,7 +7,7 @@
 
 "======================================================================
 |
-| Copyright 2007 Free Software Foundation, Inc.
+| Copyright 2007, 2013 Free Software Foundation, Inc.
 | Written by Paolo Bonzini and Stephen Compall
 |
 | This file is part of GNU Smalltalk.
@@ -243,5 +243,17 @@ TestCase subclass: IconvTest [
        b := b copyFrom: 1 to: 1024.
        self should: [ b asUnicodeString ] raise: IncompleteSequenceError.
     ]
+
+    testCodecReplacement [
+        | str unicode |
+        "Grease and others use utf8 as codec name and GNU libiconv does not
+        have support for the lookup. Check that it is working in both
+        directions."
+
+        str := #[208 184].
+        unicode := str asUnicodeString: 'utf8'.
+        self assert: unicode first = $<16r0438>.
+        self assert: (unicode asString: 'utf8') = str asString.
+    ]
 ]
 
-- 
1.7.10.4