Introduce and use gdb::unlinker

This introduces a new class, gdb::unlinker, that unlinks a file in the
destructor.  The user of this class has the option to preserve the
file instead, by calling the "keep" method.

This patch then changes the spots in gdb that use unlink in a cleanup
to use this class instead.  In one spot I went ahead and removed all
the cleanups from the function.

This fixes one latent bug -- do_bfd_delete_cleanup could refer to
freed memory, by decref'ing the BFD before using its filename.

2017-01-10  Tom Tromey  <tom@tromey.com>

	* record-full.c (record_full_save_cleanups): Remove.
	(record_full_save): Use gdb::unlinker.
	* gcore.c (do_bfd_delete_cleanup): Remove.
	(gcore_command): Use gdb::unlinker, unique_xmalloc_ptr.  Remove
	cleanups.
	* dwarf2read.c (unlink_if_set): Remove.
	(write_psymtabs_to_index): Use gdb::unlinker.
	* common/gdb_unlinker.h: New file.

5 files changed, 110 insertions, 74 deletions

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 57f21c2..f1e6458 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,16 @@
 2017-01-10  Tom Tromey  <tom@tromey.com>
 
+	* record-full.c (record_full_save_cleanups): Remove.
+	(record_full_save): Use gdb::unlinker.
+	* gcore.c (do_bfd_delete_cleanup): Remove.
+	(gcore_command): Use gdb::unlinker, unique_xmalloc_ptr.  Remove
+	cleanups.
+	* dwarf2read.c (unlink_if_set): Remove.
+	(write_psymtabs_to_index): Use gdb::unlinker.
+	* common/gdb_unlinker.h: New file.
+
+2017-01-10  Tom Tromey  <tom@tromey.com>
+
 	* windows-tdep.c (windows_xfer_shared_library): Update.
 	* windows-nat.c (windows_make_so): Update.
 	* utils.h (make_cleanup_bfd_unref): Remove.
diff --git a/gdb/common/gdb_unlinker.h b/gdb/common/gdb_unlinker.h
new file mode 100644
index 0000000..0f3b98a
--- /dev/null
+++ b/gdb/common/gdb_unlinker.h
@@ -0,0 +1,60 @@
+/* Unlinking class
+
+   Copyright (C) 2016 Free Software Foundation, Inc.
+
+   This file is part of GDB.
+
+   This program is free software; you can redistribute it and/or modify
+   it under the terms of the GNU General Public License as published by
+   the Free Software Foundation; either version 3 of the License, or
+   (at your option) any later version.
+
+   This program is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+   GNU General Public License for more details.
+
+   You should have received a copy of the GNU General Public License
+   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */
+
+#ifndef GDB_UNLINKER_H
+#define GDB_UNLINKER_H
+
+namespace gdb
+{
+
+/* An object of this class holds a filename and, when the object goes
+   of scope, the file is removed using unlink.
+
+   A user of this class can request that the file be preserved using
+   the "keep" method.  */
+class unlinker
+{
+ public:
+
+  unlinker (const char *filename) ATTRIBUTE_NONNULL (1)
+    : m_filename (filename)
+  {
+    gdb_assert (filename != NULL);
+  }
+
+  ~unlinker ()
+  {
+    if (m_filename != NULL)
+      unlink (m_filename);
+  }
+
+  /* Keep the file, rather than unlink it.  */
+  void keep ()
+  {
+    m_filename = NULL;
+  }
+
+ private:
+
+  const char *m_filename;
+};
+
+}
+
+#endif /* GDB_UNLINKER_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 8a66848..dfb79c9 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -69,6 +69,7 @@
 #include "filestuff.h"
 #include "build-id.h"
 #include "namespace.h"
+#include "common/gdb_unlinker.h"
 
 #include <fcntl.h>
 #include <sys/types.h>
@@ -23165,16 +23166,6 @@ write_obstack (FILE *file, struct obstack *obstack)
     error (_("couldn't data write to file"));
 }
 
