Skip to contentSkip to conversation

autoconf: Add tests for memmem, strtoumax and mkdtemp functions

PATCH In review Jakub Narebski 8 participants 20 messages Jun 15, 2016 1 message not in the archive

lore
Share & tools
lore

Jakub Narebski

author
#0 lore raw

PATCH 1/1 autoconf: Add tests for memmem, strtoumax and mkdtemp functions

Update configure.ac (and config.mak.in) to keep up with git development by adding tests for memmem (NO_MEMMEM), strtoumax (NO_STRTOUMAX) and mkdtemp (NO_MKDTEMP) functions.

Signed-off-by Jakub Narebski <[email protected]>

---
This is beginning of "bring configure up to date" thingy.

By the way, do you have idea how to test for the following in configure.ac:

* Define NO_PREAD if you have a problem with pread() system call (e.g. cygwin.dll before v1.5.22).

- what is the problem? how to detect it?

* Define NO_FAST_WORKING_DIRECTORY if accessing objects in pack files is generally faster on your platform than accessing the working directory.

- if at all possible

* Define NO_TRUSTABLE_FILEMODE if your filesystem may claim to support the executable mode bit, but doesn't really do so.

- I think there were some code here

* Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib" that tells runtime paths to dynamic libraries; "-Wl,-rpath=/path/lib" is used instead.

* Define NO_PERL_MAKEMAKER if you cannot use Makefiles generated by perl's MakeMaker (e.g. using ActiveState under Cygwin).

* Define ASCIIDOC8 if you want to format documentation with AsciiDoc 8
* Define DOCBOOK_XSL_172 if you want to format man pages with DocBook XSL v1.72.

- it needs some portable way to check asciidoc and docbook-xsl version

* Define OLD_ICONV if your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **).

- perhaps compile with new iconv and check for compile errors?

 config.mak.in |    3 +++
 configure.ac  |   18 ++++++++++++++++++
 2 files changed, 21 insertions(+), 0 deletions(-)
config.mak.in +3 -0
index 776b805..11d256e 100644
@@ -35,7 +35,10 @@ NO_SOCKADDR_STORAGE=@NO_SOCKADDR_STORAGE@
 NO_IPV6=@NO_IPV6@
 NO_C99_FORMAT=@NO_C99_FORMAT@
 NO_STRCASESTR=@NO_STRCASESTR@
+NO_MEMMEM=@NO_MEMMEM@
 NO_STRLCPY=@NO_STRLCPY@
+NO_STRTOUMAX=@NO_STRTOUMAX@
 NO_SETENV=@NO_SETENV@
+NO_MKDTEMP=@NO_MKDTEMP@
 NO_ICONV=@NO_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
configure.ac +18 -0
index 53e9a17..7bcf1a4 100644
@@ -289,18 +289,36 @@ AC_CHECK_FUNC(strcasestr,
 [NO_STRCASESTR=YesPlease])
 AC_SUBST(NO_STRCASESTR)
 #
+# Define NO_MEMMEM if you don't have memmem.
+AC_CHECK_FUNC(memmem,
+[NO_MEMMEM=],
+[NO_MEMMEM=YesPlease])
+AC_SUBST(NO_MEMMEM)
+#
 # Define NO_STRLCPY if you don't have strlcpy.
 AC_CHECK_FUNC(strlcpy,
 [NO_STRLCPY=],
 [NO_STRLCPY=YesPlease])
 AC_SUBST(NO_STRLCPY)
 #
+# Define NO_STRTOUMAX if you don't have strtoumax in the C library.
+AC_CHECK_FUNC(strtoumax,
+[NO_STRTOUMAX=],
+[NO_STRTOUMAX=YesPlease])
+AC_SUBST(NO_STRTOUMAX)
+#
 # Define NO_SETENV if you don't have setenv in the C library.
 AC_CHECK_FUNC(setenv,
 [NO_SETENV=],
 [NO_SETENV=YesPlease])
 AC_SUBST(NO_SETENV)
 #
