This is the mail archive of the binutils@sourceware.org mailing list for the binutils project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[patch] bfd_fopen() & co. should strdup (filename)


Hi,

a recent GDB change:

2007-08-08  Michael Snyder  <msnyder@access-company.com>

        * solib-svr4.c (open_symbol_file_object): Memory leak.
        (svr4_current_sos): Ditto.
        (enable_break): Ditto.

(solib-svr4.c 1.71 -> 1.72)

it broke the GDB testsuite, visible at least by `gdb.base/call-sc.exp':
	(gdb) run
	Starting program: /home/jkratoch/redhat/sources/gdb/testsuite/gdb.base/call-sc-tc
	BFD: reopening <NON-ASCII GARBAGE>: No such file or directory
	(general FAIL)

I believe it is more a bug in BFD as the doc for neither of BFD_FOPEN,
BFD_OPENSTREAMR, BFD_OPENR_IOVEC, BFD_OPENW nor BFD_CREATE says anything about
the required validity of their FILENAME parameter.  Therefore it is generally
assumed it can be free(3)d after the function call returns.

Providing two variants of the patch as I am not sure which one gets accepted.

IMO the only function which must not get FILENAME as NULL is BFD_OPENW - this
simplifies a bit only the `inlined' variant of the patch.


Regards,
Jan
2007-08-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* opncls.c (filename_dup): New function.
	(bfd_fopen): Call FILENAME_DUP for the FILENAME parameter of NBFD.
	(bfd_openstreamr): Likewise.
	(bfd_openr_iovec): Likewise.
	(bfd_openw): Likewise.
	(bfd_create): Likewise.

--- bfd/opncls.c	9 Aug 2007 14:22:03 -0000	1.49
+++ bfd/opncls.c	10 Aug 2007 10:29:25 -0000
@@ -42,6 +42,21 @@
 
 static unsigned int _bfd_id_counter = 0;
 
+static bfd_boolean
+filename_dup (bfd *abfd, const char *filename)
+{
+  if (filename == NULL)
+    abfd->filename = NULL;
+  else
+    {
+      abfd->filename = bfd_alloc (abfd, strlen (filename) + 1);
+      if (abfd->filename == NULL)
+        return FALSE;
+      strcpy ((char *) abfd->filename, filename);
+    }
+  return TRUE;
+}
+
 /* fdopen is a loser -- we should use stdio exclusively.  Unfortunately
    if we do that we can't use fcntl.  */
 
@@ -209,7 +224,11 @@ bfd_fopen (const char *filename, const c
     }
 
   /* OK, put everything where it belongs.  */
-  nbfd->filename = filename;
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
 
   /* Figure out whether the user is opening the file for reading,
      writing, or both, by looking at the MODE argument.  */
@@ -358,7 +377,13 @@ bfd_openstreamr (const char *filename, c
     }
 
   nbfd->iostream = stream;
-  nbfd->filename = filename;
+
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = read_direction;
 
   if (! bfd_cache_init (nbfd))
@@ -542,7 +567,12 @@ bfd_openr_iovec (const char *filename, c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = read_direction;
 
   /* `open (...)' would get expanded by an the open(2) syscall macro.  */
@@ -604,7 +634,12 @@ bfd_openw (const char *filename, const c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   nbfd->direction = write_direction;
 
   if (bfd_open_file (nbfd) == NULL)
@@ -762,7 +797,13 @@ bfd_create (const char *filename, bfd *t
   nbfd = _bfd_new_bfd ();
   if (nbfd == NULL)
     return NULL;
-  nbfd->filename = filename;
+
+  if (!filename_dup (nbfd, filename))
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+
   if (templ)
     nbfd->xvec = templ->xvec;
   nbfd->direction = no_direction;
2007-08-10  Jan Kratochvil  <jan.kratochvil@redhat.com>

	* opncls.c (bfd_fopen): STRDUP the FILENAME parameter for NBFD.
	(bfd_openstreamr): Likewise.
	(bfd_openr_iovec): Likewise.
	(bfd_openw): Likewise.
	(bfd_create): Likewise.

--- bfd/opncls.c	9 Aug 2007 14:22:03 -0000	1.49
+++ bfd/opncls.c	10 Aug 2007 10:45:20 -0000
@@ -209,7 +209,18 @@ bfd_fopen (const char *filename, const c
     }
 
   /* OK, put everything where it belongs.  */
-  nbfd->filename = filename;
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
 
   /* Figure out whether the user is opening the file for reading,
      writing, or both, by looking at the MODE argument.  */
@@ -358,7 +369,20 @@ bfd_openstreamr (const char *filename, c
     }
 
   nbfd->iostream = stream;
-  nbfd->filename = filename;
+
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
+
   nbfd->direction = read_direction;
 
   if (! bfd_cache_init (nbfd))
@@ -542,7 +566,19 @@ bfd_openr_iovec (const char *filename, c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
+
   nbfd->direction = read_direction;
 
   /* `open (...)' would get expanded by an the open(2) syscall macro.  */
@@ -604,7 +640,14 @@ bfd_openw (const char *filename, const c
       return NULL;
     }
 
-  nbfd->filename = filename;
+  nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+  if (nbfd->filename == NULL)
+    {
+      _bfd_delete_bfd (nbfd);
+      return NULL;
+    }
+  strcpy ((char *) nbfd->filename, filename);
+
   nbfd->direction = write_direction;
 
   if (bfd_open_file (nbfd) == NULL)
@@ -762,7 +805,20 @@ bfd_create (const char *filename, bfd *t
   nbfd = _bfd_new_bfd ();
   if (nbfd == NULL)
     return NULL;
-  nbfd->filename = filename;
+
+  if (filename == NULL)
+    nbfd->filename = NULL;
+  else
+    {
+      nbfd->filename = bfd_alloc (nbfd, strlen (filename) + 1);
+      if (nbfd->filename == NULL)
+	{
+	  _bfd_delete_bfd (nbfd);
+	  return NULL;
+	}
+      strcpy ((char *) nbfd->filename, filename);
+    }
+
   if (templ)
     nbfd->xvec = templ->xvec;
   nbfd->direction = no_direction;

Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]