[pushed] Re: [PATCH 1/6] Code cleanup: C++ify .gdb_index producer.

Message ID 192eac60-b233-cbe6-a613-8e01b20e6630@redhat.com
State New, archived
Headers

Commit Message

Pedro Alves June 12, 2017, 4:18 p.m. UTC
  On 06/12/2017 05:08 PM, Pedro Alves wrote:

> Thanks.  This looks generally good.
> 
> It had a number of GDB/GCC code convention issues though.
> 
> - Should use /**/ for comments.
> - CamelCase should only used in template parameters.
> - Function/ctors should be placed before data members.
> - Private data members should be prefixed with "m_".
> - Some function intro comments still referred to
>   obstacks, while the functions now take a DataBuf.
> 
> Also, the big TRY block would be better done with a RAII type.
> Tromey posted a patch to add a unique_ptr with a deleter that calls
> fclose, and makes that code use it, but that's not merged yet.
> Meanwhile we can add a little RAII class locally that does the
> same thing, still avoiding the churn.

(...)

> Anyway, in interest of forward progress:
> 
> - I fixed up the style issues mentioned above, and pushed the
>   result in, as below.  I'll post the diff as a reply to this
>   email.
> 
>   I'd much appreciate it if you'd adjust the rest of the series
>   similarly.


Thanks,
Pedro Alves
  

Patch

diff --git a/gdb/common/common-defs.h b/gdb/common/common-defs.h
index e624c5e..439bce6 100644
--- a/gdb/common/common-defs.h
+++ b/gdb/common/common-defs.h
@@ -91,18 +91,4 @@ 
 /* Pull in gdb::unique_xmalloc_ptr.  */
 #include "common/gdb_unique_ptr.h"
 
-// Provide C++14 std::make_unique<> for C++11 compilation mode.
-// A copy from: gcc/libstdc++-v3/include/bits/unique_ptr.h
-#if __cplusplus <= 201103L
-namespace std {
-  template<typename _Tp>
-    struct _MakeUniq
-    { typedef unique_ptr<_Tp> __single_object; };
-  template<typename _Tp, typename... _Args>
-    inline typename _MakeUniq<_Tp>::__single_object
-    make_unique(_Args&&... __args)
-    { return unique_ptr<_Tp>(new _Tp(std::forward<_Args>(__args)...)); }
-}
-#endif
-
 #endif /* COMMON_DEFS_H */
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 12a194a..c5c5d55 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 "common/function-view.h"
 #include "common/gdb_optional.h"
 #include "common/underlying.h"
@@ -23194,7 +23195,8 @@  dwarf2_per_objfile_free (struct objfile *objfile, void *d)
 
 /* The "save gdb-index" command.  */
 
-// Write to FILE bytes starting at DATA of length SIZE with error checking.
+/* Write SIZE bytes from the buffer pointed to by DATA to FILE, with
+   error checking.  */
 
 static void
 file_write (FILE *file, const void *data, size_t size)
@@ -23203,66 +23205,58 @@  file_write (FILE *file, const void *data, size_t size)
     error (_("couldn't data write to file"));
 }
 
-// Write to FILE bytes of std::vector VEC with error checking.
+/* Write the contents of VEC to FILE, with error checking.  */
 
-template<class Elem> static void
+template<class Elem>
+static void
 file_write (FILE *file, const std::vector<Elem> &vec)
 {
   file_write (file, vec.data (), vec.size() * sizeof (vec[0]));
 }
 