+# Define NO_MKDTEMP if you don't have mkdtemp in the C library.
+AC_CHECK_FUNC(mkdtemp,
+[NO_MKDTEMP=],
+[NO_MKDTEMP=YesPlease])
+AC_SUBST(NO_MKDTEMP)
+#
 # Define NO_MMAP if you want to avoid mmap.
 #
 # Define NO_ICONV if your libc does not properly support iconv.
Signature · 1 line
1.5.3.5
#1 lore raw
Quoted 7 lines · Jakub Narebski

By the way, do you have idea how to test for the following in configure.ac:

* Define NO_PREAD if you have a problem with pread() system call (e.g. cygwin.dll before v1.5.22).

- what is the problem? how to detect it?

This also appears to be broken on HP-UX (e.g. we need to set NO_PREAD there). Thiago on IRC talked about this the other day when he was trying to build git on a bunch of different systems.

Unfortunately this may just need to be an OS based test. The code compiles just fine but when index-pack tries to use pread to walk back through the deltas and generate their SHA-1s it barfs without an error code. I suspect the failure is pread claiming it put the data into the user buffer but not actually doing so, which causes zlib's inflate() to then see data corruption.

Ouch.

Signature · 1 line
Shawn.
author
#2 lore raw
Quoted 13 lines · Shawn O. Pearce
Quoted 7 lines · Jakub Narebski

By the way, do you have idea how to test for the following in configure.ac:

* Define NO_PREAD if you have a problem with pread() system call (e.g. cygwin.dll before v1.5.22).

- what is the problem? how to detect it?

This also appears to be broken on HP-UX (e.g. we need to set NO_PREAD there). Thiago on IRC talked about this the other day when he was trying to build git on a bunch of different systems.

Quoted 6 lines

Unfortunately this may just need to be an OS based test. The code compiles just fine but when index-pack tries to use pread to walk back through the deltas and generate their SHA-1s it barfs without an error code. I suspect the failure is pread claiming it put the data into the user buffer but not actually doing so, which causes zlib's inflate() to then see data corruption.

Can you write a short program which demonstrates this error (either fails with some error code, or produces wrong output)? That would be enough for configure.ac (see for example NO_C99_FORMAT in configure.ac)

Unless this is Schroedinger bug...

Signature · 2 lines
Jakub Narebski
Poland

A message that is not in the archive (sent off-list or not yet received): [email protected]

#4 lore raw

Also sprach Junio C Hamano am Tue, 20 Nov 2007 at 01:00:44 -0800:

Quoted 5 lines
Quoted 3 lines · Jakub Narebski

By the way, do you have idea how to test for the following in configure.ac:

Quoted 8 lines
Quoted 4 lines

* Define NO_PREAD if you have a problem with pread() system call (e.g. cygwin.dll before v1.5.22).

- what is the problem? how to detect it?

6900679c2f6d937a5a6ef616869c8887690ad19d (Replacing the system call pread() with lseek()/xread()/lseek() sequence.)

For me it happened when using git (e.g. git commit). Detecting perhaps is possible with "cygcheck" command which shows the version of cygwin system dlls.

Perhaps: cygcheck -p 'cygwin1.dll'
Shows name and version of found cygwin1.dll.

Stefan

Signature · 2 lines
Stefan-W. Hahn                          It is easy to make things.
/ mailto:[email protected] /        It is hard to make things simple.
#5 lore raw
In reply to a message not in the archive
Quoted 9 lines · Junio C Hamano
Quoted 3 lines · Jakub Narebski

* Define NO_R_TO_GCC_LINKER if your gcc does not like "-R/path/lib" that tells runtime paths to dynamic libraries; "-Wl,-rpath=/path/lib" is used instead.

Try with -R/path/lib and see if it complains, perhaps?

206488774238c94ff337dd5c997be441dbde5112 (Correct the name of NO_R_TO_GCC_LINKER in the comment describing it.)

I don't know how to test this, unfortunately. I'll be happy to test any tests you'd like me to try, though.

Quoted 33 lines
Quoted 1 line

MakeMaker (e.g. using ActiveState under Cygwin).

f848718a6980ebda0eb5afb2ca49c3bc1e7b2b1d (Make perl/ build procedure ActiveState friendly.)

