apr-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mo DeJong <mdej...@cygnus.com>
Subject Re: [PATCH] iconv tweak
Date Wed, 07 Feb 2001 19:58:42 GMT
On 7 Feb 2001, Jeff Trawick wrote:

> Doesn't AC_TRY_COMPILE() succeed if there is a warning?  I think your
> patch only helps on rare systems where the compiler won't compile it
> at all if the const-ness doesn't match the iconv() prototype.  Yeah,
> that is an improvement over the current situation but it would be nice
> to get rid of the problem -- warning or error -- altogether.

We are testing for the error condition, that is why the #define
is called HAVE_ERROR_CALL_ICONV_CONST. The error shows up on systems
with the combo of a non const header file and a gcc newer than
2.95 (like Red Hat 7.0).
 
> Mo DeJong <mdejong@cygnus.com> writes:
> 
> > On Sun, 4 Feb 2001, Greg Stein wrote:
> > 
> > > There was some discussion about iconv prototyes here recently, but I kind of
> > > missed it.
> > > 
> > > Do we always need to live with a warning about a type mismatch, or is the
> > > cast below appropriate?
> > > 
> > > IOW, do some headers have "const" on them, and others not?
> > 
> > Yup, you cannot just use a cast, you need to use a HAVE...
> > test because headers differ and newer versions of
> > gcc will generate an error on a plain cast.
> 
> What does "plain cast" in this sense mean?

A cast that does not happen using the ifdef
HAVE_ERROR_CALL_ICONV_CONST conditional logic.
The point was that just casting to char **
would fail on some systems.

> > Here is the bit of code from the Jikes java compiler
> > that deals with this issue.
> > 
> >                     size_t n = iconv(_converter,
> > #ifdef HAVE_ERROR_CALL_ICONV_CONST
> >                                      (char **)
> > #endif
> >                                      &source_ptr, &srcl,
> >                                      (char **)&chp, &chl
> >                     );
> 
> The code would be nicer if we define ICONV_INBUF_TYPE to "char **" or
> "const char **" and then do

Well, that would just be casting a const char** to a const char **
in the normal case, why put an extra cast in for no reason?
I tend to like knowing about the exceptions and why they are in the
code instead of having unexplained casts all over the place.
 
>   n = iconv(_converter,
>             (ICONV_INBUF_TYPE)&source_ptr, &srcl,
>             (char **)&chp, &chl);
>
> but that is a minor detail (particularly since we have so few calls to
> iconv).
> 
> Is it reasonable to test for a compile *warning* via autoconf?

I am not sure why that matters. The point is that apr does not
compile on some systems, this iconv #define code will fix that.

Mo DeJong
Red Hat Inc

Mime
View raw message