Re: MinGW compilation warnings in libiberty's xstrndup.c

[Pedro Alves <palves@redhat.com>]

On 05/20/2017 01:38 AM, DJ Delorie wrote:
> 
> Pedro Alves <palves@redhat.com> writes:
>> Ah, yeah.  AFAICS, all the declaration checks in libiberty.h are 
>> HAVE_DECL checks.  This suggests to me that this declaration guard 
>> should be HAVE_DECL too [1].
> 
> Except the ones in the $funcs list, which includes strnlen.  I think in
> the old days, we didn't put in declarations at all... until "char *"
> became a different size than "int" and we started needing them.

Running:

$ grep HAVE_ libiberty/config.h | sed 's/DECL_//g'| sort | uniq -c | sort -n

on the build I have handy shows:
...
      2 #define HAVE_ASPRINTF 1
      2 #define HAVE_BASENAME 1
      2 #define HAVE_CALLOC 1
      2 #define HAVE_FFS 1
      2 #define HAVE_SBRK 1
      2 #define HAVE_SNPRINTF 1
      2 #define HAVE_STRTOL 1
      2 #define HAVE_STRTOLL 1
      2 #define HAVE_STRTOUL 1
      2 #define HAVE_STRTOULL 1
      2 #define HAVE_STRVERSCMP 1
      2 #define HAVE_VASPRINTF 1

"2" means above means each FOO symbol above has both HAVE_FOO
and HAVE_DECL_FOO defines:

 $ grep "HAVE.*_SNPRINTF" config.h
 #define HAVE_DECL_SNPRINTF 1
 #define HAVE_SNPRINTF 1

> 
> So some functions in libiberty are HAVE_DECL and others are still HAVE.

But I don't see any HAVE check in libiberty.h (for function symbols),
only HAVE_DECL ones:

$ grep HAVE libiberty.h 
/* HAVE_DECL_* is a three-state macro: undefined, 0 or 1.  If it is
#if !HAVE_DECL_BASENAME
 || defined (__DragonFly__) || defined (HAVE_DECL_BASENAME) 
   autoconf which would result in HAVE_DECL_BASENAME being set.  */
#if defined (HAVE_DECL_FFS) && !HAVE_DECL_FFS
#if defined(HAVE_DECL_ASPRINTF) && !HAVE_DECL_ASPRINTF
#if !HAVE_DECL_VASPRINTF
#if defined(HAVE_DECL_SNPRINTF) && !HAVE_DECL_SNPRINTF
#if defined(HAVE_DECL_VSNPRINTF) && !HAVE_DECL_VSNPRINTF
#if defined (HAVE_DECL_STRNLEN) && !HAVE_DECL_STRNLEN
#if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP
#if defined(HAVE_DECL_STRTOL) && !HAVE_DECL_STRTOL
#if defined(HAVE_DECL_STRTOUL) && !HAVE_DECL_STRTOUL
#if defined(HAVE_LONG_LONG) && defined(HAVE_DECL_STRTOLL) && !HAVE_DECL_STRTOLL
#if defined(HAVE_LONG_LONG) && defined(HAVE_DECL_STRTOULL) && !HAVE_DECL_STRTOULL
#if defined(HAVE_DECL_STRVERSCMP) && !HAVE_DECL_STRVERSCMP

Nor in other headers under include/, while at it.
Are you looking elsewhere perhaps?  Based on the above, it looks to
me like the non-HAVE_DECL HAVE symbols are implementation detail
to libiberty, side effect of the checks used to determine whether
a replacement is necessary.

> 
> Ah, found it, this commit is incomplete:
> 
> https://gcc.gnu.org/ml/gcc-patches/2014-06/msg00784.html
> 
> It changes gcc's configure but nobody else's (and now we have an answer
> to the three-year-old question "why don't we have a more liberal commit
> policy?" ;) which breaks both libiberty and libgfortran.

Yeah, that exactly the sort of thing that gets fixed by design by having
a centralized libiberty.m4 file.

> 
>> BTW, I once proposed a new libiberty.m4 file that all libiberty
>> clients would source so that these checks are all centralized.
> 
> I have no philosophical problem with that type of change, but I have the
> usual fear of touching anything in libiberty that's been around this
> long ;-)
> 
> (this bug being a prime example of how subtle an incorrect change can be)
> 
> (and honestly, my upstream attention is elsewhere these days)
> 

Thanks,
Pedro Alves