Alex CC'ed.

Quoted 1 line

* Define ASCIIDOC8 if you want to format documentation with AsciiDoc 8

        case "$(asciidoc --version)" in
        asciidoc' '7*)	: not def ;;
        asciidoc' '8)	: def ;;
        esac
Quoted 3 lines

* Define DOCBOOK_XSL_172 if you want to format man pages with DocBook XSL v1.72.

- it needs some portable way to check asciidoc and docbook-xsl version

Hmmm.

Quoted 4 lines

* Define OLD_ICONV if your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **).

- perhaps compile with new iconv and check for compile errors?

Yeah, with appropriate includes,

        char *ibp, *obp;
        size_t insz, outsz;
        iconv(conv, &ibp, &insz, &obp, &outsz);

would cause compilation trouble if iconv() wants "const char *" for ibp.

Signature · 1 line
Matt
author
#6 lore raw

[PATCH/RFC] autoconf: Add test for OLD_ICONV

In reply to a message not in the archive

Update configure.ac (and config.mak.in) to keep up with git development by adding [compile] test whether your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **) (OLD_ICONV).

Signed-off-by Jakub Narebski <[email protected]>

---
This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

I should probably used AC_LANG_PROGRAM like for NO_C99_FORMAT instead of generating whole programlet^W test program by hand (I hope that for example I haven't missed some header file which needs to be included); I have followed example for NO_ICONV / NEEDS_LIBICONV and NO_DEFLATE_BOUND test. I'm also not sure if I have put this test in the correct autoconf section, but that is probably matter of taste.

P.S. Is there any convention on where to use YesPlease, and where UnfortunatelyYes when setting Makefile build configuration variables?

 config.mak.in |    1 +
 configure.ac  |   21 +++++++++++++++++++++
 2 files changed, 22 insertions(+), 0 deletions(-)
config.mak.in +1 -0
index 11d256e..7d5df9b 100644
@@ -41,4 +41,5 @@ NO_STRTOUMAX=@NO_STRTOUMAX@
 NO_SETENV=@NO_SETENV@
 NO_MKDTEMP=@NO_MKDTEMP@
 NO_ICONV=@NO_ICONV@
+OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
configure.ac +21 -0
index 5f8a15b..5d2936e 100644
@@ -212,6 +212,27 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 
 
 ## Checks for header files.
+AC_MSG_NOTICE([CHECKS for header files])
+#
+# Define OLD_ICONV if your library has an old iconv(), where the second
+# (input buffer pointer) parameter is declared with type (const char **).
+AC_DEFUN([OLDICONVTEST_SRC], [[
+#include <iconv.h>
+
+int main(void)
+{
+	iconv_t cd;
+	char *ibp, *obp;
+	size_t insz, outsz;
+	iconv(cd, &ibp, &insz, &obp, &outsz);
+}
+]])
+AC_MSG_CHECKING([for old iconv()])
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=YesPlease])
+AC_SUBST(OLD_ICONV)
 
 
 ## Checks for typedefs, structures, and compiler characteristics.
Signature · 1 line
1.5.3.6
#7 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 6 lines · Jakub Narebski
Signed-off-by Jakub Narebski <[email protected]>

---
This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

Not working on Cygwin:

   $ autoconf
   $ ./configure --prefix=/usr/local --build=i686-pc-cygwin
   ...
   configure: CHECKS for header files
   checking for old iconv()... no

It should be yes above. And in config.mak.autogen we have:

OLD_ICONV=

Note also that you should remove all the hard-coded settings in Makefile anyway.

Pascal.

Signature · 8 lines
--|------------------------------------------------------
--| Pascal Obry                           Team-Ada Member
--| 45, rue Gabriel Peri - 78114 Magny Les Hameaux FRANCE
--|------------------------------------------------------
--|              http://www.obry.net
--| "The best way to travel is by means of imagination"
--|
--| gpg --keyserver wwwkeys.pgp.net --recv-key C1082595
#8 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

