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

> 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  <[hidden email]>
>
> * 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  <[hidden email]>
> +
> + * 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  <[hidden email]>
>  
>   * 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.
> +    ]
>  ]
>  
>