Re: strdup falsely detected when CFLAGS contains -ansi on MSYS/MinGW (gcc)

[Chris Pickett]

Hi Eric,

Eric Blake wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
>
> According to Chris Pickett on 12/29/2008 4:06 PM:
> > In this case, it still doesn't make a difference.
>
> Which means that either AC_USE_SYSTEM_EXTENSIONS needs to be taught how to
> override 'cc -ansi' on mingw, or you are getting exactly what you asked
> for by asking for such a strict compilation environment, since strdup is
> _not_ an ansi function.

I use -ansi for its other features.  Maybe all that's needed is for 
AC_USE_SYSTEM_EXTENSIONS to #undef __STRICT_ANSI__?  I think this is 
logical, but I lack the experience to say whether it breaks things.

> >  I looked at the
> > output from configure and it seems like it gets run after AC_PROG_CC
> > regardless, i.e. `checking for minix/config.h usability' comes after
> > `checking for unistd.h' and all the other C compiler stuff.
>
> Yeah, AC_USE_SYSTEM_EXTENSIONS has an implicit AC_REQUIRE([AC_PROG_CC]),
> so you are pretty much guaranteed that the compiler existence will be
> checked prior to determining the right flags/defines to enable various
> extensions.
>
> > Nevertheless, maybe you want 1.patch.
>
> I'm not sure that it helps much, but if someone else agrees that adding
> the phrase "but after AC_PROG_CC" helped, I'll apply it.  By the way,
> patches are easier to review with a ChangeLog entry, or even when created
> by 'git format-patch' with a commit message that can be converted into a
> ChangeLog entry.

Ok, I will look into it if I send more patches.  Since you have the 
implicit AC_REQUIRE([AC_PROG_CC]) already, it seems it doesn't matter.

> > Maybe you want 2.patch.  I'm not sure if what I wrote is correct, but I
> > didn't find any indications to the contrary.
>
> There are four categories of functions:
>
> declared and linkable (for example, strcpy)
>
> undeclared and not linkable (for example, obstack_grow on non-glibc)
>
> declared but not linkable (for example, isnan, which POSIX says must be a
> macro and therefore does not have to be linkable.  Also, things like
> freopen sometimes fall here, where platforms #define it to freopen64; and
> we have even seen broken headers that declare an unavailable function)
>
> undeclared but linkable (for example, strdup in your scenario)

Well then, clearly declared does not always imply linkable.

Currently I am using:

#if !HAVE_FUNCTION
<rpl_function decl>
#endif

for things like malloc() that are broken, and

#if !HAVE_DECL_FUNCTION
<function decl>
#endif

for things that are undeclared but that may or may not be linkable.  I 
think a section detailing the different possibilities and the different 
recommended #if checks would help.  I suppose you have another 3 
categories of function if you include broken/working.  It would be 4 
except that undeclared and not linkable and broken isn't really logical.

> gnulib tends to be a pretty good collection of wisdom about which
> functions fall in the various categories.

I'm sure you are right and I'm probably going to just bite the bullet 
and switch to gnulib to avoid reinventing the wheel.

> > Finally, I suggest a macro that combines AC_REPLACE_FUNCS and
> > AC_CHECK_DECLS and that this new macro become the recommended usage.  On
> > the other hand, is there ever a time that you explicitly don't want a
> > function to be listed in both AC_REPLACE_FUNCS and AC_CHECK_DECLS?  If
> > not, maybe you can simply get AC_REPLACE_FUNCS to run AC_CHECK_DECLS.
>
> For backward compatibility reasons, we can't change AC_REPLACE_FUNCS.
> Your suggestion of a new macro that tests for both linkability and
> declaration in one go, may have merit, but as gnulib has discovered,
> sometimes one of the two tests is unnecessary, leading to unnecessary time
> spent during configure runs (or even misleading or wrong answers, in the
> case of isnan).  I'm not bothered enough by it myself to write a macro,
> but I'm not opposed to reviewing such a patch, so care to attempt writing
> such a patch?  It will probably be non-trivial, so you may want to take
> care of copyright assignment papers first.

If it's non-trivial it probably isn't worth writing since the number of 
saved bytes is small.  Copyright assignment involves annoying wrangling 
with various people on my end, and I sort of have lots to do already 
coding-wise.  Excuses, excuses.  I do appreciate the invitation though.

I currently have:

AC_REPLACE_FUNCS([fileno localtime_r setenv sleep strdup strsignal 
unsetenv])
AC_CHECK_DECLS([fileno, localtime_r, malloc, realloc, setenv, sleep, 
strdup, strsignal, unsetenv])

and I'm happy enough with that.

> As to your proposed 2.patch, I'll have to think about whether that wording
> helps, or whether yet another wording also makes things more clear.  But I
> will probably end up committing at least something that reminds people
> that AC_REPLACE_FUNCS only kicks in on failed links, and that failed
> declarations are also worth checking.

My wording is incorrect based on your analysis above.

Cheers,
Chris