[PATCH] a somewhat different approach to the iconv() issue
There are some ideas here previously described by Mo DeJong.
Additionally:
. there is a way to avoid a warning for a certain platform:
set apr_iconv_inbuf_const to "1" in hints.m4 for platforms
where the parm is "const char **" but the autoconf logic
doesn't detect
. we *know* that versions of glibc < 2.2 have "const char **"
instead of "char **" so make that work without any hints.m4
stuff (which would have to look at the glibc version)
This should get RedHat 7.0 compiling cleanly, but I don't have such a
system to test with.
My RedHat 6.0 system ("const char **") now compiles xlate.c cleanly
for the first time in a while.
A Tru64 system ("char **") still compiles xlate.c cleanly.
To get AIX compiling xlate.c cleanly (it hasn't done so in a while)
I'd expect to have to add apr_iconv_inbuf_const=1 to the AIX stanza in
hints.m4. I haven't tested that though. The same is true for
Solaris, which is "const char **" like AIX.
Comments?
Index: apr_common.m4
===
RCS file: /home/cvspublic/apr/apr_common.m4,v
retrieving revision 1.11
diff -u -r1.11 apr_common.m4
--- apr_common.m4 2001/01/11 13:55:58 1.11
+++ apr_common.m4 2001/02/08 00:01:57
@@ -285,3 +285,36 @@
$1="$$1 $2"; export $1
])
+dnl
+dnl APR_CHECK_ICONV_INBUF
+dnl
+dnl Decide whether or not the inbuf parameter to iconv() is const.
+dnl
+dnl We try to compile something without const. If it fails to
+dnl compile, we assume that the system's iconv() has const.
+dnl Unfortunately, we won't realize when there was a compile
+dnl warning, so we allow a variable -- apr_iconv_inbuf_const -- to
+dnl be set in hints.m4 to specify whether or not iconv() has const
+dnl on this parameter.
+dnl
+AC_DEFUN(APR_CHECK_ICONV_INBUF,[
+AC_MSG_CHECKING(for type of inbuf parameter to iconv)
+if test "x$apr_iconv_inbuf_const" = "x"; then
+AC_TRY_COMPILE([
+#include
+#include
+],[
+#if defined(__GLIBC__) && __GLIBC__ == 2 && __GLIBC_MINOR < 2
+#error We know this version of glibc has const char **, so fail this
compile
+#endif
+iconv(0,(char **)0,(size_t *)0,(char **)0,(size_t *)0);
+], apr_iconv_inbuf_const="0", apr_iconv_inbuf_const="1")
+fi
+if test "$apr_iconv_inbuf_const" = "1"; then
+AC_DEFINE(APR_ICONV_INBUF_CONST, 1, [Define if the inbuf parm to iconv()
is const char **])
+msg="const char **"
+else
+msg="char **"
+fi
+AC_MSG_RESULT([$msg])
+])
Index: configure.in
===
RCS file: /home/cvspublic/apr/configure.in,v
retrieving revision 1.220
diff -u -r1.220 configure.in
--- configure.in2001/01/28 12:18:38 1.220
+++ configure.in2001/02/08 00:02:07
@@ -245,6 +245,9 @@
AC_CHECK_FUNC(_getch)
AC_CHECK_FUNCS(gmtime_r localtime_r)
AC_CHECK_FUNCS(iconv, [ iconv="1" ], [ iconv="0" ])
+if test "$iconv" = "1"; then
+ APR_CHECK_ICONV_INBUF
+fi
AC_CHECK_FUNCS(mmap, [ mmap="1" ], [ mmap="0" ])
if test "$native_mmap_emul" = "1"; then
mmap="1"
Index: i18n/unix/xlate.c
===
RCS file: /home/cvspublic/apr/i18n/unix/xlate.c,v
retrieving revision 1.18
diff -u -r1.18 xlate.c
--- i18n/unix/xlate.c 2001/01/28 11:33:52 1.18
+++ i18n/unix/xlate.c 2001/02/08 00:02:33
@@ -80,6 +80,12 @@
#include
#endif
+#ifdef APR_ICONV_INBUF_CONST
+#define ICONV_INBUF_TYPE const char **
+#else
+#define ICONV_INBUF_TYPE char **
+#endif
+
#ifndef min
#define min(x,y) ((x) <= (y) ? (x) : (y))
#endif
@@ -194,7 +200,7 @@
}
inbytes_left = outbytes_left = sizeof(inbuf);
-translated = iconv(convset->ich, &inbufptr,
+translated = iconv(convset->ich, (ICONV_INBUF_TYPE)&inbufptr,
&inbytes_left, &outbufptr, &outbytes_left);
if (translated != (size_t) -1 &&
inbytes_left == 0 &&
@@ -288,7 +294,7 @@
char *inbufptr = (char *)inbuf;
char *outbufptr = outbuf;
-translated = iconv(convset->ich, &inbufptr,
+translated = iconv(convset->ich, (ICONV_INBUF_TYPE)&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
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] a somewhat different approach to the iconv() issue
Awesome.
+1
On Wed, Feb 07, 2001 at 07:35:23PM -0500, Jeff Trawick wrote:
> There are some ideas here previously described by Mo DeJong.
> Additionally:
>
> . there is a way to avoid a warning for a certain platform:
> set apr_iconv_inbuf_const to "1" in hints.m4 for platforms
> where the parm is "const char **" but the autoconf logic
> doesn't detect
>
> . we *know* that versions of glibc < 2.2 have "const char **"
> instead of "char **" so make that work without any hints.m4
> stuff (which would have to look at the glibc version)
>
> This should get RedHat 7.0 compiling cleanly, but I don't have such a
> system to test with.
>
> My RedHat 6.0 system ("const char **") now compiles xlate.c cleanly
> for the first time in a while.
>
> A Tru64 system ("char **") still compiles xlate.c cleanly.
>
> To get AIX compiling xlate.c cleanly (it hasn't done so in a while)
> I'd expect to have to add apr_iconv_inbuf_const=1 to the AIX stanza in
> hints.m4. I haven't tested that though. The same is true for
> Solaris, which is "const char **" like AIX.
>
> Comments?
>...
--
Greg Stein, http://www.lyra.org/
Re: [PATCH] iconv tweak
Sam TH <[EMAIL PROTECTED]> 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 | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: [PATCH] a somewhat different approach to the iconv() issue
> . we *know* that versions of glibc < 2.2 have "const char **" > instead of "char **" so make that work without any hints.m4 > stuff (which would have to look at the glibc version) Why not check for GCC and simply add -Werror to the compile? > This should get RedHat 7.0 compiling cleanly, but I don't have such a > system to test with. I do, but my glibc has already been updated. One warning: the glibc update RPM distributed by RedHat is completely broken -- among other things, it installs a timezone for EST and breaks ld.conf. I think the final patch should remove the first cast of inbufptr that I added in my initial "fix". Roy
Re: apr_ function prefixes
On Tue, 6 Feb 2001, Roy T. Fielding wrote: > +1 a.s.a.p. ok, i'm planning todo it later this evening.
Re: [PATCH] a somewhat different approach to the iconv() issue
"Roy T. Fielding" <[EMAIL PROTECTED]> writes:
> > . we *know* that versions of glibc < 2.2 have "const char **"
> > instead of "char **" so make that work without any hints.m4
> > stuff (which would have to look at the glibc version)
>
> Why not check for GCC and simply add -Werror to the compile?
excellent...
...
> I think the final patch should remove the first cast of inbufptr
> that I added in my initial "fix".
You're talking about this I presume...
@@ -285,7 +285,7 @@
size_t translated;
if (convset->ich != (iconv_t)-1) {
-char *inbufptr = inbuf;
+char *inbufptr = (char *)inbuf;
char *outbufptr = outbuf;
translated = iconv(convset->ich, &inbufptr,
Thanks,
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] a somewhat different approach to the iconv() issue
Jeff Trawick <[EMAIL PROTECTED]> writes:
> "Roy T. Fielding" <[EMAIL PROTECTED]> writes:
>
> > > . we *know* that versions of glibc < 2.2 have "const char **"
> > > instead of "char **" so make that work without any hints.m4
> > > stuff (which would have to look at the glibc version)
> >
> > Why not check for GCC and simply add -Werror to the compile?
>
> excellent...
well, in practice it isn't so excellent :( the template for
AC_TRY_COMPILE() doesn't even compile without warnings
int main() {
configure:4111: warning: function declaration isn't a prototype
--
Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site:
http://www.geocities.com/SiliconValley/Park/9289/
Born in Roswell... married an alien...
Re: [PATCH] a somewhat different approach to the iconv() issue
On 7 Feb 2001, Jeff Trawick wrote: > Jeff Trawick <[EMAIL PROTECTED]> writes: > > > "Roy T. Fielding" <[EMAIL PROTECTED]> writes: > > > > > > . we *know* that versions of glibc < 2.2 have "const char **" > > > > instead of "char **" so make that work without any hints.m4 > > > > stuff (which would have to look at the glibc version) > > > > > > Why not check for GCC and simply add -Werror to the compile? > > > > excellent... > > well, in practice it isn't so excellent :( the template for > AC_TRY_COMPILE() doesn't even compile without warnings Does anyone around here remember the -Werror thread? - Sascha
Re: [PATCH] a somewhat different approach to the iconv() issue
Sascha Schumann <[EMAIL PROTECTED]> writes: > On 7 Feb 2001, Jeff Trawick wrote: > > > Jeff Trawick <[EMAIL PROTECTED]> writes: > > > > > "Roy T. Fielding" <[EMAIL PROTECTED]> writes: > > > > > > > > . we *know* that versions of glibc < 2.2 have "const char **" > > > > > instead of "char **" so make that work without any hints.m4 > > > > > stuff (which would have to look at the glibc version) > > > > > > > > Why not check for GCC and simply add -Werror to the compile? > > > > > > excellent... > > > > well, in practice it isn't so excellent :( the template for > > AC_TRY_COMPILE() doesn't even compile without warnings > > > > Does anyone around here remember the -Werror thread? Well now I remember :) (off to pluck some gray hairs) -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: [PATCH] a somewhat different approach to the iconv() issue
> well, in practice it isn't so excellent :( the template for
> AC_TRY_COMPILE() doesn't even compile without warnings
>
> int main() {
> configure:4111: warning: function declaration isn't a prototype
So we write our own macro that does compile without warnings.
What was checked in doesn't work -- I now get warnings again on
my platform, and mine is the one with the correct prototypes.
I'll try to fix it.
Roy
Re: [PATCH] a somewhat different approach to the iconv() issue
"Roy T. Fielding" <[EMAIL PROTECTED]> writes: > What was checked in doesn't work -- I now get warnings again on > my platform, and mine is the one with the correct prototypes. Hmmm... I get no warnings on a system of each flavor (Tru64 and RedHat 6) and got no warnings. What warnings do you get? Did the configure output display the right message? checking for type of inbuf parameter to iconv... char ** -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: apr_ function prefixes
to see the files that will change and the line number/name change: http://perl.apache.org/~dougm/apr_rename.txt feel free to shout if anything looks wrong, i won't commit for a few hours.
Re: apr_ function prefixes
On Wed, 7 Feb 2001, Doug MacEachern wrote: > to see the files that will change and the line number/name change: > http://perl.apache.org/~dougm/apr_rename.txt +1. Wow that looks great. A toast to consistency! Is there any way you could forward along the conversion script that you used (Or is that the one you sent last week)? This is going to break Subversion a bit, and maybe I (or Greg) can get those changes in and committed first thing in the morning. -Fitz
Re: apr_ function prefixes
On Wed, 7 Feb 2001, B. W. Fitzpatrick wrote: > +1. Wow that looks great. A toast to consistency! i'll drink to that :) > Is there any way you could forward along the conversion script that you > used (Or is that the one you sent last week)? This is going to break > Subversion a bit, and maybe I (or Greg) can get those changes in and > committed first thing in the morning. sure: http://apr.apache.org/~dougm/apr_rename.pl quick-n-dirty, but seems to work ok.
Re: FreeBSD 4.2 breakage (?)
Jeff Trawick <[EMAIL PROTECTED]> writes: > Regarding _POSIX_SOURCE: Trivia on _POSIX_SOURCE from Tru64... /* * If user defines _POSIX_SOURCE and if _POSIX_C_SOURCE is not defined, * define _POSIX_C_SOURCE to be 1. (_POSIX_SOURCE maps to the POSIX 1003.1 * standard from 1990). */ This stuff can be really aggravating... I'm trying to figure out how to enable the declaration of _SC_CLK_TCK so that mod_status compiles on Tru64. YOY can't they have something like _ALL_SOURCE (like on OS/390) which says I want access to whatever you've got, standards be damned? -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: apr_ function prefixes
On Wed, Feb 07, 2001 at 10:59:47PM -0800, Doug MacEachern wrote: > On Wed, 7 Feb 2001, B. W. Fitzpatrick wrote: > > > +1. Wow that looks great. A toast to consistency! > > i'll drink to that :) > > > Is there any way you could forward along the conversion script that you > > used (Or is that the one you sent last week)? This is going to break > > Subversion a bit, and maybe I (or Greg) can get those changes in and > > committed first thing in the morning. > > sure: > http://apr.apache.org/~dougm/apr_rename.pl i'm not entirely sure, since i'm not too familiar with apr, and i don't have the time to go through and check right now, but your script looks like it has a few typos. on lines 278 and 283 you replace put with pupt for apr_os_exp_time_put and apr_os_thread_put. i assume that's wrong, since i can't for the life of me figure out what pupt would mean ;-) -garrett -- garrett rooney my pid is inigo montoya. [EMAIL PROTECTED] you kill -9 my parent process. http://electricjellyfish.net/prepare to vi.
Re: apr_ function prefixes
On Thursday, February 8, 2001, at 12:59 AM, Doug MacEachern wrote: On Wed, 7 Feb 2001, B. W. Fitzpatrick wrote: +1. Wow that looks great. A toast to consistency! i'll drink to that :) Is there any way you could forward along the conversion script that you used (Or is that the one you sent last week)? This is going to break Subversion a bit, and maybe I (or Greg) can get those changes in and committed first thing in the morning. sure: http://apr.apache.org/~dougm/apr_rename.pl quick-n-dirty, but seems to work ok. It looks like Greg has taken care of Subversion, but did anyone else see Doug's commit message go by or did my mailer drop it on the floor? -Fitz
Re: apr_ function prefixes
On Thu, 8 Feb 2001, Garrett Rooney wrote: > on lines 278 and 283 you replace put with pupt for apr_os_exp_time_put > and apr_os_thread_put. i assume that's wrong, since i can't for the > life of me figure out what pupt would mean ;-) whoops! you're right, i'll fix that and pupt the changes back in cvs.
Re: apr_ function prefixes
On Thu, 8 Feb 2001, B. W. Fitzpatrick wrote: > It looks like Greg has taken care of Subversion, but did anyone else see > Doug's commit message go by or did my mailer drop it on the floor? i committed at the top-level and it mailed the diff for everything to [EMAIL PROTECTED]
Re: apr_ function prefixes
On Thursday, February 8, 2001, at 10:36 AM, Doug MacEachern wrote: On Thu, 8 Feb 2001, B. W. Fitzpatrick wrote: It looks like Greg has taken care of Subversion, but did anyone else see Doug's commit message go by or did my mailer drop it on the floor? i committed at the top-level and it mailed the diff for everything to [EMAIL PROTECTED] Urgh. Guess it's time for me to subscribe to a new list. :) -Fitz
Re: apr_ function prefixes
On Thu, 8 Feb 2001, B. W. Fitzpatrick wrote: > Urgh. Guess it's time for me to subscribe to a new list. i probably should have done two commits, the apr tree then httpd-2.0
Re: apr_ function prefixes
Garrett Rooney <[EMAIL PROTECTED]> writes: > on lines 278 and 283 you replace put with pupt for apr_os_exp_time_put > and apr_os_thread_put. i assume that's wrong, since i can't for the > life of me figure out what pupt would mean ;-) good for you if sleeping child cooperates I'll fix it up Real Soon Now... $ grep pupt `find . -name '*.h'` ./srclib/apr/include/apr_portable.h: * @deffunc apr_status_t apr_os_exp_time_pupt(apr_exploded_time_t *aprtime, apr_os_exp_time_t **ostime, apr_pool_t *cont) ./srclib/apr/include/apr_portable.h:APR_DECLARE(apr_status_t) apr_os_exp_time_pupt(apr_exploded_time_t *aprtime, ./srclib/apr/include/apr_portable.h: * @deffunc apr_status_t apr_os_thread_pupt(apr_thread_t **thd, apr_os_thread_t *thethd, ./srclib/apr/include/apr_portable.h:APR_DECLARE(apr_status_t) apr_os_thread_pupt(apr_thread_t **thd, $ grep pupt `find . -name '*.c'` ./server/exports.c:const void *ap_hack_apr_os_exp_time_pupt = (const void *)apr_os_exp_time_pupt; ./server/exports.c:const void *ap_hack_apr_os_thread_pupt = (const void *)apr_os_thread_pupt; ./srclib/apr/threadproc/beos/thread.c:apr_status_t apr_os_thread_pupt(apr_thread_t **thd, apr_os_thread_t *thethd, ./srclib/apr/threadproc/unix/thread.c:apr_status_t apr_os_thread_pupt(apr_thread_t **thd, apr_os_thread_t *thethd, ./srclib/apr/threadproc/win32/thread.c:APR_DECLARE(apr_status_t) apr_os_thread_pupt(apr_thread_t **thd, ./srclib/apr/time/unix/time.c:apr_status_t apr_os_exp_time_pupt(apr_exploded_time_t *aprtime, ./srclib/apr/time/win32/time.c:APR_DECLARE(apr_status_t) apr_os_exp_time_pupt(apr_exploded_time_t *aprtime, -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
Re: apr_ function prefixes
On Thu, Feb 08, 2001 at 09:10:48AM -0600, B. W. Fitzpatrick wrote: > On Thursday, February 8, 2001, at 12:59 AM, Doug MacEachern wrote: >... > > sure: > > http://apr.apache.org/~dougm/apr_rename.pl > > > > quick-n-dirty, but seems to work ok. > > It looks like Greg has taken care of Subversion, but did anyone else see > Doug's commit message go by or did my mailer drop it on the floor? I deleted the thing (it was over 800k). I'm not sure which mail list it was sent to, but it probably went to the httpd-2.0-cvs mailing list rather than APR's list. The other alternative (which I'd doubt) is that your mail system rejected it due to size. Cheers, -g -- Greg Stein, http://www.lyra.org/
Re: [PATCH] a somewhat different approach to the iconv() issue
"Roy T. Fielding" <[EMAIL PROTECTED]> writes: > > > What was checked in doesn't work -- I now get warnings again on > > > my platform, and mine is the one with the correct prototypes. > > > > Hmmm... I get no warnings on a system of each flavor (Tru64 and > > RedHat 6) and got no warnings. What warnings do you get? Did the > > configure output display the right message? > > > > checking for type of inbuf parameter to iconv... char ** > > RedHat 7 with the latest glibc compiled from source. There are > two problems, one easy and one not yet figured out: > > 1) The #ifdef APR_ in xlate.c must be a #if Why is that? We generally use #ifdef FOO if FOO is sometimes defined and sometimes not (e.g., APR_ICONV_INBUF_CONST). We generally use #if FOO if FOO is always defined but sometimes to 1 and sometimes to 0 (e.g., APR_HAS_XLATE). > 2) The test is looking at the versions and triggering #error > > I don't know why the version test is giving a false-positive, nor > do I particularly care -- I don't think this can be fixed by assuming > the version numbers mean anything. The fix I am working on is a > replacement for AC_TRY_COMPILE, but m4 isn't one of my strong > points. I don't follow your comment about "assuming the version numbers mean anything". Maybe if I saw the version numbers in glibc 2.2's it would be easy to follow what you are saying. If you can get -Werror working for gcc, obviously we don't need the glibc version check since we can expect to use gcc on a platform which uses glibc 99.99% of the time, but I can't help but be curious about why it generated the #error on RH 7.0 which has glibc 2.2 (and I guess __GLIBC_MINOR__ == 2). (I wasn't too eager to write the compile macro either. When I considered a different flavor of AC_TRY_COMPILE it was obvious that I would have to start by cutting and pasting AC_TRY_COMPILE. A quick look at the license reminded me that I am not a lawyer :) ) -- Jeff Trawick | [EMAIL PROTECTED] | PGP public key at web site: http://www.geocities.com/SiliconValley/Park/9289/ Born in Roswell... married an alien...
shtool invokes head but misses and invokes something else?
I'm trying to start the port to Darwin, and whenever the apr build calls shtool, I get this wierd unhappy spewage: Unknown option: 1 Usage: head [-options] ... -muse method for the request (default is 'HEAD') -fmake request even if head believes method is illegal -b Use the specified URL as base -t Set timeout value -i Set the If-Modified-Since header on the request -c use this content-type for POST, PUT, CHECKIN -aUse text mode for content I/O -p use this as a proxy -Pdon't load proxy settings from environment -Hsend this HTTP header (you can specify several) -uDisplay method and URL before any response -UDisplay request headers (implies -u) -sDisplay response status code -SDisplay response status chain -eDisplay response headers -dDo not display content -oProcess HTML content in various ways -vShow program version -hPrint this message -xExtra debugging output Apparently, shtool is invoking "head -1" which should be fine, because head *does* accept a -1 argument, but instead some other program with the above usage is being invoked and I can't figure out why this might be the case. Anybody recognize that usage output? Certainly looks Apache-ish, but I can't find it in APR. -Fred Wilfredo Sánchez - [EMAIL PROTECTED] Apache Software Foundation
Re: apr_ function prefixes
> > On Thu, 8 Feb 2001, B. W. Fitzpatrick wrote: > > > >> It looks like Greg has taken care of Subversion, but did anyone else see > >> Doug's commit message go by or did my mailer drop it on the floor? > > > > i committed at the top-level and it mailed the diff for everything to > > [EMAIL PROTECTED] > > Urgh. Guess it's time for me to subscribe to a new list. > > :) I'm trying to figure out how to make CVS send mail to the right repository, even if the commit is from a higher directory. Ryan ___ Ryan Bloom [EMAIL PROTECTED] 406 29th St. San Francisco, CA 94131 ---
Re: cvs commit: apr buildconf
On Wednesday, February 7, 2001, at 02:54 PM, Jeff Trawick wrote: I think you meant "ltpath=`dirname $libtoolize`" buildconf ain't working right for me and configure isn't either :) Oy. Sorry about that. -Fred
