You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@apr.apache.org by Greg Stein <gs...@lyra.org> on 2001/02/05 06:26:39 UTC
[PATCH] iconv tweak
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?
Cheers,
-g
Index: xlate.c
===================================================================
RCS file: /home/cvs/apr/i18n/unix/xlate.c,v
retrieving revision 1.18
diff -u -r1.18 xlate.c
--- xlate.c 2001/01/28 11:33:52 1.18
+++ xlate.c 2001/02/05 05:23:19
@@ -194,7 +194,7 @@
}
inbytes_left = outbytes_left = sizeof(inbuf);
- translated = iconv(convset->ich, &inbufptr,
+ translated = iconv(convset->ich, (const char **)&inbufptr,
&inbytes_left, &outbufptr, &outbytes_left);
if (translated != (size_t) -1 &&
inbytes_left == 0 &&
@@ -288,7 +288,7 @@
char *inbufptr = (char *)inbuf;
char *outbufptr = outbuf;
- translated = iconv(convset->ich, &inbufptr,
+ translated = iconv(convset->ich, (const char **)&inbufptr,
inbytes_left, &outbufptr, outbytes_left);
/* If everything went fine but we ran out of buffer, don't
* report it as an error. Caller needs to look at the two
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] iconv tweak
Posted by Sam TH <sa...@uchicago.edu>.
On Sun, Feb 04, 2001 at 09:26:39PM -0800, 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?
Yes, sadly, iconv() prototypes differ on this issue. It isn't any fun
at all. And if that wasn't enough, sometimes there is no one way to
get it to work on all compilers, with all prototypes. This
neccessitates some ugly hacks in the AbiWord code, for example.
(another reason to use APR).
Everybody thank the POSIX folks for changing this midstream.
sam th
sam@uchicago.edu
http://www.abisource.com/~sam/
GnuPG Key:
http://www.abisource.com/~sam/key
Re: [PATCH] iconv tweak
Posted by Sam TH <sa...@uchicago.edu>.
On Wed, Feb 07, 2001 at 11:58:42AM -0800, Mo DeJong wrote:
> On 7 Feb 2001, Jeff Trawick wrote:
> > > 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.
>
Just for reference, in AbiWord this is dealt with in the following way
iconv(arg_1, (ICONV_CONST char **) arg_2, etc ...)
where ICONV_CONST is either defined to const or not depending on the
platform.
I have to say that this is ugly and horrible, and really wish there
was a better solution.
sam th
sam@uchicago.edu
http://www.abisource.com/~sam/
GnuPG Key:
http://www.abisource.com/~sam/key
Re: [PATCH] iconv tweak
Posted by Jeff Trawick <tr...@bellsouth.net>.
Sam TH <sa...@uchicago.edu> writes:
> The easiest way would probably be to use c++ to test this, since in
> c++ cast from a char ** to a const char ** is illegal, and generates
> and error.
c++ is cool, but we can't require it so we'd have to see if it failed
due to no c++, then back down to the c test
--
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] iconv tweak
Posted by Sam TH <sa...@uchicago.edu>.
On Wed, Feb 07, 2001 at 01:14:59PM -0500, Jeff Trawick wrote:
>
> Is it reasonable to test for a compile *warning* via autoconf?
>
The easiest way would probably be to use c++ to test this, since in
c++ cast from a char ** to a const char ** is illegal, and generates
and error.
sam th
sam@uchicago.edu
http://www.abisource.com/~sam/
GnuPG Key:
http://www.abisource.com/~sam/key
Re: [PATCH] iconv tweak
Posted by Jeff Trawick <tr...@bellsouth.net>.
Mo DeJong <md...@cygnus.com> writes:
>> 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.
Warnings do matter, hence my questions.
I certainly agree that it is important to get all systems compiling
even if we have to have warnings; I'm just wondering about a better
solution.
Thanks,
Jeff
--
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] iconv tweak
Posted by Mo DeJong <md...@cygnus.com>.
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 <md...@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
Re: [PATCH] iconv tweak
Posted by Jeff Trawick <tr...@bellsouth.net>.
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.
Mo DeJong <md...@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?
> 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
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?
--
Jeff Trawick | trawickj@bellsouth.net | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] iconv tweak
Posted by Mo DeJong <md...@cygnus.com>.
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.
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
);
And here is the autoconf macro that checks for
the error that will require a cast.
dnl @synopsis AC_CHECK_ICONV
dnl
dnl Check to see if the iconv functions and libs are present
dnl @version $Id: ac_check_iconv.m4,v 1.1 2000/10/10 07:06:50 mdejong Exp $
dnl @author Mo DeJong <md...@redhat.com>
dnl
AC_DEFUN(AC_CHECK_ICONV,
[
dnl Check for the iconv header file
AC_CHECK_HEADERS(iconv.h)
dnl Check if iconv library is present
AC_CHECK_LIB(iconv, iconv)
iconv_includes="
#ifdef HAVE_STRING_H
# if !STDC_HEADERS && HAVE_MEMORY_H
# include <memory.h>
# endif
# include <string.h>
#else
# ifdef HAVE_STRINGS_H
# include <strings.h>
# endif
#endif
#ifdef HAVE_WCHAR_H
# include <wchar.h>
#endif
#ifdef HAVE_ICONV_H
# include <iconv.h>
#endif
"
dnl Make sure we can compile a simple call to iconv
AC_CACHE_CHECK(simple call to iconv,
ac_cv_call_iconv,
[
AC_TRY_COMPILE($iconv_includes, [iconv(0, NULL, NULL, NULL, NULL)],
ac_cv_call_iconv=yes,
ac_cv_call_iconv=no)
])
dnl No additional testing is possible if regular call does not work.
dnl In this case, we just assume iconv can not be used by the application
if test "$ac_cv_call_iconv" = "yes" ; then
dnl Check for declaration of iconv without const qualifier in 2nd argument
AC_CACHE_CHECK(for error calling iconv with const 2nd argument,
ac_cv_error_call_iconv_const,
[AC_TRY_COMPILE($iconv_includes, [
const char ** s = NULL;
iconv(0, s, NULL, NULL, NULL);
],
ac_cv_error_call_iconv_const=no, ac_cv_error_call_iconv_const=yes)
])
if test "$ac_cv_error_call_iconv_const" = "yes"; then
dnl define symbol so we know when to cast the argument to iconv
dnl from const char ** to char *
AC_DEFINE(HAVE_ERROR_CALL_ICONV_CONST, ,
[Defined when the compiler would generate an error on a call to iconv
with a non const 2nd argument. This is a known problem on IRIX systems.])
dnl Double check that our suggested fix works.
AC_CACHE_CHECK(fix for calling iconv with non const argument,
ac_cv_call_iconv_non_const,
[AC_TRY_COMPILE($iconv_includes, [
const wchar_t * s = NULL;
iconv( 0,
#ifdef HAVE_ERROR_CALL_ICONV_CONST
(char **)
#endif
s, NULL, NULL, NULL);
],
ac_cv_call_iconv_non_const=yes,
AC_MSG_ERROR([Could not compile with iconv const cast fix])
)])
fi
fi
]
)
cheers
Mo DeJong
Red Hat Inc