Re: [PATCH] Fix PR bootstrap/42798

[Ian Lance Taylor <iant at google dot com>]

Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:

> * Ian Lance Taylor wrote on Fri, Jun 04, 2010 at 04:59:01PM CEST:
>> Ralf Wildenhues <Ralf.Wildenhues@gmx.de> writes:
>> 
>> > This is the followup patch for src.  Tested by building binutils+gdb,
>> > I'm starting a combined tree with --enable-build-with-cxx now.
>> > Ok to commit if no problems show up?
>> >
>> > A note to the gold change: I've looked at git glibc headers, of the
>> > declarations tested only 'basename' was overloaded.  If more are added
>> > in the future, the configure.ac files need to be adjusted to give
>> > parameters for them, too.  This applies to the whole tree of course for
>> > --enable-build-with-cxx, not just for gold.
>> 
>> With regard to the gold change.  What will happen if the system header
>> files provide only a declaration
>>     char *basename(const char *)
>> ?
>> 
>> Since gold doesn't actually basename, another option here would be to
>> skip the check of the basename declaration entirely.  In fact, we
>> should consider dropping basename from libiberty; our code should in
>> general use lbasename anyhow.
>
> Well, these are two additional changes on top of the one I posted though
> and can easily be addressed in the future.  Meanwhile, a combined build
> with maintainer-mode enabled will cause spurious changes to generated
> files, and the patch is pretty minimal in that it fixes the issues but
> doesn't change semantics otherwise.  So, ok to commit to src?

My question about a basename declaration in the system header was a
genuine question.  My concern is that on some systems this patch might
decide incorrectly whether or not basename is defined, which could
then possibly lead to a compilation error when libiberty.h is
included.

The gold patch is fine if you omit basename entirely from the gold
configure.ac.  I don't see how that could cause trouble given the
current libiberty.h.

Ian