This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
[patch] bfd_fopen() & co. should strdup (filename)
- From: Jan Kratochvil <jan dot kratochvil at redhat dot com>
- To: binutils at sourceware dot org
- Cc: Michael Snyder <msnyder at access-company dot com>
- Date: Fri, 10 Aug 2007 12:49:21 +0200
- Subject: [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;