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

Andrew Bartlett abartlet at samba.org
Sat Sep 17 22:53:42 MDT 2011


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.

Andrew Bartlett

-- 
Andrew Bartlett                                http://samba.org/~abartlet/
Authentication Developer, Samba Team           http://samba.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-lib-util-charcnv-Remove-broken-internal-CP850-and-CP.patch
Type: text/x-patch
Size: 20567 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20110917/47f77f80/attachment.bin>


More information about the samba-technical mailing list