Re: [PATCH] Makefile: enable DEVELOPER by default

[Jonathan Nieder <jrnieder@gmail.com>]

Hi,

Eric Sunshine wrote:
> On Sat, Aug 4, 2018 at 11:17 PM Jonathan Nieder <jrnieder@gmail.com> wrote:

>>> utf8.c:486:28: warning: passing 'iconv_ibp *' (aka 'const char **') to parameter
>>>       of type 'char **' discards qualifiers in nested pointer types
>>>       [-Wincompatible-pointer-types-discards-qualifiers]
>>
>> Oh, good catch!  POSIX documents iconv has having signature
>>
>>         size_t iconv(iconv_t cd, char **restrict inbuf,
>>                size_t *restrict inbytesleft, char **restrict outbuf,
>>                size_t *restrict outbytesleft);
>>
>> config.mak.uname contains
>>
>>         ifeq ($(uname_S),FreeBSD)
>>                 NEEDS_LIBICONV = YesPlease
>>                 OLD_ICONV = YesPlease
>>
>> So it looks like FreeBSD has modernized and we need to make that
>> conditional in config.mak.uname on $(uname_R).  Do you know which
>> version of FreeBSD changed the signature?  Care to write a patch?
>
> Unfortunately, I don't know in which version of FreeBSD that changed.

"git blame" tells me it's from r281550[1] (Remove the const qualifier
from iconv(3) to comply with POSIX, 2015-04-15), which was part of
FreeBSD 11.  r282275[2] (2015-04-30) backported the same change to
FreeBSD 10 and is part of 10.2.

FreeBSD 9 has

	#define iconv(cd, in, insize, out, outsize)    libiconv(cd, __DECONST(char **, in), insize, out, outsize)

from r219019[3] (2011-02-25).  I don't know what to make of it.  The
underlying libiconv function it calls takes char ** (the modern thing)
but the macro does the __DECONST thing for compatibility with GNU
libiconv.

Older versions, going back at least as far as FreeBSD 3, behave the
same way as FreeBSD 9.  So that must be what was tested with
OLD_ICONV=YesPlease.

FreeBSD 10.1.0 and 10.0.0 have

	size_t  iconv(iconv_t, const char ** __restrict,
		      size_t * __restrict, char ** __restrict,
		      size_t * __restrict);

which also needs OLD_ICONV.  If I assume everyone on 10.x is using
10.2 or newer, then the patch could be something like this (completely
untested):

diff --git i/config.mak.uname w/config.mak.uname
index 684fc5bf026..8078c099313 100644
--- i/config.mak.uname
+++ w/config.mak.uname
@@ -192,7 +192,9 @@ ifeq ($(uname_O),Cygwin)
 endif
 ifeq ($(uname_S),FreeBSD)
 	NEEDS_LIBICONV = YesPlease
-	OLD_ICONV = YesPlease
+	ifeq ($(shell expr "$(uname_R)" : '[1-9]\.'),2)
+		OLD_ICONV = YesPlease
+	endif
 	NO_MEMMEM = YesPlease
 	BASIC_CFLAGS += -I/usr/local/include
 	BASIC_LDFLAGS += -L/usr/local/lib

Deciding whether to fix the pattern matching to also include 10.0 and
10.1 (and doing so if warranted) is left as an exercise to the reader.

Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>

[1] https://github.com/freebsd/freebsd/commit/b0813ee288f64f677a2cebf7815754b027a8215b
[2] https://github.com/freebsd/freebsd/commit/b709ec868adb5170d09bc5a66b18d0e0d5987ab6
[3] https://github.com/freebsd/freebsd/commit/c91ab1769b1237e3663d59888cebe31ceee47570