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

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|

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

Holger 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  <[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.
+    ]
 ]
 
--
1.7.10.4


_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk
Reply | Threaded
Open this post in threaded view
|

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

Paolo Bonzini-2
Il 20/05/2013 19:36, Holger Hans Peter Freyther ha scritto:

> 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.
> +    ]
>  ]
>  
>

Ok.

Paolo

_______________________________________________
help-smalltalk mailing list
[hidden email]
https://lists.gnu.org/mailman/listinfo/help-smalltalk