-// In-memory buffer to prepare data to be written later to a file.
-class DataBuf
+/* In-memory buffer to prepare data to be written later to a file.  */
+class data_buf
 {
-private:
-  std::vector<gdb_byte> vec;
 public:
-
-  // Append space of SIZE number of bytes to the end of buffer.
-  // Return pointer to its start.
-
-  gdb_byte *
-  append_space (size_t size)
+  /* Add SIZE bytes at the end of the buffer.  Returns a pointer to
+     the start of the new block.  */
+  gdb_byte *append_space (size_t size)
   {
-    vec.resize (vec.size () + size);
-    return &*vec.end () - size;
+    m_vec.resize (m_vec.size () + size);
+    return &*m_vec.end () - size;
   }
 
-  // Copy DATA to the end of buffer.
-
-  template<typename T> void
-  append_data (const T &data)
+  /* Copy DATA to the end of the buffer.  */
+  template<typename T>
+  void append_data (const T &data)
   {
     std::copy (reinterpret_cast<const gdb_byte *> (&data),
 	       reinterpret_cast<const gdb_byte *> (&data + 1),
 	       append_space (sizeof (data)));
   }
 
-  // Copy CSTR zero-terminated string to the end of buffer including its
-  // terminating zero.
-
-  void
-  append_cstr0 (const char *cstr)
+  /* Copy CSTR (a null-terminated string) to the end of the buffer.
+     The terminating null is appended too.  */
+  void append_cstr0 (const char *cstr)
   {
-    const size_t size (strlen (cstr) + 1);
+    const size_t size = strlen (cstr) + 1;
     std::copy (cstr, cstr + size, append_space (size));
   }
 
-  // Return size of the buffer.
-
-  size_t
-  size () const
+  /* Return the size of the buffer.  */
+  size_t size () const
   {
-    return vec.size ();
+    return m_vec.size ();
   }
 
   /* Write the buffer to FILE.  */
-
-  void
-  file_write (FILE *file) const
+  void file_write (FILE *file) const
   {
-    ::file_write (file, vec);
+    ::file_write (file, m_vec);
   }
+
+private:
+  std::vector<gdb_byte> m_vec;
 };
 
 /* An entry in the symbol table.  */
@@ -23280,13 +23274,13 @@  struct symtab_index_entry
 /* The symbol table.  This is a power-of-2-sized hash table.  */
 struct mapped_symtab
 {
-public:
-  offset_type n_elements = 0;
-  std::vector<std::unique_ptr<symtab_index_entry>> data;
   mapped_symtab ()
   {
     data.resize (1024);
   }
+
+  offset_type n_elements = 0;
+  std::vector<std::unique_ptr<symtab_index_entry>> data;
 };
 
 /* Find a slot in SYMTAB for the symbol NAME.  Returns a reference to
@@ -23316,15 +23310,15 @@  find_slot (struct mapped_symtab *symtab, const char *name)
 static void
 hash_expand (struct mapped_symtab *symtab)
 {
-  auto old_entries (std::move (symtab->data));
+  auto old_entries = std::move (symtab->data);
 
   symtab->data.clear ();
   symtab->data.resize (old_entries.size () * 2);
 
-  for (auto &it:old_entries)
-    if (it)
+  for (auto &it : old_entries)
+    if (it != NULL)
       {
-	auto &ref (find_slot (symtab, it->name));
+	auto &ref = find_slot (symtab, it->name);
 	ref = std::move (it);
       }
 }
@@ -23344,10 +23338,10 @@  add_index_entry (struct mapped_symtab *symtab, const char *name,
   if (4 * symtab->n_elements / 3 >= symtab->data.size ())
     hash_expand (symtab);
 
-  std::unique_ptr<symtab_index_entry> &slot (find_slot (symtab, name));
-  if (!slot)
+  std::unique_ptr<symtab_index_entry> &slot = find_slot (symtab, name);
+  if (slot == NULL)
     {
-      slot = std::make_unique<symtab_index_entry> ();
+      slot.reset (new symtab_index_entry ());
       slot->name = name;
       /* index_offset is set later.  */
     }
