[PATCH] remove support for CP850/CP464 on systems without iconv

[Jeremy Allison]

On Sat, Sep 17, 2011 at 09:53:42PM -0700, Andrew Bartlett wrote:
> On Mon, 2011-09-12 at 10:45 +1000, Andrew Bartlett wrote:
> > On Sat, 2011-09-10 at 22:16 +1000, Andrew Bartlett wrote:
> > 
> > > However we still have a problem with our implementation of CP850 and the
> > > other '8 bit' modules:  they fail our own tests.  Testing against the
> > > libiconv on my Fedora system shows differences in the samba4.local.iconv
> > > test ('5M random UTF16 codepoints), and in the
> > > samba.local.convert_string tests with some expected values.  The issue
> > > is that our internal module does not correctly return EINVAL and EILSEQ
> > > correctly (and the trival fixes do not seem to be correct). 
> > 
> > I should point out that these tests didn't exist in the past, or were
> > not ever run against these modules.
> > 
> > > The distinction between these error codes does matter - our code takes
> > > different paths for these two different errors.
> > > 
> > > This worries me, as subtle errors in our iconv layers tend to be
> > > incredibly difficult to chase down.  My preference would be to always
> > > rely on a system iconv, not to provide CP850 et al, and to test that
> > > with our testsuite.  We would then either not use modules at all or only
> > > use the modules for macosxfs and weird.  
> > > 
> > > However if that is not the view of the team (and it certainly may not be
> > > realistic for 3.6), then I suggest that someone may wish to investigate
> > > the generated 8-bit modules.  See the new entries in knownfail. 
> > 
> > The failures in the 8-bit modules seem to be behind the segfaults on
> > FreeBSD in the build farm (caused because that host does not have iconv
> > development libs installed).  The test is
> > samba3.blackbox.net.local.registry.roundtrip
> > 
> > This illustrates that using our internal iconv modules is not currently
> > a safe option.  How many of our supported platforms do we not have iconv
> > support available for?  Do we need DOS charset support on these
> > platforms for non-ASCII values?
> 
> Jeremy,
> 
> Per our chat on Firday, attached is a patch to remove the buggy modules,
> and require a system iconv for us to use these code pages for non-ASCII
> values.  
> 
> Once you ack this, I'll also work to remove the module loading
> mechanism, and tests.  The other modules will simply be built in, as
> there isn't the value (and there is great complexity) to having charcnv
> modules for the ones that are left.

Yes, please remove this code. I don't think it's correct and it
certainly isn't returning the right error codes for incomplete
sequences. I think we need to have a working system iconv at
this point.

If we really need this to work on a system with no iconv, the
person who re-adds them needs to make them pass all torture
tests (IMHO).

Jeremy.