Re: [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set

4 messages, 3 authors, 2016-06-15 · open the first message on its own page

Re: [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:46

"David Symonds" [off-list ref] writes:
On 11/2/07, Junio C Hamano [off-list ref] wrote:
quoted
Blake Ramsdell [off-list ref] writes:
quoted
Signed-off-by: Blake Ramsdell <redacted>
---
 Makefile |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)
diff --git a/Makefile b/Makefile
index 71479a2..5d83756 100644
--- a/Makefile
+++ b/Makefile
@@ -401,7 +401,9 @@ endif
 ifeq ($(uname_S),Darwin)
      NEEDS_SSL_WITH_CRYPTO = YesPlease
      NEEDS_LIBICONV = YesPlease
-     OLD_ICONV = UnfortunatelyYes
+     ifneq ($(uname_R),9.0.0)
+             OLD_ICONV = UnfortunatelyYes
+     endif
      NO_STRLCPY = YesPlease
      NO_MEMMEM = YesPlease
 endif
I do not have an access to a Darwin box, but do you mean 10.5
gives 9.0.0 as uname_R?
Further, that comparison is going to fail as soon as the next revision
of Darwin (9.0.1, etc.) is released.
Can we do something intelligent with $(shell iconv --version)
there instead, I wonder, then?

Re: [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set

From: David Symonds <hidden>
Date: 2016-06-15 22:43:46

On 11/2/07, Junio C Hamano [off-list ref] wrote:
"David Symonds" [off-list ref] writes:
quoted
Further, that comparison is going to fail as soon as the next revision
of Darwin (9.0.1, etc.) is released.
Can we do something intelligent with $(shell iconv --version)
there instead, I wonder, then?
It would probably be most appropriate for the autoconf script. Now
that I look at configure.ac, there's already a test for iconv in
there; is it not used?


Dave.

Re: [PATCH] Mac OS X 10.5 does not require the OLD_ICONV flag set

From: Blake Ramsdell <hidden>
Date: 2016-06-15 22:43:46

On 11/2/07, David Symonds [off-list ref] wrote:
It would probably be most appropriate for the autoconf script. Now
that I look at configure.ac, there's already a test for iconv in
there; is it not used?
The crux of this problem is that the prototype for iconv in
/usr/include/iconv.h is different between OS X 10.4 and OS X 10.5. So
the "right thing" is definitely to determine what is in the function
prototype, and then act accordingly.
From OS X 10.4.10:
#define _LIBICONV_VERSION 0x0109    /* version number: (major<<8) + minor */
...
extern size_t iconv (iconv_t cd, const char* * inbuf, size_t
*inbytesleft, char* * outbuf, size_t *outbytesleft);
From OS X 10.5:
#define _LIBICONV_VERSION 0x010B    /* version number: (major<<8) + minor */
...
size_t iconv (iconv_t /*cd*/,
        char ** __restrict /*inbuf*/,  size_t * __restrict /*inbytesleft*/,
        char ** __restrict /*outbuf*/, size_t * __restrict /*outbytesleft*/);

So what happened in git is that someone put in OLD_ICONV to
dynamically adjust the const-ness of parameter 2 to the iconv
function, and the way they chose to do that is to identify the OS
(more accurately, the kernel), and then I went and furthered that by
identifying the version.

Now that I look at it further, it seems that yanking OLD_ICONV
altogether is a better approach, and to just check _LIBICONV_VERSION
to make sure it's "new enough". Now, I'm not sure what that comparison
would be, but we know that "later than 0x0109" is a good start. Based
on the version difference between OS X 10.4 and 10.5 I note that there
is only 0x010A intervening.

This strategy presumes that this const parameter was const all the way
up to a particular point, and then stopped being const, which is
probably a reasonable assumption.

Blake
-- 
Blake Ramsdell | http://www.blakeramsdell.com

[PATCH] Removed OLD_ICONV in favor of checking _LIBICONV_VERSION directly

From: Blake Ramsdell <hidden>
Date: 2016-06-15 22:43:46

Signed-off-by: Blake Ramsdell <redacted>
---
 Makefile |    8 --------
 utf8.c   |    2 +-
 2 files changed, 1 insertions(+), 9 deletions(-)
diff --git a/Makefile b/Makefile
index 71479a2..96c8a73 100644
--- a/Makefile
+++ b/Makefile
@@ -95,9 +95,6 @@ all::
 #
 # Define NO_ICONV if your libc does not properly support iconv.
 #
-# Define OLD_ICONV if your library has an old iconv(), where the second
-# (input buffer pointer) parameter is declared with type (const char **).
-#
 # 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.
@@ -401,7 +398,6 @@ endif
 ifeq ($(uname_S),Darwin)
 	NEEDS_SSL_WITH_CRYPTO = YesPlease
 	NEEDS_LIBICONV = YesPlease
-	OLD_ICONV = UnfortunatelyYes
 	NO_STRLCPY = YesPlease
 	NO_MEMMEM = YesPlease
 endif
@@ -657,10 +653,6 @@ ifdef NO_ICONV
 	BASIC_CFLAGS += -DNO_ICONV
 endif
 
-ifdef OLD_ICONV
-	BASIC_CFLAGS += -DOLD_ICONV
-endif
-
 ifdef PPC_SHA1
 	SHA1_HEADER = "ppc/sha1.h"
 	LIB_OBJS += ppc/sha1.o ppc/sha1ppc.o
diff --git a/utf8.c b/utf8.c
index 4efef6f..a7feb4f 100644
--- a/utf8.c
+++ b/utf8.c
@@ -300,7 +300,7 @@ int is_encoding_utf8(const char *name)
  * with iconv.  If the conversion fails, returns NULL.
  */
 #ifndef NO_ICONV
-#ifdef OLD_ICONV
+#if _LIBICONV_VERSION <= 0x0109
 	typedef const char * iconv_ibp;
 #else
 	typedef char * iconv_ibp;
-- 
1.5.3.GIT
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help