Re: [PATCH] codecvt.h

[Phil Edwards <pedwards at disaster dot jaj dot com>]

On Mon, Nov 06, 2000 at 11:59:29AM -0800, Ulrich Drepper wrote:
> Phil Edwards <pedwards@disaster.jaj.com> writes:
> 
> > > No.  The code is correct as it is.  Don't change anything.
> > 
> > Then... what /should/ be changed?  The iconv()'s I see all take a second
> > argument of 'const char **'.  The testsuites fail due to that mismatch:
> 
> The system headers have to be fixed.  Use fixinclude.

I don't think that's going to work, at least not for Solaris.  We're going
to have to change /some/ code, somewhere.

Linux and AIX actually get a compiled fixinclude binary which can run all
those tests and tweak individual lines as needed (e.g., what David's patch
for AIX does).  But Solaris headers are close enough to standard that it
(and a few other OSes) use the "fixinc.wrap" script instead.

The fixed headers installed just do a few #defines and then do #include_next
of the real system header.  There's no way we can add or remove a 'const'
qualifier from a single function signature using that method.  Okay,
I concede that /you/ could, probably, but it would be ugly as sin.

As an experiment, I removed solaris from the list of machines which can
safely using the "wrapped" fix method, so it would build the binary and run
David's test.  Installation time went up, because now it's scanning the
whole /usr/include tree again, just like it used to in the bad old days.
I don't think forcing solaris to go /back/ to the ugly slow copy-everything
method just for the sake of correcting one function signature could possibly
be considered a win.


Before I write anything about my attempt to come up with a better solution,
I'll mention that the relevant tests fail under i686-pc-linux-gnu the
exact same way they fail under sparc-sun-solaris2.8, for whatever version
of glibc that this box is using (2.1.something).

  + Without 'const', three tests fail to compile.

  + With 'const', those same three tests compile but get runtime crashes
  from failing an assert().  For one of those, it might be a typo in
  the testsuite (I am checking the standard now).  The other two are the
  codecvt_unicode tests.  No other tests changed either way.

The /only/ difference between those two installations/checks was the
addition of 'const' for __cfrom.  Again, both solaris 8 and glibc 2.1.


The fix I used was to change the two declarations of __cfrom to

    _GLIBCPP_ICONV_CONST char* __cfrom;

and #define'd _GLIBCPP_ICONV_CONST to either 'const' or nothing, as
appropriate.

Opinions on this approach, please, before I post a patch?  I don't think
forcing every platform except Linux with glibc 2.2 to use fixincludes is
a good approach.


Phil

-- 
pedwards at disaster dot jaj dot com  |  pme at sources dot redhat dot com
devphil at several other less interesting addresses in various dot domains
The gods do not protect fools.  Fools are protected by more capable fools.