@@ -23372,7 +23366,7 @@  add_index_entry (struct mapped_symtab *symtab, const char *name,
 static void
 uniquify_cu_indices (struct mapped_symtab *symtab)
 {
-  for (const auto &entry:symtab->data)
+  for (const auto &entry : symtab->data)
     {
       if (entry && !entry->cu_indices.empty ())
 	{
@@ -23397,87 +23391,89 @@  uniquify_cu_indices (struct mapped_symtab *symtab)
     }
 }
 
-// Provide form of const char * suitable for container keys.
-// Only the pointer is being stored.
-// Comparison is done for the strings themselves - not for the pointer.
-class CstrView {
-private:
-  friend class CstrViewHasher;
-  const char *const cstr;
+/* A form of 'const char *' suitable for container keys.  Only the
+   pointer is stored.  The strings themselves are compared, not the
+   pointers.  */
+class c_str_view
+{
 public:
-  CstrView (const char *cstr_) : cstr (cstr_)
-  {
-  }
-  bool
-  operator == (const CstrView &other) const
+  c_str_view (const char *cstr)
+    : m_cstr (cstr)
+  {}
+
+  bool operator== (const c_str_view &other) const
   {
-    return !strcmp (cstr, other.cstr);
+    return strcmp (m_cstr, other.m_cstr) == 0;
   }
+
+private:
+  friend class c_str_view_hasher;
+  const char *const m_cstr;
 };
 
-// Provide std::unordered_map::hasher for CstrView.
-class CstrViewHasher
+/* A std::unordered_map::hasher for c_str_view that uses the right
+   hash function for strings in a mapped index.  */
+class c_str_view_hasher
 {
 public:
-  size_t
-  operator () (const CstrView &x) const
+  size_t operator () (const c_str_view &x) const
   {
-    return mapped_index_string_hash (INT_MAX, x.cstr);
+    return mapped_index_string_hash (INT_MAX, x.m_cstr);
   }
 };
 
-// Provide std::unordered_map::hasher for std::vector<>.
-template<class T> class VectorHasher
+/* A std::unordered_map::hasher for std::vector<>.  */
+template<class T>
+class vector_hasher
 {
 public:
-  size_t
-  operator () (const std::vector<T> &key) const
+  size_t operator () (const std::vector<T> &key) const
   {
-    // return boost::hash_value<std::vector<T>> (key);
     return iterative_hash (key.data (),
 			   sizeof (key.front ()) * key.size (), 0);
   }
 };
 
-/* Write the mapped hash table SYMTAB to the obstack OUTPUT, with
-   constant pool entries going into the obstack CPOOL.  */
+/* Write the mapped hash table SYMTAB to the data buffer OUTPUT, with
+   constant pool entries going into the data buffer CPOOL.  */
 
 static void
-write_hash_table (struct mapped_symtab *symtab, DataBuf &output, DataBuf &cpool)
+write_hash_table (mapped_symtab *symtab, data_buf &output, data_buf &cpool)
 {
   {
-    /* Elements are sorted vectors of the indices of all the CUs that hold
-       an object of this name.  */
+    /* Elements are sorted vectors of the indices of all the CUs that
+       hold an object of this name.  */
     std::unordered_map<std::vector<offset_type>, offset_type,
-		       VectorHasher<offset_type>> symbol_hash_table;
+		       vector_hasher<offset_type>>
+      symbol_hash_table;
 
     /* We add all the index vectors to the constant pool first, to
        ensure alignment is ok.  */
-    for (const std::unique_ptr<symtab_index_entry> &it:symtab->data)
+    for (const std::unique_ptr<symtab_index_entry> &it : symtab->data)
       {
-	if (!it)
+	if (it == NULL)
 	  continue;
 	gdb_assert (it->index_offset == 0);
-	const auto insertpair (symbol_hash_table.emplace (it->cu_indices,
-							  cpool.size ()));
+	const auto insertpair
+	  = symbol_hash_table.emplace (it->cu_indices, cpool.size ());
 	it->index_offset = insertpair.first->second;
 	if (!insertpair.second)
 	  continue;
 	cpool.append_data (MAYBE_SWAP (it->cu_indices.size ()));
-	for (const auto iter:it->cu_indices)
+	for (const auto iter : it->cu_indices)
 	  cpool.append_data (MAYBE_SWAP (iter));
       }
   }
 
   /* Now write out the hash table.  */
-  std::unordered_map<CstrView, offset_type, CstrViewHasher> str_table;
-  for (const auto &it:symtab->data)
+  std::unordered_map<c_str_view, offset_type, c_str_view_hasher> str_table;
+  for (const auto &it : symtab->data)
     {
       offset_type str_off, vec_off;
 
-      if (it)
+      if (it != NULL)
 	{
-	  const auto insertpair (str_table.emplace (it->name, cpool.size ()));
+	  const auto insertpair = str_table.emplace (it->name, cpool.size ());
 	  if (insertpair.second)
 	    cpool.append_cstr0 (it->name);
 	  str_off = insertpair.first->second;
@@ -23496,12 +23492,18 @@  write_hash_table (struct mapped_symtab *symtab, DataBuf &output, DataBuf &cpool)
     }
 }
 
+typedef std::unordered_map<partial_symtab *, unsigned int> psym_index_map;
+
 /* Helper struct for building the address table.  */
 struct addrmap_index_data
 {
+  addrmap_index_data (data_buf &addr_vec_, psym_index_map &cu_index_htab_)
+    : addr_vec (addr_vec_), cu_index_htab (cu_index_htab_)
+  {}
+
   struct objfile *objfile;
-  DataBuf &addr_vec;
-  std::unordered_map<struct partial_symtab *, unsigned int> &cu_index_htab;
+  data_buf &addr_vec;
+  psym_index_map &cu_index_htab;
 
   /* Non-zero if the previous_* fields are valid.
      We can't write an entry until we see the next entry (since it is only then
@@ -23511,17 +23513,12 @@  struct addrmap_index_data
   unsigned int previous_cu_index;
   /* Start address of the CU.  */
   CORE_ADDR previous_cu_start;
-
-  addrmap_index_data (DataBuf &addr_vec_,
-      std::unordered_map<struct partial_symtab *, unsigned int> &cu_index_htab_)
-  : addr_vec (addr_vec_), cu_index_htab (cu_index_htab_)
-  {}
 };
 
-/* Write an address entry to OBSTACK.  */
+/* Write an address entry to ADDR_VEC.  */
 
 static void
-add_address_entry (struct objfile *objfile, DataBuf &addr_vec,
+add_address_entry (struct objfile *objfile, data_buf &addr_vec,
 		   CORE_ADDR start, CORE_ADDR end, unsigned int cu_index)
 {
   CORE_ADDR baseaddr;
@@ -23551,24 +23548,24 @@  add_address_entry_worker (void *datap, CORE_ADDR start_addr, void *obj)
   data->previous_cu_start = start_addr;
   if (pst != NULL)
     {
-      const auto it (data->cu_index_htab.find (pst));
+      const auto it = data->cu_index_htab.find (pst);
       gdb_assert (it != data->cu_index_htab.cend ());
       data->previous_cu_index = it->second;
       data->previous_valid = 1;
     }
   else
-      data->previous_valid = 0;
+    data->previous_valid = 0;
 
   return 0;
 }
 
-/* Write OBJFILE's address map to OBSTACK.
+/* Write OBJFILE's address map to ADDR_VEC.
    CU_INDEX_HTAB is used to map addrmap entries to their CU indices
    in the index file.  */
 
 static void
-write_address_map (struct objfile *objfile, DataBuf &addr_vec,
-       std::unordered_map<struct partial_symtab *, unsigned int> &cu_index_htab)
+write_address_map (struct objfile *objfile, data_buf &addr_vec,
+		   psym_index_map &cu_index_htab)
 {
   struct addrmap_index_data addrmap_index_data (addr_vec, cu_index_htab);
 
@@ -23662,16 +23659,16 @@  write_psymbols (struct mapped_symtab *symtab,
 /* A helper struct used when iterating over debug_types.  */
 struct signatured_type_index_data
 {
+  signatured_type_index_data (data_buf &types_list_,
+                              std::unordered_set<partial_symbol *> &psyms_seen_)
+    :types_list (types_list_), psyms_seen (psyms_seen_)
+  {}
+
   struct objfile *objfile;
   struct mapped_symtab *symtab;
-  DataBuf &types_list;
+  data_buf &types_list;
   std::unordered_set<partial_symbol *> &psyms_seen;
   int cu_index;
-
-  signatured_type_index_data (DataBuf &types_list_,
-                              std::unordered_set<partial_symbol *> &psyms_seen_)
-  :types_list (types_list_), psyms_seen (psyms_seen_)
-  {}
 };
 
 /* A helper function that writes a single signatured_type to an
@@ -23741,6 +23738,20 @@  recursively_write_psymbols (struct objfile *objfile,
 		  1);
 }
 
+/* Closes FILE on scope exit.  */
+struct file_closer
+{
+  explicit file_closer (FILE *file)
+    : m_file (file)
+  {}
+
+  ~file_closer ()
+  { fclose (m_file); }
+
+private:
+  FILE *m_file;
+};
+
 /* Create an index file for OBJFILE in the directory DIR.  */
 
 static void
@@ -23762,121 +23773,116 @@  write_psymtabs_to_index (struct objfile *objfile, const char *dir)
   std::string filename (std::string (dir) + SLASH_STRING
 			+ lbasename (objfile_name (objfile)) + INDEX_SUFFIX);
 
-  FILE *out_file (gdb_fopen_cloexec (filename.c_str (), "wb"));
+  FILE *out_file = gdb_fopen_cloexec (filename.c_str (), "wb");
   if (!out_file)
     error (_("Can't open `%s' for writing"), filename.c_str ());
 
-  TRY
-    {
-      mapped_symtab symtab;
-      DataBuf cu_list;
-      std::unordered_set<partial_symbol *> psyms_seen;
-
-      /* While we're scanning CU's create a table that maps a psymtab pointer
-	 (which is what addrmap records) to its index (which is what is recorded
-	 in the index file).  This will later be needed to write the address
-	 table.  */
-      std::unordered_map<struct partial_symtab *, unsigned int> cu_index_htab;
-      cu_index_htab.reserve (dwarf2_per_objfile->n_comp_units);
-
-      /* The CU list is already sorted, so we don't need to do additional
-	 work here.  Also, the debug_types entries do not appear in
-	 all_comp_units, but only in their own hash table.  */
-      for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
-	{
-	  struct dwarf2_per_cu_data *per_cu
-	    = dwarf2_per_objfile->all_comp_units[i];
-	  struct partial_symtab *psymtab = per_cu->v.psymtab;
-
-	  /* CU of a shared file from 'dwz -m' may be unused by this main file.
-	     It may be referenced from a local scope but in such case it does
-	     not need to be present in .gdb_index.  */
-	  if (psymtab == NULL)
-	    continue;
+  file_closer close_out_file (out_file);
+  gdb::unlinker unlink_file (filename.c_str ());
+
+  mapped_symtab symtab;
+  data_buf cu_list;
+  std::unordered_set<partial_symbol *> psyms_seen;
+
+  /* While we're scanning CU's create a table that maps a psymtab pointer
+     (which is what addrmap records) to its index (which is what is recorded
+     in the index file).  This will later be needed to write the address
+     table.  */
+  psym_index_map cu_index_htab;
+  cu_index_htab.reserve (dwarf2_per_objfile->n_comp_units);
+
+  /* The CU list is already sorted, so we don't need to do additional
+     work here.  Also, the debug_types entries do not appear in
+     all_comp_units, but only in their own hash table.  */
+  for (int i = 0; i < dwarf2_per_objfile->n_comp_units; ++i)
+    {
+      struct dwarf2_per_cu_data *per_cu
+	= dwarf2_per_objfile->all_comp_units[i];
+      struct partial_symtab *psymtab = per_cu->v.psymtab;
+
+      /* CU of a shared file from 'dwz -m' may be unused by this main file.
+	 It may be referenced from a local scope but in such case it does not
+	 need to be present in .gdb_index.  */
+      if (psymtab == NULL)
+	continue;
 
-	  if (psymtab->user == NULL)
-	    recursively_write_psymbols (objfile, psymtab, &symtab, psyms_seen,
-					i);
+      if (psymtab->user == NULL)
+	recursively_write_psymbols (objfile, psymtab, &symtab,
+				    psyms_seen, i);
 
-	  const auto insertpair (cu_index_htab.emplace (psymtab, i));
-	  gdb_assert (insertpair.second);
+      const auto insertpair = cu_index_htab.emplace (psymtab, i);
+      gdb_assert (insertpair.second);
 
-	  store_unsigned_integer (cu_list.append_space (8), 8,
-				  BFD_ENDIAN_LITTLE,
-				  to_underlying (per_cu->sect_off));
-	  store_unsigned_integer (cu_list.append_space (8), 8,
-				  BFD_ENDIAN_LITTLE, per_cu->length);
-	}
+      store_unsigned_integer (cu_list.append_space (8), 8,
+			      BFD_ENDIAN_LITTLE,
+			      to_underlying (per_cu->sect_off));
+      store_unsigned_integer (cu_list.append_space (8), 8,
+			      BFD_ENDIAN_LITTLE, per_cu->length);
+    }
 
-      /* Dump the address map.  */
-      DataBuf addr_vec;
-      write_address_map (objfile, addr_vec, cu_index_htab);
+  /* Dump the address map.  */
+  data_buf addr_vec;
+  write_address_map (objfile, addr_vec, cu_index_htab);
 
-      /* Write out the .debug_type entries, if any.  */
-      DataBuf types_cu_list;
-      if (dwarf2_per_objfile->signatured_types)
-	{
-	  struct signatured_type_index_data sig_data (types_cu_list,
-						      psyms_seen);
-
-	  sig_data.objfile = objfile;
-	  sig_data.symtab = &symtab;
-	  sig_data.cu_index = dwarf2_per_objfile->n_comp_units;
-	  htab_traverse_noresize (dwarf2_per_objfile->signatured_types,
-				  write_one_signatured_type, &sig_data);
-	}
+  /* Write out the .debug_type entries, if any.  */
+  data_buf types_cu_list;
+  if (dwarf2_per_objfile->signatured_types)
+    {
+      signatured_type_index_data sig_data (types_cu_list,
+					   psyms_seen);
 
-      /* Now that we've processed all symbols we can shrink their cu_indices
-	 lists.  */
-      uniquify_cu_indices (&symtab);
+      sig_data.objfile = objfile;
+      sig_data.symtab = &symtab;
+      sig_data.cu_index = dwarf2_per_objfile->n_comp_units;
+      htab_traverse_noresize (dwarf2_per_objfile->signatured_types,
+			      write_one_signatured_type, &sig_data);
+    }
 
-      DataBuf symtab_vec, constant_pool;
-      write_hash_table (&symtab, symtab_vec, constant_pool);
+  /* Now that we've processed all symbols we can shrink their cu_indices
+     lists.  */
+  uniquify_cu_indices (&symtab);
 
-      DataBuf contents;
-      const offset_type size_of_contents (6 * sizeof (offset_type));
-      offset_type total_len (size_of_contents);
+  data_buf symtab_vec, constant_pool;
+  write_hash_table (&symtab, symtab_vec, constant_pool);
 
-      /* The version number.  */
-      contents.append_data (MAYBE_SWAP (8));
+  data_buf contents;
+  const offset_type size_of_contents = 6 * sizeof (offset_type);
+  offset_type total_len = size_of_contents;
 
-      /* The offset of the CU list from the start of the file.  */
-      contents.append_data (MAYBE_SWAP (total_len));
-      total_len += cu_list.size ();
+  /* The version number.  */
+  contents.append_data (MAYBE_SWAP (8));
 
-      /* The offset of the types CU list from the start of the file.  */
-      contents.append_data (MAYBE_SWAP (total_len));
-      total_len += types_cu_list.size ();
+  /* The offset of the CU list from the start of the file.  */
+  contents.append_data (MAYBE_SWAP (total_len));
+  total_len += cu_list.size ();
 
-      /* The offset of the address table from the start of the file.  */
-      contents.append_data (MAYBE_SWAP (total_len));
-      total_len += addr_vec.size ();
+  /* The offset of the types CU list from the start of the file.  */
+  contents.append_data (MAYBE_SWAP (total_len));
+  total_len += types_cu_list.size ();
 
-      /* The offset of the symbol table from the start of the file.  */
-      contents.append_data (MAYBE_SWAP (total_len));
-      total_len += symtab_vec.size ();
+  /* The offset of the address table from the start of the file.  */
+  contents.append_data (MAYBE_SWAP (total_len));
+  total_len += addr_vec.size ();
 
-      /* The offset of the constant pool from the start of the file.  */
-      contents.append_data (MAYBE_SWAP (total_len));
-      total_len += constant_pool.size ();
+  /* The offset of the symbol table from the start of the file.  */
+  contents.append_data (MAYBE_SWAP (total_len));
+  total_len += symtab_vec.size ();
 
-      gdb_assert (contents.size () == size_of_contents);
+  /* The offset of the constant pool from the start of the file.  */
+  contents.append_data (MAYBE_SWAP (total_len));
+  total_len += constant_pool.size ();
 
-      contents.file_write (out_file);
-      cu_list.file_write (out_file);
-      types_cu_list.file_write (out_file);
-      addr_vec.file_write (out_file);
-      symtab_vec.file_write (out_file);
-      constant_pool.file_write (out_file);
-    }
-  CATCH (except, RETURN_MASK_ALL)
-    {
-      fclose (out_file);
-      unlink (filename.c_str ());
-      throw_exception (except);
-    }
-  END_CATCH
-  fclose (out_file);
+  gdb_assert (contents.size () == size_of_contents);
+
+  contents.file_write (out_file);
+  cu_list.file_write (out_file);
+  types_cu_list.file_write (out_file);
+  addr_vec.file_write (out_file);
+  symtab_vec.file_write (out_file);
+  constant_pool.file_write (out_file);
+
+  /* We want to keep the file.  */
+  unlink_file.keep ();
 }
 
 /* Implementation of the `save gdb-index' command.