RFA: always close fd if bfd_fopen fails

Tom Tromey tromey@redhat.com
Fri May 25 13:52:00 GMT 2012


Currently, bfd_fopen has confused error handling.

If the caller passes in a file descriptor, then on any error exit before
fdopen is called, the fd will be left open; but on any error exit after
fdopen is called, the fd will be closed.

This patch changes bfd_fopen to always close the fd on error.

I audited all callers of bfd_fopen and bfd_fdopen in gdb, binutils, and
ld and fixed them not to close the fd on error.  There was only one call
outside of gdb, in ar.c, and it immediately calls bfd_fatal, and so does
not need a change.

Ok?

Tom

b/bfd/ChangeLog:
2012-05-25  Tom Tromey  <tromey@redhat.com>

	* opncls.c (bfd_fopen): Always close fd on failure.
	(bfd_fdopenr): Likewise.

b/gdb/ChangeLog:
2012-05-25  Tom Tromey  <tromey@redhat.com>

	* symfile.c (symfile_bfd_open): Don't close desc if bfd_fopen
	fails.
	* solib.c (solib_bfd_fopen): Don't close fd if bfd_fopen fails.
	* exec.c (exec_file_attach): Don't close scratch_chan if bfd_fopen
	fails.
	* dwarf2read.c (try_open_dwo_file): Don't close fd if bfd_fopen
	fails.

diff --git a/bfd/opncls.c b/bfd/opncls.c
index 9d33f39..7c1d2f9 100644
--- a/bfd/opncls.c
+++ b/bfd/opncls.c
@@ -1,6 +1,6 @@
 /* opncls.c -- open and close a BFD.
    Copyright 1990, 1991, 1992, 1993, 1994, 1995, 1996, 1997, 1998, 2000,
-   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010
+   2001, 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2012
    Free Software Foundation, Inc.
 
    Written by Cygnus Support.
@@ -190,6 +190,8 @@ DESCRIPTION
 	If <<NULL>> is returned then an error has occured.   Possible errors
 	are <<bfd_error_no_memory>>, <<bfd_error_invalid_target>> or
 	<<system_call>> error.
+
+	On error, @var{fd} is always closed.
 */
 
 bfd *
@@ -200,11 +202,17 @@ bfd_fopen (const char *filename, const char *target, const char *mode, int fd)
 
   nbfd = _bfd_new_bfd ();
   if (nbfd == NULL)
-    return NULL;
+    {
+      if (fd != -1)
+	close (fd);
+      return NULL;
+    }
 
   target_vec = bfd_find_target (target, nbfd);
   if (target_vec == NULL)
     {
+      if (fd != -1)
+	close (fd);
       _bfd_delete_bfd (nbfd);
       return NULL;
     }
@@ -307,6 +315,8 @@ DESCRIPTION
 
 	Possible errors are <<bfd_error_no_memory>>,
 	<<bfd_error_invalid_target>> and <<bfd_error_system_call>>.
+
+	On error, @var{fd} is closed.
 */
 
 bfd *
@@ -323,6 +333,10 @@ bfd_fdopenr (const char *filename, const char *target, int fd)
   fdflags = fcntl (fd, F_GETFL, NULL);
   if (fdflags == -1)
     {
+      int save = errno;
+
+      close (fd);
+      errno = save;
       bfd_set_error (bfd_error_system_call);
       return NULL;
     }
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 53100c5..8dbc53e 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -6957,7 +6957,6 @@ try_open_dwo_file (const char *file_name)
   sym_bfd = bfd_fopen (absolute_name, gnutarget, FOPEN_RB, desc);
   if (!sym_bfd)
     {
-      close (desc);
       xfree (absolute_name);
       return NULL;
     }
diff --git a/gdb/exec.c b/gdb/exec.c
index 58fb55e..6ba1986 100644
--- a/gdb/exec.c
+++ b/gdb/exec.c
@@ -238,7 +238,6 @@ exec_file_attach (char *filename, int from_tty)
 
       if (!exec_bfd)
 	{
-	  close (scratch_chan);
 	  error (_("\"%s\": could not open as an executable file: %s"),
 		 scratch_pathname, bfd_errmsg (bfd_get_error ()));
 	}
diff --git a/gdb/solib.c b/gdb/solib.c
index 656e8df..90439ba 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -380,8 +380,6 @@ solib_bfd_fopen (char *pathname, int fd)
 
       if (abfd)
 	bfd_set_cacheable (abfd, 1);
-      else if (fd != -1)
-	close (fd);
     }
 
   if (!abfd)
diff --git a/gdb/symfile.c b/gdb/symfile.c
index 416d35d..31da4e4 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1764,7 +1764,6 @@ symfile_bfd_open (char *name)
   sym_bfd = bfd_fopen (name, gnutarget, FOPEN_RB, desc);
   if (!sym_bfd)
     {
-      close (desc);
       make_cleanup (xfree, name);
       error (_("`%s': can't open to read symbols: %s."), name,
 	     bfd_errmsg (bfd_get_error ()));



More information about the Binutils mailing list