abort in bfd_cache_lookup_worker

[Alan Modra <amodra at bigpond dot net dot au>]

Hi Nick,
  In http://sources.redhat.com/bugzilla/show_bug.cgi?id=136, you fixed a
problem with bfd_cache_lookup returning NULL and resulting segfaults, by
aborting when bfd_cache_lookup_worker couldn't reopen a file for some
reason.  Having looked at this today, I'm inclined to think that an
abort is inappropriate, because a bfd_cache_lookup_worker failure could
easily be due to user actions such as deleting the file, or the
application using BFD truncating the file or opening too many files.
"BFD: internal error" and "BFD: Please report this bug" are likely to
mislead people into thinking that the problem is in BFD, when it's more
likely elsewhere.

So, how about we remove the abort but continue to squark about the error?

	* cache.c (bfd_cache_lookup_worker): Don't abort on failing to
	reopen file.
	(cache_btell, cache_bseek, cache_bflush, cache_bstat): Return -1 on
	bfd_cache_lookup failure.
	(cache_bread, cache_bwrite): Return 0 on the same.
	* bfdwin.c (bfd_get_file_window): Likewise.
	* hppabsd-core.c (hppabsd_core_core_file_p): Likewise.
	* sco5-core.c (sco5_core_file_p): Likewise.
	* trad-core.c (trad_unix_core_file_p): Likewise.

