public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: James E Wilson <wilson@specifixinc.com>
To: DJ Delorie <dj@redhat.com>
Cc: sje@cup.hp.com, binutils@sources.redhat.com
Subject: Re: Another HP-UX IA64 Build patch
Date: Fri, 06 May 2005 01:57:00 -0000	[thread overview]
Message-ID: <1115335760.8413.204.camel@aretha.corp.specifixinc.com> (raw)
In-Reply-To: <200505052224.j45MOH22019801@greed.delorie.com>

On Thu, 2005-05-05 at 15:24, DJ Delorie wrote:
> One option is to not allow the use of basename() without the
> HAVE_DECL_BASENAME test.  We can do this by #definining basename() to
> you_forgot_the_have_decl_basename_check() in the non-prototype case in
> libiberty.h.

This makes the solution more obvious, but does not seem to fix anything.

The problem here is that libiberty.h declares basename always, even if
you aren't using it.  Thus we must always have a configure test for
basename even if we aren't using it.  This change just makes it more
obvious that the otherwise unnecessary configure test is missing.

Also, as you mention, this is likely to break lots of code using
libiberty.h that isn't already using -Wstrict-prototypes and -Werror, as
the current libiberty.h code fails only in this case.

> But how does gcc deal with system headers that have empty prototypes
> for functions?  If we can hook into that, that would help.  But I
> wouldn't recommend using <libiberty.h> where "libiberty.h" is
> appropriate.

I think the main answer to this is that now we require an ISO C
compiler, everybody has prototypes for everything that matters.

-Wstrict-prototypes does not warn for declarations in system header
files.  -Wmissing-prototypes requires a prototype before use.  So we
only need to ensure that a prototype exists in the local gcc system.h
(or whatever) if there is no prototype in the system header files. 
However, I don't think this is a problem, because I think everybody we
care about has prototypes in the system header files nowadays.

This fails in the libiberty.h case, because libiberty.h is not a system
header file, and contains a non-prototype declaration for basename. 
Using <libiberty.h> would fix this, but as you say, might cause other
problems, like getting the wrong version of it.

The use of libiberty.h in gcc itself is not a problem, because gcc has
the basename configure check.
 
> basename is different because we *can't* just emit a prototype, as it
> sometimes conflicts with the OS's prototype.

The prototype is only emitted if there is a configure check for
basename, and configure failed to find it.  In this case, there should
be no conflict unless the configure check is broken

> It's the only one that doesn't return "int" so we must, or we corrupt
> pointers on 64-bit-pointer machines.

So what you are saying here is that we declare basename even if there is
no configure check for basename, just in case someone needed the
configure check but forgot to perform it.  However, in this case, I
would argue that the libiberty.h user is clearly broken (because of the
missing configure check), and it isn't libiberty's responsibility to
make it work.

Changing this might break some programs that have relied on this feature
though.

This brings us back to the previously suggested solutions, hack in
__hpux__ like is done for other targets, or add otherwise unnecessary
basename configure checks in the binutils and gas directories.

I suppose if we want a more involved fix, we could modify gcc to define
some macros when -Wstrict-prototypes -Wmissing-prototypes and -Werror
are used.  We can then modify libiberty.h to check for these macros.  If
all three are defined, then we don't emit the non-prototype declaration
of basename.  In this case, if someone did use it by accident without
the configure check, and it was necessary, then they would get an error
from gcc for the missing prototype.  This seems safe, but requires a new
gcc release to build binutils, which is impractical.
-- 
Jim Wilson, GNU Tools Support, http://www.SpecifixInc.com


  reply	other threads:[~2005-05-05 23:29 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2005-05-05 17:26 Steve Ellcey
2005-05-05 17:41 ` Daniel Jacobowitz
2005-05-05 17:55   ` Steve Ellcey
2005-05-05 18:12     ` James E Wilson
2005-05-05 18:16       ` Daniel Jacobowitz
2005-05-05 19:17         ` James E Wilson
2005-05-05 19:32       ` DJ Delorie
2005-05-05 19:47         ` James E Wilson
2005-05-05 21:40       ` Andreas Schwab
2005-05-05 21:43         ` DJ Delorie
2005-05-05 18:10 ` James E Wilson
2005-05-05 20:12 ` James E Wilson
2005-05-05 20:42   ` Steve Ellcey
2005-05-05 21:36     ` DJ Delorie
2005-05-05 21:41       ` James E Wilson
2005-05-05 22:46         ` DJ Delorie
2005-05-06  1:57           ` James E Wilson [this message]
2005-05-06  1:58             ` DJ Delorie
2005-05-09 23:28               ` Steve Ellcey
2005-05-09 23:33                 ` DJ Delorie
2005-05-12 16:37                   ` Steve Ellcey
2005-05-05 22:06   ` James E Wilson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1115335760.8413.204.camel@aretha.corp.specifixinc.com \
    --to=wilson@specifixinc.com \
    --cc=binutils@sources.redhat.com \
    --cc=dj@redhat.com \
    --cc=sje@cup.hp.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).