-/* Unlink a file if the argument is not NULL.  */
-
-static void
-unlink_if_set (void *p)
-{
-  char **filename = (char **) p;
-  if (*filename)
-    unlink (*filename);
-}
-
 /* A helper struct used when iterating over debug_types.  */
 struct signatured_type_index_data
 {
@@ -23259,7 +23250,7 @@ static void
 write_psymtabs_to_index (struct objfile *objfile, const char *dir)
 {
   struct cleanup *cleanup;
-  char *filename, *cleanup_filename;
+  char *filename;
   struct obstack contents, addr_obstack, constant_pool, symtab_obstack;
   struct obstack cu_list, types_cu_list;
   int i;
@@ -23289,8 +23280,7 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
   if (!out_file)
     error (_("Can't open `%s' for writing"), filename);
 
-  cleanup_filename = filename;
-  make_cleanup (unlink_if_set, &cleanup_filename);
+  gdb::unlinker unlink_file (filename);
 
   symtab = create_mapped_symtab ();
   make_cleanup (cleanup_mapped_symtab, symtab);
@@ -23429,9 +23419,8 @@ write_psymtabs_to_index (struct objfile *objfile, const char *dir)
 
   fclose (out_file);
 
-  /* We want to keep the file, so we set cleanup_filename to NULL
-     here.  See unlink_if_set.  */
-  cleanup_filename = NULL;
+  /* We want to keep the file.  */
+  unlink_file.keep ();
 
   do_cleanups (cleanup);
 }
diff --git a/gdb/gcore.c b/gdb/gcore.c
index 1d8a63f..c32d2ff 100644
--- a/gdb/gcore.c
+++ b/gdb/gcore.c
@@ -35,6 +35,7 @@
 #include "gdb_bfd.h"
 #include "readline/tilde.h"
 #include <algorithm>
+#include "common/gdb_unlinker.h"
 
 /* The largest amount of memory to read from the target at once.  We
    must throttle it to limit the amount of memory used by GDB during
@@ -135,59 +136,44 @@ write_gcore_file (bfd *obfd)
     throw_exception (except);
 }
 
-static void
-do_bfd_delete_cleanup (void *arg)
-{
-  bfd *obfd = (bfd *) arg;
-  const char *filename = obfd->filename;
-
-  gdb_bfd_unref ((bfd *) arg);
-  unlink (filename);
-}
-
 /* gcore_command -- implements the 'gcore' command.
    Generate a core file from the inferior process.  */
 
 static void
 gcore_command (char *args, int from_tty)
 {
-  struct cleanup *filename_chain;
-  struct cleanup *bfd_chain;
-  char *corefilename;
-  bfd *obfd;
+  gdb::unique_xmalloc_ptr<char> corefilename;
 
   /* No use generating a corefile without a target process.  */
   if (!target_has_execution)
     noprocess ();
 
   if (args && *args)
-    corefilename = tilde_expand (args);
+    corefilename.reset (tilde_expand (args));
   else
     {
       /* Default corefile name is "core.PID".  */
-      corefilename = xstrprintf ("core.%d", ptid_get_pid (inferior_ptid));
+      corefilename.reset (xstrprintf ("core.%d", ptid_get_pid (inferior_ptid)));
     }
-  filename_chain = make_cleanup (xfree, corefilename);
 
   if (info_verbose)
     fprintf_filtered (gdb_stdout,
-		      "Opening corefile '%s' for output.\n", corefilename);
+		      "Opening corefile '%s' for output.\n",
+		      corefilename.get ());
 
   /* Open the output file.  */
-  obfd = create_gcore_bfd (corefilename).release ();
+  gdb_bfd_ref_ptr obfd (create_gcore_bfd (corefilename.get ()));
 
-  /* Need a cleanup that will close and delete the file.  */
-  bfd_chain = make_cleanup (do_bfd_delete_cleanup, obfd);
+  /* Arrange to unlink the file on failure.  */
+  gdb::unlinker unlink_file (corefilename.get ());
 
   /* Call worker function.  */
-  write_gcore_file (obfd);
+  write_gcore_file (obfd.get ());
 
   /* Succeeded.  */
-  discard_cleanups (bfd_chain);
-  gdb_bfd_unref (obfd);
+  unlink_file.keep ();
 
-  fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename);
-  do_cleanups (filename_chain);
+  fprintf_filtered (gdb_stdout, "Saved corefile %s\n", corefilename.get ());
 }
 
 static unsigned long
diff --git a/gdb/record-full.c b/gdb/record-full.c
index e4ff6a4..fdd613b 100644
--- a/gdb/record-full.c
+++ b/gdb/record-full.c
@@ -35,6 +35,7 @@
 #include "gdb_bfd.h"
 #include "observer.h"
 #include "infrun.h"
+#include "common/gdb_unlinker.h"
 
 #include <signal.h>
 
@@ -2537,17 +2538,6 @@ cmd_record_full_restore (char *args, int from_tty)
   record_full_open (args, from_tty);
 }
 
-static void
-record_full_save_cleanups (void *data)
-{
-  bfd *obfd = (bfd *) data;
-  char *pathname = xstrdup (bfd_get_filename (obfd));
-
-  gdb_bfd_unref (obfd);
-  unlink (pathname);
-  xfree (pathname);
-}
-
 /* Save the execution log to a file.  We use a modified elf corefile
    format, with an extra section for our data.  */
 
@@ -2558,9 +2548,7 @@ record_full_save (struct target_ops *self, const char *recfilename)
   uint32_t magic;
   struct regcache *regcache;
   struct gdbarch *gdbarch;
-  struct cleanup *old_cleanups;
   struct cleanup *set_cleanups;
-  bfd *obfd;
   int save_size = 0;
   asection *osec = NULL;
   int bfd_offset = 0;
@@ -2571,8 +2559,10 @@ record_full_save (struct target_ops *self, const char *recfilename)
 			recfilename);
 
   /* Open the output file.  */
-  obfd = create_gcore_bfd (recfilename).release ();
-  old_cleanups = make_cleanup (record_full_save_cleanups, obfd);
+  gdb_bfd_ref_ptr obfd (create_gcore_bfd (recfilename));
+
+  /* Arrange to remove the output file on failure.  */
+  gdb::unlinker unlink_file (recfilename);
 
   /* Save the current record entry to "cur_record_full_list".  */
   cur_record_full_list = record_full_list;