In reply to Jakub Narebski
Quoted 20 lines · Jakub Narebski
configure.ac +12 -0
index 5f8a15b..5d2936e 100644
@@ -212,6 +212,27 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 
 
 ## Checks for header files.
+AC_MSG_NOTICE([CHECKS for header files])
+#
+# Define OLD_ICONV if your library has an old iconv(), where the second
+# (input buffer pointer) parameter is declared with type (const char **).
+AC_DEFUN([OLDICONVTEST_SRC], [[
+#include <iconv.h>
+
+int main(void)
+{
+	iconv_t cd;
+	char *ibp, *obp;
+	size_t insz, outsz;

configure.ac test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket" · line 236

Quoted 10 lines
configure.ac +9 -0
+	iconv(cd, &ibp, &insz, &obp, &outsz);
+}
+]])
+AC_MSG_CHECKING([for old iconv()])
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=YesPlease])
+AC_SUBST(OLD_ICONV)

Which result does COMPILE_IFELSE give for non error warnings? Ok, or Bad?

author
#9 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 12 lines · Junio C Hamano

configure.ac test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket" · line 235

Quoted 7 lines · Jakub Narebski
configure.ac +6 -0
+AC_MSG_CHECKING([for old iconv()])
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=YesPlease])
+AC_SUBST(OLD_ICONV)

Which result does COMPILE_IFELSE give for non error warnings? Ok, or Bad?

- Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
Run the compiler and compilation flags of the current language (*note Language Choice::) on the INPUT, run the shell commands ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT can be made by `AC_LANG_PROGRAM' and friends.

And if I have checked correctly code which causes only warnings returns Ok (in this case print 'no' after 'checking for old iconv()... ' and do not set OLD_ICONV, which means it will be unset).

Signature · 2 lines
Jakub Narebski
Poland
#10 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 21 lines · Jakub Narebski
Quoted 12 lines · Junio C Hamano

configure.ac test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket" · line 235

Quoted 7 lines · Jakub Narebski
configure.ac +6 -0
+AC_MSG_CHECKING([for old iconv()])
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=YesPlease])
+AC_SUBST(OLD_ICONV)

Which result does COMPILE_IFELSE give for non error warnings? Ok, or Bad?

- Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
Run the compiler and compilation flags of the current language (*note Language Choice::) on the INPUT, run the shell commands ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT can be made by `AC_LANG_PROGRAM' and friends.

Quoted 3 lines

And if I have checked correctly code which causes only warnings returns Ok (in this case print 'no' after 'checking for old iconv()... ' and do not set OLD_ICONV, which means it will be unset).

Which means the real-life compilation will get the warning on type mismatch. Wasn't OLD_ICONV about squelching that?

author
#11 lore raw

[PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)

Quoted 22 lines · Junio C Hamano
Quoted 21 lines · Jakub Narebski
Quoted 12 lines · Junio C Hamano

Review

Quoted 7 lines · Jakub Narebski
(quoted diff) +6 -0
+AC_MSG_CHECKING([for old iconv()])
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=YesPlease])
+AC_SUBST(OLD_ICONV)

Which result does COMPILE_IFELSE give for non error warnings? Ok, or Bad?

