PATCH: Abort on NULL bfd (Problem with linker with binutils-040414)

["H. J. Lu" <hjl at lucon dot org>]

On Mon, Apr 19, 2004 at 05:20:13PM +0100, Dave Korn wrote:
> > -----Original Message-----
> > From: binutils-owner On Behalf Of H. J. Lu
> > Sent: 19 April 2004 16:55
> 
> > On Mon, Apr 19, 2004 at 08:47:39AM -0700, H. J. Lu wrote:
> > > On Mon, Apr 19, 2004 at 11:43:31AM -0400, Ian Lance Taylor wrote:
> > > > "H. J. Lu" <hjl@lucon.org> writes:
> > > > > What is the defined way for bfd_archive_filename to 
> > report an error?
> > > > > How should it be used? I don't believe 
> > bfd_archive_filename should
> > > > > return a bogus filename on a bogus input to hide the 
> > caller's mistake.
> > > > > It does no one good.
> > > > 
> > > > Well, that I agree with.  A bogus filename doesn't make sense.  It
> > > > should presumably return NULL.
> > > > 
> > > 
> > > Return NULL, abort or segfault are OK with me in this case.
> > > 
> > > 
> > 
> > How about this patch?
> > 
> > 
> > H.J.
> > ----
> > 2004-04-19  H.J. Lu  <hongjiu.lu@intel.com>
> > 
> > 	* bfd.c (bfd_archive_filename): Return NULL on NULL bfd.
> > 
> > --- bfd.c	2004-04-19 08:29:13.000000000 -0700
> > +++ bfd.c	2004-04-19 08:52:19.000000000 -0700
> > @@ -513,7 +513,7 @@ const char *
> >  bfd_archive_filename (bfd *abfd)
> >  {
> >    if (abfd == NULL)
> > -    return _("<unknown>");
> > +    return NULL;
> >    
> >    if (abfd->my_archive)
> >      {
> 
> 
>   Surely if it is done like this, what will happen is that the segv will be
> deferred to some point in the code where the filename is actually used,
> possibly much further on in the execution flow, making it more difficult to
> locate the site of the underlying error that has resulted in a NULL bfd
> pointer being passed around in the first place?
> 
>   With code like this, I fear that people trying to fix bugs in future will
> spend a long time trying to trace down why on earth a null pointer is being
> passed around for a filename; when they eventually do work their way back
> and figure out that it came from here, they'll realise the real problem was
> a null pointer being passed around for a bfd, and they've got to re-do a
> whole load of debugging effort to trace the *real* cause of the problem.
> ISTM that a whole lot of effort and motivation could get wasted that way.
> 
>   On the whole, when it's clearly an internal coding error or other serious
> internal inconsistency, I think the purposes of debugging are better served
> by making the program stop as close as possible to the actual site of the
> fault, rather than having it stagger along in an inconsistent state,
> trashing more of its internal data and making the situation that much more
> unclear to diagnose when it finally does fall down.
> 
> 

This is my first choice. Basically, anything but return bogus filename.


H.J.
---
2004-04-19  H.J. Lu  <hongjiu.lu@intel.com>

	* bfd.c (bfd_archive_filename): Abort on NULL bfd.

--- bfd.c	2004-04-19 08:29:13.000000000 -0700
+++ bfd.c	2004-04-19 08:52:19.000000000 -0700
@@ -513,7 +513,7 @@ const char *
 bfd_archive_filename (bfd *abfd)
 {
   if (abfd == NULL)
-    return _("<unknown>");
+    abort ();
   
   if (abfd->my_archive)
     {