@@ -2615,20 +2605,20 @@ record_full_save (struct target_ops *self, const char *recfilename)
       }
 
   /* Make the new bfd section.  */
-  osec = bfd_make_section_anyway_with_flags (obfd, "precord",
+  osec = bfd_make_section_anyway_with_flags (obfd.get (), "precord",
                                              SEC_HAS_CONTENTS
                                              | SEC_READONLY);
   if (osec == NULL)
     error (_("Failed to create 'precord' section for corefile %s: %s"),
 	   recfilename,
            bfd_errmsg (bfd_get_error ()));
-  bfd_set_section_size (obfd, osec, save_size);
-  bfd_set_section_vma (obfd, osec, 0);
-  bfd_set_section_alignment (obfd, osec, 0);
-  bfd_section_lma (obfd, osec) = 0;
+  bfd_set_section_size (obfd.get (), osec, save_size);
+  bfd_set_section_vma (obfd.get (), osec, 0);
+  bfd_set_section_alignment (obfd.get (), osec, 0);
+  bfd_section_lma (obfd.get (), osec) = 0;
 
   /* Save corefile state.  */
-  write_gcore_file (obfd);
+  write_gcore_file (obfd.get ());
 
   /* Write out the record log.  */
   /* Write the magic code.  */
@@ -2638,7 +2628,7 @@ record_full_save (struct target_ops *self, const char *recfilename)
 			"  Writing 4-byte magic cookie "
 			"RECORD_FULL_FILE_MAGIC (0x%s)\n",
 		      phex_nz (magic, 4));
-  bfdcore_write (obfd, osec, &magic, sizeof (magic), &bfd_offset);
+  bfdcore_write (obfd.get (), osec, &magic, sizeof (magic), &bfd_offset);
 
   /* Save the entries to recfd and forward execute to the end of
      record list.  */
@@ -2653,7 +2643,7 @@ record_full_save (struct target_ops *self, const char *recfilename)
           uint64_t addr;
 
 	  type = record_full_list->type;
-          bfdcore_write (obfd, osec, &type, sizeof (type), &bfd_offset);
+          bfdcore_write (obfd.get (), osec, &type, sizeof (type), &bfd_offset);
 
           switch (record_full_list->type)
             {
@@ -2668,11 +2658,11 @@ record_full_save (struct target_ops *self, const char *recfilename)
 
               /* Write regnum.  */
               regnum = netorder32 (record_full_list->u.reg.num);
-              bfdcore_write (obfd, osec, &regnum,
+              bfdcore_write (obfd.get (), osec, &regnum,
 			     sizeof (regnum), &bfd_offset);
 
               /* Write regval.  */
-              bfdcore_write (obfd, osec,
+              bfdcore_write (obfd.get (), osec,
 			     record_full_get_loc (record_full_list),
 			     record_full_list->u.reg.len, &bfd_offset);
               break;
@@ -2690,15 +2680,16 @@ record_full_save (struct target_ops *self, const char *recfilename)
 
 	      /* Write memlen.  */
 	      len = netorder32 (record_full_list->u.mem.len);
-	      bfdcore_write (obfd, osec, &len, sizeof (len), &bfd_offset);
+	      bfdcore_write (obfd.get (), osec, &len, sizeof (len),
+			     &bfd_offset);
 
 	      /* Write memaddr.  */
 	      addr = netorder64 (record_full_list->u.mem.addr);
-	      bfdcore_write (obfd, osec, &addr, 
+	      bfdcore_write (obfd.get (), osec, &addr, 
 			     sizeof (addr), &bfd_offset);
 
 	      /* Write memval.  */
-	      bfdcore_write (obfd, osec,
+	      bfdcore_write (obfd.get (), osec,
 			     record_full_get_loc (record_full_list),
 			     record_full_list->u.mem.len, &bfd_offset);
               break;
@@ -2712,12 +2703,12 @@ record_full_save (struct target_ops *self, const char *recfilename)
 				      (unsigned long) sizeof (count));
 		/* Write signal value.  */
 		signal = netorder32 (record_full_list->u.end.sigval);
-		bfdcore_write (obfd, osec, &signal,
+		bfdcore_write (obfd.get (), osec, &signal,
 			       sizeof (signal), &bfd_offset);
 
 		/* Write insn count.  */
 		count = netorder32 (record_full_list->u.end.insn_num);
-		bfdcore_write (obfd, osec, &count,
+		bfdcore_write (obfd.get (), osec, &count,
 			       sizeof (count), &bfd_offset);
                 break;
             }
@@ -2746,8 +2737,7 @@ record_full_save (struct target_ops *self, const char *recfilename)
     }
 
   do_cleanups (set_cleanups);
-  gdb_bfd_unref (obfd);
-  discard_cleanups (old_cleanups);
+  unlink_file.keep ();
 
   /* Succeeded.  */
   printf_filtered (_("Saved core file %s with execution log.\n"),