- Macro: AC_COMPILE_IFELSE (INPUT, [ACTION-IF-FOUND], [ACTION-IF-NOT-FOUND])
Run the compiler and compilation flags of the current language (*note Language Choice::) on the INPUT, run the shell commands ACTION-IF-TRUE on success, ACTION-IF-FALSE otherwise. The INPUT can be made by `AC_LANG_PROGRAM' and friends.

Quoted 6 lines
Quoted 3 lines

And if I have checked correctly code which causes only warnings returns Ok (in this case print 'no' after 'checking for old iconv()... ' and do not set OLD_ICONV, which means it will be unset).

Which means the real-life compilation will get the warning on type mismatch. Wasn't OLD_ICONV about squelching that?

Gah, I don't know why I though OLD_ICONV was about compile errors, and not about compile warnings. This version uses -Werror to check for warnings; I hope it doesn't give false positives...

Quoted 15 lines · Pascal Obry
Quoted 5 lines · Jakub Narebski

---
This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

Not working on Cygwin:

   $ autoconf
   $ ./configure --prefix=/usr/local --build=i686-pc-cygwin
   ...
   configure: CHECKS for header files
   checking for old iconv()... no
Quoted 3 lines

It should be yes above. And in config.mak.autogen we have:

OLD_ICONV=

Check out current version of patch. It should work correctly now (I thought OLD_ICONV was about compile errors, and it is about squelching compile warnings). It should give now:

  $ make configure
  $ ./configure --prefix=/usr/local --build=i686-pc-cygwin

configure: CHECKS for header files
checking for old iconv()... yes

  $ cat config.mak.autogen

OLD_ICONV=UnfortunatelyYes

Quoted 2 lines

Note also that you should remove all the hard-coded settings in Makefile anyway.

No, I should not. ./configure script is purely optional in git, and compiling should work with reasonable defaults even if you don't have autoconf installed and/or you don't want to run ./configure script (because e.g. it is too slow).

-- >8 --
From: Jakub Narebski <[email protected]>
Subject: [PATCH] autoconf: Add test for OLD_ICONV (squelching compiler warning)

Update configure.ac (and config.mak.in) to keep up with git development by adding [compile] test whether your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **) (OLD_ICONV).

Test-proposed-by Junio C Hamano <[email protected]>
Signed-off-by Jakub Narebski <[email protected]>
---
 config.mak.in |    1 +
 configure.ac  |   24 ++++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)
config.mak.in +1 -0
index 11d256e..7d5df9b 100644
@@ -41,4 +41,5 @@ NO_STRTOUMAX=@NO_STRTOUMAX@
 NO_SETENV=@NO_SETENV@
 NO_MKDTEMP=@NO_MKDTEMP@
 NO_ICONV=@NO_ICONV@
+OLD_ICONV=@OLD_ICONV@
 NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@
configure.ac +24 -0
index 5f8a15b..196ab3e 100644
@@ -212,6 +212,30 @@ test -n "$NEEDS_SOCKET" && LIBS="$LIBS -lsocket"
 
 
 ## Checks for header files.
+AC_MSG_NOTICE([CHECKS for header files])
+#
+# Define OLD_ICONV if your library has an old iconv(), where the second
+# (input buffer pointer) parameter is declared with type (const char **).
+AC_DEFUN([OLDICONVTEST_SRC], [[
+#include <iconv.h>
+
+int main(void)
+{
+	iconv_t cd;
+	char *ibp, *obp;
+	size_t insz, outsz;
+	iconv(cd, &ibp, &insz, &obp, &outsz);
+}
+]])
+AC_MSG_CHECKING([for old iconv()])
+CFLAGS_ORIG=$CFLAGS
+CFLAGS="$CFLAGS -Werror"
+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,
+	[AC_MSG_RESULT([no])],
+	[AC_MSG_RESULT([yes])
+	OLD_ICONV=UnfortunatelyYes])
+CFLAGS=$CFLAGS_ORIG
+AC_SUBST(OLD_ICONV)
 
 
 ## Checks for typedefs, structures, and compiler characteristics.
Signature · 1 line
1.5.3.7
#12 lore raw

Re: [PATCH/RFC (amend)] autoconf: Add test for OLD_ICONV (squelching compiler warning)

Quoted 6 lines · Jakub Narebski
Quoted 2 lines

Which means the real-life compilation will get the warning on type mismatch. Wasn't OLD_ICONV about squelching that?

Gah, I don't know why I though OLD_ICONV was about compile errors, and not about compile warnings. This version uses -Werror to check for warnings; I hope it doesn't give false positives...

But use of -Werror means you are married to gcc, doesn't it?

How important is it to detect OLD_ICONV anyway, I have to wonder?

Quoted 6 lines · Pascal Obry

...

Quoted 2 lines

Note also that you should remove all the hard-coded settings in Makefile anyway.

No, I should not. ./configure script is purely optional in git,

Correct. Thanks.

#13 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

In reply to Jakub Narebski
Quoted 11 lines · Jakub Narebski

Update configure.ac (and config.mak.in) to keep up with git development by adding [compile] test whether your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **) (OLD_ICONV).

Signed-off-by Jakub Narebski <[email protected]>

---
This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

I could not get git am to apply the patch cleanly, but will try again later when I have more time. That said, 10.5's default /usr/include/iconv.h uses the newer non-const definition of iconv_open. I've had to add a "OLD_ICONV=" line to my config.mak to avoid the warning. I'll still test to look for a false positive.

Quoted 2 lines

P.S. Is there any convention on where to use YesPlease, and where UnfortunatelyYes when setting Makefile build configuration variables?

AFAICT, YesPlease is the "normal" choice and UnfortunatelyYes is used when the person who added it considered it an obnoxious hack or a stupid failing in a system. The test in the Makefile is just if the variable is defined, so you technically could use "IDontLikeSillyVariableValuesInMyMakefiles", and it would work just fine. ;-)

~~ Brian

author
#14 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 9 lines · Brian Gernhardt
Quoted 4 lines · Jakub Narebski

This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

I could not get git am to apply the patch cleanly, but will try again later when I have more time.

Hmmm... it should apply cleanly to both 'master' (7a4a2e1f797) and 'next' (bd1cfb7fbddc). It just adds some contents...

Signature · 2 lines
Jakub Narebski
Poland
#15 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

In reply to Jakub Narebski
Quoted 11 lines · Jakub Narebski

Update configure.ac (and config.mak.in) to keep up with git development by adding [compile] test whether your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **) (OLD_ICONV).

Signed-off-by Jakub Narebski <[email protected]>

---
This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

Before applying your patch:

CC utf8.o
utf8.c: In function ‘reencode_string’:
utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible pointer type
CC convert.o

After applying your patch:

CC utf8.o
CC convert.o

This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).

Cheers,
Wincent

author
#16 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 22 lines · Wincent Colaiuta
Quoted 11 lines · Jakub Narebski

Update configure.ac (and config.mak.in) to keep up with git development by adding [compile] test whether your library has an old iconv(), where the second (input buffer pointer) parameter is declared with type (const char **) (OLD_ICONV).

Signed-off-by Jakub Narebski <[email protected]>

---
This patch needs checking if it correctly sets OLD_ICONV when needed. I have checked only that it is not set when with new iconv() declaration. Could people using Cygwin (and other with OLD_ICONV: Darwin) test it?

Before applying your patch:

CC utf8.o
utf8.c: In function ‘reencode_string’:
utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible pointer type
CC convert.o

Quoted 4 lines

After applying your patch:

CC utf8.o
CC convert.o

Do I understand correctly that above is excerpt from the output of the following sequence of commands before and after this patch applied?

  $ make configure
  $ ./configure [options]
  $ make

Do you have something like below in ./configure output?

configure: CHECKS for header files
checking for old iconv()... yes

Quoted 1 line

This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).

Strange... in Makefile there is

ifeq ($(uname_S),Darwin)
NEEDS_SSL_WITH_CRYPTO = YesPlease
NEEDS_LIBICONV = YesPlease
OLD_ICONV = UnfortunatelyYes
NO_STRLCPY = YesPlease
NO_MEMMEM = YesPlease
endif

so the uname based guessing should set OLD_ICONV on Darwin...

Signature · 2 lines
Jakub Narebski
Poland
#17 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 15 lines · Jakub Narebski
Quoted 9 lines · Wincent Colaiuta

Before applying your patch:

CC utf8.o
utf8.c: In function ‘reencode_string’:
utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible pointer type
CC convert.o

Quoted 4 lines

After applying your patch:

CC utf8.o
CC convert.o

Quoted 6 lines

Do I understand correctly that above is excerpt from the output of the following sequence of commands before and after this patch applied?

 $ make configure
 $ ./configure [options]
 $ make

Yes, that's right, but with a "make clean" before anything else.

Quoted 4 lines

Do you have something like below in ./configure output?

configure: CHECKS for header files
checking for old iconv()... yes

This:

configure: CHECKS for header files
checking for old iconv()... no

Quoted 12 lines
Quoted 1 line

This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).

Strange... in Makefile there is

ifeq ($(uname_S),Darwin)
NEEDS_SSL_WITH_CRYPTO = YesPlease
NEEDS_LIBICONV = YesPlease
OLD_ICONV = UnfortunatelyYes
NO_STRLCPY = YesPlease
NO_MEMMEM = YesPlease
endif

Quoted 1 line

so the uname based guessing should set OLD_ICONV on Darwin...

That happens *before* config.mak.autogen is included in the Makefile, so it gets overridden.

Cheers,
Wincent

author
#18 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 47 lines · Wincent Colaiuta
Quoted 15 lines · Jakub Narebski
Quoted 9 lines · Wincent Colaiuta

Before applying your patch:

CC utf8.o
utf8.c: In function ‘reencode_string’:
utf8.c:328: warning: passing argument 2 of ‘iconv’ from incompatible pointer type
CC convert.o

Quoted 4 lines

After applying your patch:

CC utf8.o
CC convert.o

Quoted 6 lines

Do I understand correctly that above is excerpt from the output of the following sequence of commands before and after this patch applied?

 $ make configure
 $ ./configure [options]
 $ make

Yes, that's right, but with a "make clean" before anything else.

Quoted 4 lines

Do you have something like below in ./configure output?

configure: CHECKS for header files
checking for old iconv()... yes

This:

configure: CHECKS for header files
checking for old iconv()... no

Quoted 12 lines
Quoted 1 line

This on Darwin Kernel Version 9.1.0 (Mac OS X 10.5.1).

Strange... in Makefile there is

ifeq ($(uname_S),Darwin)
NEEDS_SSL_WITH_CRYPTO = YesPlease
NEEDS_LIBICONV = YesPlease
OLD_ICONV = UnfortunatelyYes
NO_STRLCPY = YesPlease
NO_MEMMEM = YesPlease
endif

Quoted 4 lines
Quoted 1 line

so the uname based guessing should set OLD_ICONV on Darwin...

That happens *before* config.mak.autogen is included in the Makefile, so it gets overridden.

Ahhh... now I understand. You have installed new iconv() on your computer, and generic 'uname -s' (OS name) based guessing in Makefile guesses wrongly that you need OLD_ICONV, while ./configure script actually tests it and correctly decides to unset OLD_ICONV !

BTW. Perhaps it whould be written more explicitely:

+AC_COMPILE_IFELSE(OLDICONVTEST_SRC,

+       [AC_MSG_RESULT([no])
+       OLD_ICONV=],
+       [AC_MSG_RESULT([yes])
+       OLD_ICONV=YesPlease])
Signature · 2 lines
Jakub Narebski
Poland
#19 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

Quoted 4 lines · Jakub Narebski

Ahhh... now I understand. You have installed new iconv() on your computer, and generic 'uname -s' (OS name) based guessing in Makefile guesses wrongly that you need OLD_ICONV, while ./configure script actually tests it and correctly decides to unset OLD_ICONV !

As far as the "installed new iconv()" goes, you may be wrong there. The latest major release of OS X includes a version of iconv that does not need OLD_ICONV as part of the base system. I've had to unset it in my config.mak in order to avoid the warning.

~~ Brian Gernhardt

#20 lore raw

Re: [PATCH/RFC] autoconf: Add test for OLD_ICONV

In reply to Jakub Narebski
Quoted 4 lines · Jakub Narebski

Ahhh... now I understand. You have installed new iconv() on your computer, and generic 'uname -s' (OS name) based guessing in Makefile guesses wrongly that you need OLD_ICONV, while ./configure script actually tests it and correctly decides to unset OLD_ICONV !

No, nothing installed. It's that Mac OS X 10.5 (Leopard)/Darwin 9 comes with a newer version of iconv. So while there was no warning prior to Leopard there is one now, which your patch fixes for those using configure.

This came up before here:

<http://marc.info/?l=git&m=119397507006026&w=2>

Still, I'd like to fix the warning for those who don't use configure. Have posted a separate patch for that just now.

Cheers,

Quoted 1 line

Wincent