Index: bfd/cache.c
===================================================================
RCS file: /cvs/src/src/bfd/cache.c,v
retrieving revision 1.23
diff -u -p -r1.23 cache.c
--- bfd/cache.c	26 Oct 2005 07:38:25 -0000	1.23
+++ bfd/cache.c	26 Oct 2005 08:45:04 -0000
@@ -51,13 +51,19 @@ static bfd_boolean bfd_cache_delete (bfd
 static file_ptr
 cache_btell (struct bfd *abfd)
 {
-  return real_ftell (bfd_cache_lookup (abfd));
+  FILE *f = bfd_cache_lookup (abfd);
+  if (f == NULL)
+    return -1;
+  return real_ftell (f);
 }
 
 static int
 cache_bseek (struct bfd *abfd, file_ptr offset, int whence)
 {
-  return real_fseek (bfd_cache_lookup (abfd), offset, whence);
+  FILE *f = bfd_cache_lookup (abfd);
+  if (f == NULL)
+    return -1;
+  return real_fseek (f, offset, whence);
 }
 
 /* Note that archive entries don't have streams; they share their parent's.
@@ -70,6 +76,7 @@ cache_bseek (struct bfd *abfd, file_ptr 
 static file_ptr
 cache_bread (struct bfd *abfd, void *buf, file_ptr nbytes)
 {
+  FILE *f;
   file_ptr nread;
   /* FIXME - this looks like an optimization, but it's really to cover
      up for a feature of some OSs (not solaris - sigh) that
@@ -83,10 +90,14 @@ cache_bread (struct bfd *abfd, void *buf
   if (nbytes == 0)
     return 0;
 
+  f = bfd_cache_lookup (abfd);
+  if (f == NULL)
+    return 0;
+
 #if defined (__VAX) && defined (VMS)
   /* Apparently fread on Vax VMS does not keep the record length
      information.  */
-  nread = read (fileno (bfd_cache_lookup (abfd)), buf, nbytes);
+  nread = read (fileno (f), buf, nbytes);
   /* Set bfd_error if we did not read as much data as we expected.  If
      the read failed due to an error set the bfd_error_system_call,
      else set bfd_error_file_truncated.  */
@@ -96,11 +107,11 @@ cache_bread (struct bfd *abfd, void *buf
       return -1;
     }
 #else
-  nread = fread (buf, 1, nbytes, bfd_cache_lookup (abfd));
+  nread = fread (buf, 1, nbytes, f);
   /* Set bfd_error if we did not read as much data as we expected.  If
      the read failed due to an error set the bfd_error_system_call,
      else set bfd_error_file_truncated.  */
-  if (nread < nbytes && ferror (bfd_cache_lookup (abfd)))
+  if (nread < nbytes && ferror (f))
     {
       bfd_set_error (bfd_error_system_call);
       return -1;
@@ -112,8 +123,12 @@ cache_bread (struct bfd *abfd, void *buf
 static file_ptr
 cache_bwrite (struct bfd *abfd, const void *where, file_ptr nbytes)
 {
-  file_ptr nwrite = fwrite (where, 1, nbytes, bfd_cache_lookup (abfd));
-  if (nwrite < nbytes && ferror (bfd_cache_lookup (abfd)))
+  file_ptr nwrite;
+  FILE *f = bfd_cache_lookup (abfd);
+  if (f == NULL)
+    return 0;
+  nwrite = fwrite (where, 1, nbytes, f);
+  if (nwrite < nbytes && ferror (f))
     {
       bfd_set_error (bfd_error_system_call);
       return -1;
@@ -130,7 +145,11 @@ cache_bclose (struct bfd *abfd)
 static int
 cache_bflush (struct bfd *abfd)
 {
-  int sts = fflush (bfd_cache_lookup (abfd));
+  int sts;
+  FILE *f = bfd_cache_lookup (abfd);
+  if (f == NULL)
+    return -1;
+  sts = fflush (f);
   if (sts < 0)
     bfd_set_error (bfd_error_system_call);
   return sts;
@@ -139,7 +158,11 @@ cache_bflush (struct bfd *abfd)
 static int
 cache_bstat (struct bfd *abfd, struct stat *sb)
 {
-  int sts = fstat (fileno (bfd_cache_lookup (abfd)), sb);
+  int sts;
+  FILE *f = bfd_cache_lookup (abfd);
+  if (f == NULL)
+    return -1;
+  sts = fstat (fileno (f), sb);
   if (sts < 0)
     bfd_set_error (bfd_error_system_call);
   return sts;
@@ -470,8 +493,8 @@ DESCRIPTION
 	quick answer.  Find a file descriptor for @var{abfd}.  If
 	necessary, it open it.  If there are already more than
 	<<BFD_CACHE_MAX_OPEN>> files open, it tries to close one first, to
-	avoid running out of file descriptors.  It will abort rather than
-	returning NULL if it is unable to (re)open the @var{abfd}.
+	avoid running out of file descriptors.  It will return NULL
+	if it is unable to (re)open the @var{abfd}.
 */
 
 FILE *
@@ -504,6 +527,5 @@ bfd_cache_lookup_worker (bfd *abfd)
 
   (*_bfd_error_handler) (_("reopening %B: %s\n"),
 			 orig_bfd, bfd_errmsg (bfd_get_error ()));
-  abort ();
   return NULL;
 }
Index: bfd/bfdwin.c
===================================================================
RCS file: /cvs/src/src/bfd/bfdwin.c,v
retrieving revision 1.6
diff -u -p -r1.6 bfdwin.c
--- bfd/bfdwin.c	4 May 2005 15:53:01 -0000	1.6
+++ bfd/bfdwin.c	26 Oct 2005 07:42:18 -0000
@@ -153,6 +153,8 @@ bfd_get_file_window (bfd *abfd,
 	  abfd = abfd->my_archive;
 	}
       f = bfd_cache_lookup (abfd);
+      if (f == NULL)
+	return FALSE;
       fd = fileno (f);
 
       /* Compute offsets and size for mmap and for the user's data.  */
Index: bfd/hppabsd-core.c
===================================================================
RCS file: /cvs/src/src/bfd/hppabsd-core.c,v
retrieving revision 1.15
diff -u -p -r1.15 hppabsd-core.c
--- bfd/hppabsd-core.c	4 May 2005 15:53:32 -0000	1.15
+++ bfd/hppabsd-core.c	26 Oct 2005 07:42:18 -0000
@@ -140,6 +140,8 @@ hppabsd_core_core_file_p (abfd)
     FILE *stream = bfd_cache_lookup (abfd);
     struct stat statbuf;
 
+    if (stream == NULL)
+      return NULL;
     if (fstat (fileno (stream), &statbuf) < 0)
       {
 	bfd_set_error (bfd_error_system_call);
Index: bfd/sco5-core.c
===================================================================
RCS file: /cvs/src/src/bfd/sco5-core.c,v
retrieving revision 1.15
diff -u -p -r1.15 sco5-core.c
--- bfd/sco5-core.c	4 May 2005 15:53:38 -0000	1.15
+++ bfd/sco5-core.c	26 Oct 2005 07:42:18 -0000
@@ -129,6 +129,8 @@ sco5_core_file_p (abfd)
     FILE *stream = bfd_cache_lookup (abfd);
     struct stat statbuf;
 
+    if (stream == NULL)
+      return NULL;
     if (fstat (fileno (stream), &statbuf) < 0)
       {
 	bfd_set_error (bfd_error_system_call);
Index: bfd/trad-core.c
===================================================================
RCS file: /cvs/src/src/bfd/trad-core.c,v
retrieving revision 1.20
diff -u -p -r1.20 trad-core.c
--- bfd/trad-core.c	4 May 2005 15:53:40 -0000	1.20
+++ bfd/trad-core.c	26 Oct 2005 07:42:18 -0000
@@ -112,6 +112,8 @@ trad_unix_core_file_p (abfd)
     FILE *stream = bfd_cache_lookup (abfd);
     struct stat statbuf;
 
+    if (stream == NULL)
+      return 0;
     if (fstat (fileno (stream), &statbuf) < 0)
       {
 	bfd_set_error (bfd_error_system_call);

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre