Re: [xml] [PATCH] Character encodng cleanup

[Daniel Veillard <veillard redhat com>]

On Tue, Jul 29, 2003 at 10:36:42AM +0200, Peter Jacobi wrote:
> Hi Daniel, All,
>
> Daniel wrote:
> >   I'm on the road ATM, I will try to integrate your patch, but didn't had
> > time yet, will do for sure, if I forgot poke me again :-)
>
> I want to remind you of the UTF-16 output patch in: 
> "[PATCH] for UTF-16 output (was Re: [xml] UTF16 output broken in 2.5.8 
> (WIN32)?)" (Message-ID: <3F19318D 10784 228302 localhost>)

  Okay, thanks for the reminder.
I think that patch got applied c.f. my reply 20030719081950 I20537 redhat com
so that should be in CVS since Saturday 19.

> and the ISO-8859-2..9 (removal of broken support) patch in:
> "[xml] [PATCH] Character encodng cleanup" (Message-ID: 
> <3F1ADBC9 28168 4A84F6 localhost>)

Seems your patch misses one of the path, basically if parsing an
UTF16 document, with a Byte Order Mark, there is a memory leak if
parsing from memory. This is reproduced by
   xmllint --memory test/utf16bom.xml
This should be dependant on having iconv support and it seems it might
be a path that you didn't covered in the non-iconv case either. I fixed
it by removing the tests against UTF-16 lines 1784-1789 in 
xmlSwitchToEncoding() in parserInternals.c

> Looking into that second message, you may also want to consider whether to 
> include native (non iconv) ISO-8859-2..16 support, demonstrated in the 
> third attachment.

  Hum, okay here is my take on this:
    - the support should be integrated in encoding.c
    - this should not be seen as new API but only code compiled
      and registered in the library if LIBXML_ICONV_ENABLED is not enabled
    - I would rather have them as an internal part of the module
      (i.e. no separate .c or .h files)
    - avoiding the ISO-8859-1 duplication would be nice :-)
      I prefer the existing routine since it uses no data space.
    - keep the change within #ifdef LIBXML_ISO_8895 / #endif so that
      people on embedded systems who don't want to use the memory needed
      for the tables can avoid it easilly (I would modify configure to
      provide a switch when integrating it)
I think it then becomes the best combination:
    - no code duplication with iconv internal routines if present.
    - allow native support of the full set on all platforms
    - doesn't change the API/ABI

  if you could remod your patch to follow those directions I will be happy
to put it in,

   thanks,

Daniel

-- 
Daniel Veillard      | Red Hat Network https://rhn.redhat.com/
veillard redhat com  | libxml GNOME XML XSLT toolkit  http://xmlsoft.org/
http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/