[RFA 00/13] more cleanup removal

[RFA 00/13] more cleanup removal

[Tom Tromey]

This series removes some more cleanups.  Most of the patches are
straightforward.

Tested by the buildbot.  However, the buildbot doesn't really test
some of these changes (e.g, h8300-tdep.c); so be sure to examine these
more carefully.

Tom

[RFA 11/13] Use unique_xmalloc_ptr in c_type_print_base

[Tom Tromey]

This changes c_type_print_base to use unique_xmalloc_ptr, removing a
cleanup.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* c-typeprint.c (c_type_print_base): Use gdb::unique_xmalloc_ptr.
---
 gdb/ChangeLog     |  4 ++++
 gdb/c-typeprint.c | 13 +++----------
 2 files changed, 7 insertions(+), 10 deletions(-)

diff --git a/gdb/c-typeprint.c b/gdb/c-typeprint.c
index 22fdaa5beb..ed5a1a4b8a 100644
--- a/gdb/c-typeprint.c
+++ b/gdb/c-typeprint.c
@@ -1218,8 +1218,8 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 	      for (j = 0; j < len2; j++)
 		{
 		  const char *mangled_name;
+		  gdb::unique_xmalloc_ptr<char> mangled_name_holder;
 		  char *demangled_name;
-		  struct cleanup *inner_cleanup;
 		  const char *physname = TYPE_FN_FIELD_PHYSNAME (f, j);
 		  int is_full_physname_constructor =
 		    TYPE_FN_FIELD_CONSTRUCTOR (f, j)
@@ -1231,8 +1231,6 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 		  if (TYPE_FN_FIELD_ARTIFICIAL (f, j))
 		    continue;
 
-		  inner_cleanup = make_cleanup (null_cleanup, NULL);
-
 		  QUIT;
 		  section_type = output_access_specifier
 		    (stream, section_type, level,
@@ -1265,12 +1263,9 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 		    }
 		  if (TYPE_FN_FIELD_STUB (f, j))
 		    {
-		      char *tem;
-
 		      /* Build something we can demangle.  */
-		      tem = gdb_mangle_name (type, i, j);
-		      make_cleanup (xfree, tem);
-		      mangled_name = tem;
+		      mangled_name_holder.reset (gdb_mangle_name (type, i, j));
+		      mangled_name = mangled_name_holder.get ();
 		    }
 		  else
 		    mangled_name = TYPE_FN_FIELD_PHYSNAME (f, j);
@@ -1328,8 +1323,6 @@ c_type_print_base (struct type *type, struct ui_file *stream,
 		      xfree (demangled_name);
 		    }
 
-		  do_cleanups (inner_cleanup);
-
 		  fprintf_filtered (stream, ";\n");
 		}
 	    }
-- 
2.13.6

[RFA 02/13] Remove cleanups from link_callbacks_einfo

[Tom Tromey]

This removes a cleanup from link_callbacks_einfo by using std::string.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* compile/compile-object-load.c (link_callbacks_einfo): Use
	std::string.
---
 gdb/ChangeLog                     | 5 +++++
 gdb/compile/compile-object-load.c | 9 ++-------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 41d5fc347f..8e55d3c2c9 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -281,18 +281,13 @@ static void link_callbacks_einfo (const char *fmt, ...)
 static void
 link_callbacks_einfo (const char *fmt, ...)
 {
-  struct cleanup *cleanups;
   va_list ap;
-  char *str;
 
   va_start (ap, fmt);
-  str = xstrvprintf (fmt, ap);
+  std::string str = string_vprintf (fmt, ap);
   va_end (ap);
-  cleanups = make_cleanup (xfree, str);
-
-  warning (_("Compile module: warning: %s"), str);
 
-  do_cleanups (cleanups);
+  warning (_("Compile module: warning: %s"), str.c_str ());
 }
 
 /* Helper for bfd_get_relocated_section_contents.
-- 
2.13.6

[RFA 08/13] Remove make_cleanup_free_objfile

[Tom Tromey]

This replaces make_cleanup_free_objfile with std::unique_ptr.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* objfiles.c (do_free_objfile_cleanup): Remove.
	* compile/compile-object-load.c (compile_object_load): Update.
	* objfiles.h (make_cleanup_free_objfile): Remove.
---
 gdb/ChangeLog                     |  6 ++++++
 gdb/compile/compile-object-load.c | 13 ++++++-------
 gdb/objfiles.c                    | 12 ------------
 gdb/objfiles.h                    |  2 --
 gdb/symfile.c                     | 10 +++++++---
 5 files changed, 19 insertions(+), 24 deletions(-)

diff --git a/gdb/compile/compile-object-load.c b/gdb/compile/compile-object-load.c
index 8e55d3c2c9..e69fbc6c9c 100644
--- a/gdb/compile/compile-object-load.c
+++ b/gdb/compile/compile-object-load.c
@@ -606,7 +606,7 @@ struct compile_module *
 compile_object_load (const compile_file_names &file_names,
 		     enum compile_i_scope_types scope, void *scope_data)
 {
-  struct cleanup *cleanups, *cleanups_free_objfile;
+  struct cleanup *cleanups;
   struct setup_sections_data setup_sections_data;
   CORE_ADDR addr, regs_addr, out_value_addr = 0;
   struct symbol *func_sym;
@@ -656,9 +656,10 @@ compile_object_load (const compile_file_names &file_names,
 
   /* SYMFILE_VERBOSE is not passed even if FROM_TTY, user is not interested in
      "Reading symbols from ..." message for automatically generated file.  */
-  objfile = symbol_file_add_from_bfd (abfd.get (), filename.get (),
-				      0, NULL, 0, NULL);
-  cleanups_free_objfile = make_cleanup_free_objfile (objfile);
+  std::unique_ptr<struct objfile> objfile_holder
+    (symbol_file_add_from_bfd (abfd.get (), filename.get (),
+			       0, NULL, 0, NULL));
+  objfile = objfile_holder.get ();
 
   func_sym = lookup_global_symbol_from_objfile (objfile,
 						GCC_FE_WRAPPER_FUNCTION,
@@ -812,10 +813,8 @@ compile_object_load (const compile_file_names &file_names,
 			    paddress (target_gdbarch (), out_value_addr));
     }
 
-  discard_cleanups (cleanups_free_objfile);
-
   retval = XNEW (struct compile_module);
-  retval->objfile = objfile;
+  retval->objfile = objfile_holder.release ();
   retval->source_file = xstrdup (file_names.source_file ());
   retval->func_sym = func_sym;
   retval->regs_addr = regs_addr;
diff --git a/gdb/objfiles.c b/gdb/objfiles.c
index d8fe88b136..edde399802 100644
--- a/gdb/objfiles.c
+++ b/gdb/objfiles.c
@@ -729,18 +729,6 @@ objfile::~objfile ()
     htab_delete (static_links);
 }
 
-static void
-do_free_objfile_cleanup (void *obj)
-{
-  delete (struct objfile *) obj;
-}
-
-struct cleanup *
-make_cleanup_free_objfile (struct objfile *obj)
-{
-  return make_cleanup (do_free_objfile_cleanup, obj);
-}
-
 /* Free all the object files at once and clean up their users.  */
 
 void
diff --git a/gdb/objfiles.h b/gdb/objfiles.h
index 453166a001..4f11756248 100644
--- a/gdb/objfiles.h
+++ b/gdb/objfiles.h
@@ -479,8 +479,6 @@ extern void unlink_objfile (struct objfile *);
 
 extern void free_objfile_separate_debug (struct objfile *);
 
-extern struct cleanup *make_cleanup_free_objfile (struct objfile *);
-
 extern void free_all_objfiles (void);
 
 extern void objfile_relocate (struct objfile *, const struct section_offsets *);
diff --git a/gdb/symfile.c b/gdb/symfile.c
index fb63441ac0..4077c2e31c 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -997,7 +997,8 @@ syms_from_objfile_1 (struct objfile *objfile,
 
   /* Make sure that partially constructed symbol tables will be cleaned up
      if an error occurs during symbol reading.  */
-  old_chain = make_cleanup_free_objfile (objfile);
+  old_chain = make_cleanup (null_cleanup, NULL);
+  std::unique_ptr<struct objfile> objfile_holder (objfile);
 
   /* If ADDRS is NULL, put together a dummy address list.
      We now establish the convention that an addr of zero means
@@ -1053,6 +1054,7 @@ syms_from_objfile_1 (struct objfile *objfile,
 
   /* Discard cleanups as symbol reading was successful.  */
 
+  objfile_holder.release ();
   discard_cleanups (old_chain);
   xfree (local_addr);
 }
@@ -2436,9 +2438,10 @@ reread_symbols (void)
 	  /* If we get an error, blow away this objfile (not sure if
 	     that is the correct response for things like shared
 	     libraries).  */
-	  old_cleanups = make_cleanup_free_objfile (objfile);
+	  std::unique_ptr<struct objfile> objfile_holder (objfile);
+
 	  /* We need to do this whenever any symbols go away.  */
-	  make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
+	  old_cleanups = make_cleanup (clear_symtab_users_cleanup, 0 /*ignore*/);
 
 	  if (exec_bfd != NULL
 	      && filename_cmp (bfd_get_filename (objfile->obfd),
@@ -2600,6 +2603,7 @@ reread_symbols (void)
 	  reinit_frame_cache ();
 
 	  /* Discard cleanups as symbol reading was successful.  */
+	  objfile_holder.release ();
 	  discard_cleanups (old_cleanups);
 
 	  /* If the mtime has changed between the time we set new_modtime
-- 
2.13.6

[RFA 10/13] Remove cleanups from linux-tdep.c

[Tom Tromey]

This removes some cleanups from linux-tdep.c, replacing them with
def_vector or unique_xmalloc_ptr as appropriate.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* linux-tdep.c (linux_core_info_proc_mappings): Use
	gdb::def_vector.
	(linux_get_siginfo_data): Return gdb::unique_xmalloc_ptr.
	(linux_corefile_thread): Update.
---
 gdb/ChangeLog    |  7 +++++++
 gdb/linux-tdep.c | 45 ++++++++++++++-------------------------------
 2 files changed, 21 insertions(+), 31 deletions(-)

diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
index 83ff59faee..0350ccea16 100644
--- a/gdb/linux-tdep.c
+++ b/gdb/linux-tdep.c
@@ -998,10 +998,9 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
 {
   asection *section;
   ULONGEST count, page_size;
-  unsigned char *descdata, *filenames, *descend, *contents;
+  unsigned char *descdata, *filenames, *descend;
   size_t note_size;
   unsigned int addr_size_bits, addr_size;
-  struct cleanup *cleanup;
   struct gdbarch *core_gdbarch = gdbarch_from_bfd (core_bfd);
   /* We assume this for reading 64-bit core files.  */
   gdb_static_assert (sizeof (ULONGEST) >= 8);
@@ -1020,12 +1019,12 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
   if (note_size < 2 * addr_size)
     error (_("malformed core note - too short for header"));
 
-  contents = (unsigned char *) xmalloc (note_size);
-  cleanup = make_cleanup (xfree, contents);
-  if (!bfd_get_section_contents (core_bfd, section, contents, 0, note_size))
+  gdb::def_vector<unsigned char> contents (note_size);
+  if (!bfd_get_section_contents (core_bfd, section, contents.data (),
+				 0, note_size))
     error (_("could not get core note contents"));
 
-  descdata = contents;
+  descdata = contents.data ();
   descend = descdata + note_size;
 
   if (descdata[note_size - 1] != '\0')
@@ -1090,8 +1089,6 @@ linux_core_info_proc_mappings (struct gdbarch *gdbarch, const char *args)
 
       filenames += 1 + strlen ((char *) filenames);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Implement "info proc" for a corefile.  */
@@ -1516,7 +1513,6 @@ static char *
 linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd *obfd,
 				    char *note_data, int *note_size)
 {
-  struct cleanup *cleanup;
   struct linux_make_mappings_data mapping_data;
   struct type *long_type
     = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch), 0, "long");
@@ -1646,14 +1642,12 @@ linux_collect_thread_registers (const struct regcache *regcache,
    with the size of the data.  The caller is responsible for freeing
    the data.  */
 
-static gdb_byte *
+static gdb::unique_xmalloc_ptr<gdb_byte>
 linux_get_siginfo_data (thread_info *thread, struct gdbarch *gdbarch,
 			LONGEST *size)
 {
   struct type *siginfo_type;
-  gdb_byte *buf;
   LONGEST bytes_read;
-  struct cleanup *cleanups;
 
   if (!gdbarch_get_siginfo_type_p (gdbarch))
     return NULL;
@@ -1663,21 +1657,15 @@ linux_get_siginfo_data (thread_info *thread, struct gdbarch *gdbarch,
 
   siginfo_type = gdbarch_get_siginfo_type (gdbarch);
 
-  buf = (gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type));
-  cleanups = make_cleanup (xfree, buf);
+  gdb::unique_xmalloc_ptr<gdb_byte> buf
+    ((gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type)));
 
   bytes_read = target_read (&current_target, TARGET_OBJECT_SIGNAL_INFO, NULL,
-			    buf, 0, TYPE_LENGTH (siginfo_type));
+			    buf.get (), 0, TYPE_LENGTH (siginfo_type));
   if (bytes_read == TYPE_LENGTH (siginfo_type))
-    {
-      discard_cleanups (cleanups);
-      *size = bytes_read;
-    }
+    *size = bytes_read;
   else
-    {
-      do_cleanups (cleanups);
-      buf = NULL;
-    }
+    return NULL;
 
   return buf;
 }
@@ -1698,17 +1686,14 @@ static void
 linux_corefile_thread (struct thread_info *info,
 		       struct linux_corefile_thread_data *args)
 {
-  struct cleanup *old_chain;
   struct regcache *regcache;
-  gdb_byte *siginfo_data;
   LONGEST siginfo_size = 0;
 
   regcache = get_thread_arch_regcache (info->ptid, args->gdbarch);
 
   target_fetch_registers (regcache, -1);
-  siginfo_data = linux_get_siginfo_data (info, args->gdbarch, &siginfo_size);
-
-  old_chain = make_cleanup (xfree, siginfo_data);
+  gdb::unique_xmalloc_ptr<gdb_byte> siginfo_data
+    = linux_get_siginfo_data (info, args->gdbarch, &siginfo_size);
 
   args->note_data = linux_collect_thread_registers
     (regcache, info->ptid, args->obfd, args->note_data,
@@ -1722,9 +1707,7 @@ linux_corefile_thread (struct thread_info *info,
 					    args->note_data,
 					    args->note_size,
 					    "CORE", NT_SIGINFO,
-					    siginfo_data, siginfo_size);
-
-  do_cleanups (old_chain);
+					    siginfo_data.get (), siginfo_size);
 }
 
 /* Fill the PRPSINFO structure with information about the process being
-- 
2.13.6

Re: [RFA 10/13] Remove cleanups from linux-tdep.c

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> This removes some cleanups from linux-tdep.c, replacing them with
> def_vector or unique_xmalloc_ptr as appropriate.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* linux-tdep.c (linux_core_info_proc_mappings): Use
> 	gdb::def_vector.
> 	(linux_get_siginfo_data): Return gdb::unique_xmalloc_ptr.
> 	(linux_corefile_thread): Update.

Just to be pedantic, you should mention the change to 
linux_make_mappings_corefile_notes.

> ---
>  gdb/ChangeLog    |  7 +++++++
>  gdb/linux-tdep.c | 45 ++++++++++++++-------------------------------
>  2 files changed, 21 insertions(+), 31 deletions(-)
> 
> diff --git a/gdb/linux-tdep.c b/gdb/linux-tdep.c
> index 83ff59faee..0350ccea16 100644
> --- a/gdb/linux-tdep.c
> +++ b/gdb/linux-tdep.c
> @@ -998,10 +998,9 @@ linux_core_info_proc_mappings (struct gdbarch
> *gdbarch, const char *args)
>  {
>    asection *section;
>    ULONGEST count, page_size;
> -  unsigned char *descdata, *filenames, *descend, *contents;
> +  unsigned char *descdata, *filenames, *descend;
>    size_t note_size;
>    unsigned int addr_size_bits, addr_size;
> -  struct cleanup *cleanup;
>    struct gdbarch *core_gdbarch = gdbarch_from_bfd (core_bfd);
>    /* We assume this for reading 64-bit core files.  */
>    gdb_static_assert (sizeof (ULONGEST) >= 8);
> @@ -1020,12 +1019,12 @@ linux_core_info_proc_mappings (struct gdbarch
> *gdbarch, const char *args)
>    if (note_size < 2 * addr_size)
>      error (_("malformed core note - too short for header"));
> 
> -  contents = (unsigned char *) xmalloc (note_size);
> -  cleanup = make_cleanup (xfree, contents);
> -  if (!bfd_get_section_contents (core_bfd, section, contents, 0, 
> note_size))
> +  gdb::def_vector<unsigned char> contents (note_size);
> +  if (!bfd_get_section_contents (core_bfd, section, contents.data (),
> +				 0, note_size))
>      error (_("could not get core note contents"));
> 
> -  descdata = contents;
> +  descdata = contents.data ();
>    descend = descdata + note_size;
> 
>    if (descdata[note_size - 1] != '\0')
> @@ -1090,8 +1089,6 @@ linux_core_info_proc_mappings (struct gdbarch
> *gdbarch, const char *args)
> 
>        filenames += 1 + strlen ((char *) filenames);
>      }
> -
> -  do_cleanups (cleanup);
>  }
> 
>  /* Implement "info proc" for a corefile.  */
> @@ -1516,7 +1513,6 @@ static char *
>  linux_make_mappings_corefile_notes (struct gdbarch *gdbarch, bfd 
> *obfd,
>  				    char *note_data, int *note_size)
>  {
> -  struct cleanup *cleanup;
>    struct linux_make_mappings_data mapping_data;
>    struct type *long_type
>      = arch_integer_type (gdbarch, gdbarch_long_bit (gdbarch), 0, 
> "long");
> @@ -1646,14 +1642,12 @@ linux_collect_thread_registers (const struct
> regcache *regcache,
>     with the size of the data.  The caller is responsible for freeing
>     the data.  */
> 
> -static gdb_byte *
> +static gdb::unique_xmalloc_ptr<gdb_byte>
>  linux_get_siginfo_data (thread_info *thread, struct gdbarch *gdbarch,
>  			LONGEST *size)
>  {
>    struct type *siginfo_type;
> -  gdb_byte *buf;
>    LONGEST bytes_read;
> -  struct cleanup *cleanups;
> 
>    if (!gdbarch_get_siginfo_type_p (gdbarch))
>      return NULL;
> @@ -1663,21 +1657,15 @@ linux_get_siginfo_data (thread_info *thread,
> struct gdbarch *gdbarch,
> 
>    siginfo_type = gdbarch_get_siginfo_type (gdbarch);
> 
> -  buf = (gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type));
> -  cleanups = make_cleanup (xfree, buf);
> +  gdb::unique_xmalloc_ptr<gdb_byte> buf
> +    ((gdb_byte *) xmalloc (TYPE_LENGTH (siginfo_type)));
> 
>    bytes_read = target_read (&current_target, 
> TARGET_OBJECT_SIGNAL_INFO, NULL,
> -			    buf, 0, TYPE_LENGTH (siginfo_type));
> +			    buf.get (), 0, TYPE_LENGTH (siginfo_type));
>    if (bytes_read == TYPE_LENGTH (siginfo_type))
> -    {
> -      discard_cleanups (cleanups);
> -      *size = bytes_read;
> -    }
> +    *size = bytes_read;
>    else
> -    {
> -      do_cleanups (cleanups);
> -      buf = NULL;
> -    }
> +    return NULL;
> 
>    return buf;

I think this could return a gdb::byte_vector, with an empty vector 
meaning that there is no value, the equivalent of NULL currently (I 
don't think we can have a siginfo type with size 0...).

Simon

Re: [RFA 10/13] Remove cleanups from linux-tdep.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Just to be pedantic, you should mention the change to
Simon> linux_make_mappings_corefile_notes.

...
Simon> I think this could return a gdb::byte_vector, with an empty vector
Simon> meaning that there is no value, the equivalent of NULL currently (I
Simon> don't think we can have a siginfo type with size 0...).

I made these changes.

Tom

[RFA 09/13] Use gdb::def_vector in ppc-linux-tdep.c

[Tom Tromey]

This removes a cleanup from ppc-linux-tdep.c, replacing it with
gdb::def_vector.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* ppc-linux-tdep.c (ppc_linux_get_syscall_number): Use
	gdb::def_vector.
---
 gdb/ChangeLog        |  5 +++++
 gdb/ppc-linux-tdep.c | 13 ++++---------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
index 847908a6da..4d10a24ca9 100644
--- a/gdb/ppc-linux-tdep.c
+++ b/gdb/ppc-linux-tdep.c
@@ -725,26 +725,21 @@ ppc_linux_get_syscall_number (struct gdbarch *gdbarch,
   struct regcache *regcache = get_thread_regcache (ptid);
   struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
   enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
-  struct cleanup *cleanbuf;
-  /* The content of a register */
-  gdb_byte *buf;
   /* The result */
   LONGEST ret;
 
   /* Make sure we're in a 32- or 64-bit machine */
   gdb_assert (tdep->wordsize == 4 || tdep->wordsize == 8);
 
-  buf = (gdb_byte *) xmalloc (tdep->wordsize * sizeof (gdb_byte));
-
-  cleanbuf = make_cleanup (xfree, buf);
+  /* The content of a register */
+  gdb::def_vector<gdb_byte> buf (tdep->wordsize);
 
   /* Getting the system call number from the register.
      When dealing with PowerPC architecture, this information
      is stored at 0th register.  */
-  regcache_cooked_read (regcache, tdep->ppc_gp0_regnum, buf);
+  regcache_cooked_read (regcache, tdep->ppc_gp0_regnum, buf.data ());
 
-  ret = extract_signed_integer (buf, tdep->wordsize, byte_order);
-  do_cleanups (cleanbuf);
+  ret = extract_signed_integer (buf.data (), tdep->wordsize, byte_order);
 
   return ret;
 }
-- 
2.13.6

Re: [RFA 09/13] Use gdb::def_vector in ppc-linux-tdep.c

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> This removes a cleanup from ppc-linux-tdep.c, replacing it with
> gdb::def_vector.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* ppc-linux-tdep.c (ppc_linux_get_syscall_number): Use
> 	gdb::def_vector.
> ---
>  gdb/ChangeLog        |  5 +++++
>  gdb/ppc-linux-tdep.c | 13 ++++---------
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/gdb/ppc-linux-tdep.c b/gdb/ppc-linux-tdep.c
> index 847908a6da..4d10a24ca9 100644
> --- a/gdb/ppc-linux-tdep.c
> +++ b/gdb/ppc-linux-tdep.c
> @@ -725,26 +725,21 @@ ppc_linux_get_syscall_number (struct gdbarch 
> *gdbarch,
>    struct regcache *regcache = get_thread_regcache (ptid);
>    struct gdbarch_tdep *tdep = gdbarch_tdep (gdbarch);
>    enum bfd_endian byte_order = gdbarch_byte_order (gdbarch);
> -  struct cleanup *cleanbuf;
> -  /* The content of a register */
> -  gdb_byte *buf;
>    /* The result */
>    LONGEST ret;
> 
>    /* Make sure we're in a 32- or 64-bit machine */
>    gdb_assert (tdep->wordsize == 4 || tdep->wordsize == 8);
> 
> -  buf = (gdb_byte *) xmalloc (tdep->wordsize * sizeof (gdb_byte));
> -
> -  cleanbuf = make_cleanup (xfree, buf);
> +  /* The content of a register */
> +  gdb::def_vector<gdb_byte> buf (tdep->wordsize);

I would prefer if you used gdb::byte_vector, so it's consistent across 
the codebase.

> 
>    /* Getting the system call number from the register.
>       When dealing with PowerPC architecture, this information
>       is stored at 0th register.  */
> -  regcache_cooked_read (regcache, tdep->ppc_gp0_regnum, buf);
> +  regcache_cooked_read (regcache, tdep->ppc_gp0_regnum, buf.data ());
> 
> -  ret = extract_signed_integer (buf, tdep->wordsize, byte_order);
> -  do_cleanups (cleanbuf);
> +  ret = extract_signed_integer (buf.data (), tdep->wordsize, 
> byte_order);
> 
>    return ret;
>  }

You can skip the ret variable.

Simon

Re: [RFA 09/13] Use gdb::def_vector in ppc-linux-tdep.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> I would prefer if you used gdb::byte_vector, so it's consistent across
Simon> the codebase.

...

Simon> You can skip the ret variable.

I made these changes.

Tom

[RFA 03/13] Use std::vector in compile-loc2c.c

[Tom Tromey]

This changes compile-loc2c.c to use std::vector, removing some
cleanups.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* compile/compile-loc2c.c (compute_stack_depth_worker): Change
	type of "info".
	(compute_stack_depth): Likewise.
	(do_compile_dwarf_expr_to_c): Use std::vector.
---
 gdb/ChangeLog               |  7 +++++++
 gdb/compile/compile-loc2c.c | 42 +++++++++++++++++-------------------------
 2 files changed, 24 insertions(+), 25 deletions(-)

diff --git a/gdb/compile/compile-loc2c.c b/gdb/compile/compile-loc2c.c
index 7ec6f67cdf..ca26f13eda 100644
--- a/gdb/compile/compile-loc2c.c
+++ b/gdb/compile/compile-loc2c.c
@@ -62,7 +62,7 @@ struct insn_info
    NEED_TEMPVAR is an out parameter which is set if this expression
    needs a special temporary variable to be emitted (see the code
    generator).
-   INFO is an array of insn_info objects, indexed by offset from the
+   INFO is a vector of insn_info objects, indexed by offset from the
    start of the DWARF expression.
    TO_DO is a list of bytecodes which must be examined; it may be
    added to by this function.
@@ -71,7 +71,7 @@ struct insn_info
 
 static void
 compute_stack_depth_worker (int start, int *need_tempvar,
-			    struct insn_info *info,
+			    std::vector<struct insn_info> *info,
 			    std::vector<int> *to_do,
 			    enum bfd_endian byte_order, unsigned int addr_size,
 			    const gdb_byte *op_ptr, const gdb_byte *op_end)
@@ -80,8 +80,8 @@ compute_stack_depth_worker (int start, int *need_tempvar,
   int stack_depth;
 
   op_ptr += start;
-  gdb_assert (info[start].visited);
-  stack_depth = info[start].depth;
+  gdb_assert ((*info)[start].visited);
+  stack_depth = (*info)[start].depth;
 
   while (op_ptr < op_end)
     {
@@ -91,16 +91,16 @@ compute_stack_depth_worker (int start, int *need_tempvar,
       int ndx = op_ptr - base;
 
 #define SET_CHECK_DEPTH(WHERE)				\
-      if (info[WHERE].visited)				\
+      if ((*info)[WHERE].visited)				\
 	{						\
-	  if (info[WHERE].depth != stack_depth)		\
+	  if ((*info)[WHERE].depth != stack_depth)		\
 	    error (_("inconsistent stack depths"));	\
 	}						\
       else						\
 	{						\
 	  /* Stack depth not set, so set it.  */	\
-	  info[WHERE].visited = 1;			\
-	  info[WHERE].depth = stack_depth;		\
+	  (*info)[WHERE].visited = 1;			\
+	  (*info)[WHERE].depth = stack_depth;		\
 	}
 
       SET_CHECK_DEPTH (ndx);
@@ -324,7 +324,7 @@ compute_stack_depth_worker (int start, int *need_tempvar,
 
 	case DW_OP_GNU_push_tls_address:
 	case DW_OP_form_tls_address:
-	  info[ndx].is_tls = 1;
+	  (*info)[ndx].is_tls = 1;
 	  break;
 
 	case DW_OP_skip:
@@ -333,10 +333,10 @@ compute_stack_depth_worker (int start, int *need_tempvar,
 	  offset = op_ptr + offset - base;
 	  /* If the destination has not been seen yet, add it to the
 	     to-do list.  */
-	  if (!info[offset].visited)
+	  if (!(*info)[offset].visited)
 	    to_do->push_back (offset);
 	  SET_CHECK_DEPTH (offset);
-	  info[offset].label = 1;
+	  (*info)[offset].label = 1;
 	  /* We're done with this line of code.  */
 	  return;
 
@@ -347,10 +347,10 @@ compute_stack_depth_worker (int start, int *need_tempvar,
 	  --stack_depth;
 	  /* If the destination has not been seen yet, add it to the
 	     to-do list.  */
-	  if (!info[offset].visited)
+	  if (!(*info)[offset].visited)
 	    to_do->push_back (offset);
 	  SET_CHECK_DEPTH (offset);
-	  info[offset].label = 1;
+	  (*info)[offset].label = 1;
 	  break;
 
 	case DW_OP_nop:
@@ -387,15 +387,13 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
 		     int *need_tempvar, int *is_tls,
 		     const gdb_byte *op_ptr, const gdb_byte *op_end,
 		     int initial_depth,
-		     struct insn_info **info)
+		     std::vector<struct insn_info> *info)
 {
   unsigned char *set;
-  struct cleanup *outer_cleanup;
   std::vector<int> to_do;
   int stack_depth, i;
 
-  *info = XCNEWVEC (struct insn_info, op_end - op_ptr);
-  outer_cleanup = make_cleanup (xfree, *info);
+  info->resize (op_end - op_ptr);
 
   to_do.push_back (0);
   (*info)[0].depth = initial_depth;
@@ -406,7 +404,7 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
       int ndx = to_do.back ();
       to_do.pop_back ();
 
-      compute_stack_depth_worker (ndx, need_tempvar, *info, &to_do,
+      compute_stack_depth_worker (ndx, need_tempvar, info, &to_do,
 				  byte_order, addr_size,
 				  op_ptr, op_end);
     }
@@ -421,7 +419,6 @@ compute_stack_depth (enum bfd_endian byte_order, unsigned int addr_size,
 	*is_tls = 1;
     }
 
-  discard_cleanups (outer_cleanup);
   return stack_depth + 1;
 }
 
@@ -594,8 +591,7 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
   const gdb_byte * const base = op_ptr;
   int need_tempvar = 0;
   int is_tls = 0;
-  struct cleanup *cleanup;
-  struct insn_info *info;
+  std::vector<struct insn_info> info;
   int stack_depth;
 
   ++scope;
@@ -609,7 +605,6 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
 				     &need_tempvar, &is_tls,
 				     op_ptr, op_end, initial != NULL,
 				     &info);
-  cleanup = make_cleanup (xfree, info);
 
   /* This is a hack until we can add a feature to glibc to let us
      properly generate code for TLS.  You might think we could emit
@@ -643,7 +638,6 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
 			 result_name,
 			 core_addr_to_string (value_address (val)));
       fprintfi_filtered (indent - 2, &stream, "}\n");
-      do_cleanups (cleanup);
       return;
     }
 
@@ -1117,8 +1111,6 @@ do_compile_dwarf_expr_to_c (int indent, string_file &stream,
   fprintfi_filtered (indent, &stream, "%s = __gdb_stack[__gdb_tos];\n",
 		     result_name);
   fprintfi_filtered (indent - 2, &stream, "}\n");
-
-  do_cleanups (cleanup);
 }
 
 /* See compile.h.  */
-- 
2.13.6

[RFA 06/13] Replace start_rbreak_breakpoints and end_rbreak_breakpoints

[Tom Tromey]

This replaces start_rbreak_breakpoints and end_rbreak_breakpoints with
a new scoped class.  This allows the removal of a cleanup.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* breakpoint.c
	(scoped_rbreak_breakpoints::scoped_rbreak_breakpoints): Rename
	from start_rbreak_breakpoints.
	(scoped_rbreak_breakpoints): Rename from end_rbreak_breakpoints.
	* breakpoint.h (class scoped_rbreak_breakpoints): New.
	(start_rbreak_breakpoints, end_rbreak_breakpoints): Remove.
	* symtab.c (do_end_rbreak_breakpoints): Remove.
	(rbreak_command): Use scoped_rbreak_breakpoints.
---
 gdb/ChangeLog    | 11 +++++++++++
 gdb/breakpoint.c |  6 ++----
 gdb/breakpoint.h | 16 ++++++++++++----
 gdb/symtab.c     | 14 +-------------
 4 files changed, 26 insertions(+), 21 deletions(-)

diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0bf47d5f56..61e41283b1 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -644,8 +644,7 @@ static int rbreak_start_breakpoint_count;
 /* Called at the start an "rbreak" command to record the first
    breakpoint made.  */
 
-void
-start_rbreak_breakpoints (void)
+scoped_rbreak_breakpoints::scoped_rbreak_breakpoints ()
 {
   rbreak_start_breakpoint_count = breakpoint_count;
 }
@@ -653,8 +652,7 @@ start_rbreak_breakpoints (void)
 /* Called at the end of an "rbreak" command to record the last
    breakpoint made.  */
 
-void
-end_rbreak_breakpoints (void)
+scoped_rbreak_breakpoints::~scoped_rbreak_breakpoints ()
 {
   prev_breakpoint_count = rbreak_start_breakpoint_count;
 }
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9a9433bd66..69b63ea932 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1601,10 +1601,18 @@ extern VEC(breakpoint_p) *static_tracepoints_here (CORE_ADDR addr);
    that each command is suitable for tracepoint command list.  */
 extern void check_tracepoint_command (char *line, void *closure);
 
-/* Call at the start and end of an "rbreak" command to register
-   breakpoint numbers for a later "commands" command.  */
-extern void start_rbreak_breakpoints (void);
-extern void end_rbreak_breakpoints (void);
+/* Create an instance of this to start registering breakpoint numbers
+   for a later "commands" command.  */
+
+class scoped_rbreak_breakpoints
+{
+public:
+
+  scoped_rbreak_breakpoints ();
+  ~scoped_rbreak_breakpoints ();
+
+  DISABLE_COPY_AND_ASSIGN (scoped_rbreak_breakpoints);
+};
 
 /* Breakpoint iterator function.
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 16a6b2eb6f..b1c3515cc7 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4506,18 +4506,9 @@ rbreak_command_wrapper (char *regexp, int from_tty)
   rbreak_command (regexp, from_tty);
 }
 
-/* A cleanup function that calls end_rbreak_breakpoints.  */
-
-static void
-do_end_rbreak_breakpoints (void *ignore)
-{
-  end_rbreak_breakpoints ();
-}
-
 static void
 rbreak_command (char *regexp, int from_tty)
 {
-  struct cleanup *old_chain;
   char *string = NULL;
   int len = 0;
   const char **files = NULL;
@@ -4550,8 +4541,7 @@ rbreak_command (char *regexp, int from_tty)
 						       FUNCTIONS_DOMAIN,
 						       nfiles, files);
 
-  start_rbreak_breakpoints ();
-  old_chain = make_cleanup (do_end_rbreak_breakpoints, NULL);
+  scoped_rbreak_breakpoints finalize;
   for (const symbol_search &p : symbols)
     {
       if (p.msymbol.minsym == NULL)
@@ -4596,8 +4586,6 @@ rbreak_command (char *regexp, int from_tty)
 			   MSYMBOL_PRINT_NAME (p.msymbol.minsym));
 	}
     }
-
-  do_cleanups (old_chain);
 }
 \f
 
-- 
2.13.6

Re: [RFA 06/13] Replace start_rbreak_breakpoints and end_rbreak_breakpoints

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
>  static void
>  rbreak_command (char *regexp, int from_tty)
>  {
> -  struct cleanup *old_chain;
>    char *string = NULL;
>    int len = 0;
>    const char **files = NULL;
> @@ -4550,8 +4541,7 @@ rbreak_command (char *regexp, int from_tty)
>  						       FUNCTIONS_DOMAIN,
>  						       nfiles, files);
> 
> -  start_rbreak_breakpoints ();
> -  old_chain = make_cleanup (do_end_rbreak_breakpoints, NULL);
> +  scoped_rbreak_breakpoints finalize;
>    for (const symbol_search &p : symbols)
>      {
>        if (p.msymbol.minsym == NULL)
> @@ -4596,8 +4586,6 @@ rbreak_command (char *regexp, int from_tty)
>  			   MSYMBOL_PRINT_NAME (p.msymbol.minsym));
>  	}
>      }
> -
> -  do_cleanups (old_chain);
>  }
>  \f

Your patch LGTM.  But how is the "string" string freed in 
rbreak_command?

Simon

Re: [RFA 06/13] Replace start_rbreak_breakpoints and end_rbreak_breakpoints

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Your patch LGTM.  But how is the "string" string freed in
Simon> rbreak_command?

It is leaked, which apparently is a regression introduced by an earlier
patch of mine.  Sorry about that!  I've updated the patch as appended to
fix this problem.

Tom

commit 391e4d7aa45fc4947632aa06a7aa462eeef3377c
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Nov 1 09:00:09 2017 -0600

    Replace start_rbreak_breakpoints and end_rbreak_breakpoints
    
    This replaces start_rbreak_breakpoints and end_rbreak_breakpoints with
    a new scoped class.  This allows the removal of a cleanup.
    
    This also fixes an earlier memory leak regression, by changing
    "string" to be a std::string.
    
    gdb/ChangeLog
    2017-11-03  Tom Tromey  <tom@tromey.com>
    
            * breakpoint.c
            (scoped_rbreak_breakpoints::scoped_rbreak_breakpoints): Rename
            from start_rbreak_breakpoints.
            (scoped_rbreak_breakpoints): Rename from end_rbreak_breakpoints.
            * breakpoint.h (class scoped_rbreak_breakpoints): New.
            (start_rbreak_breakpoints, end_rbreak_breakpoints): Remove.
            * symtab.c (do_end_rbreak_breakpoints): Remove.
            (rbreak_command): Use scoped_rbreak_breakpoints, std::string.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ce6c8db631..56b7e903bc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,14 @@
+2017-11-03  Tom Tromey  <tom@tromey.com>
+
+	* breakpoint.c
+	(scoped_rbreak_breakpoints::scoped_rbreak_breakpoints): Rename
+	from start_rbreak_breakpoints.
+	(scoped_rbreak_breakpoints): Rename from end_rbreak_breakpoints.
+	* breakpoint.h (class scoped_rbreak_breakpoints): New.
+	(start_rbreak_breakpoints, end_rbreak_breakpoints): Remove.
+	* symtab.c (do_end_rbreak_breakpoints): Remove.
+	(rbreak_command): Use scoped_rbreak_breakpoints, std::string.
+
 2017-11-02  Tom Tromey  <tom@tromey.com>
 
 	* cp-namespace.c (reset_directive_searched): Remove.
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 0bf47d5f56..61e41283b1 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -644,8 +644,7 @@ static int rbreak_start_breakpoint_count;
 /* Called at the start an "rbreak" command to record the first
    breakpoint made.  */
 
-void
-start_rbreak_breakpoints (void)
+scoped_rbreak_breakpoints::scoped_rbreak_breakpoints ()
 {
   rbreak_start_breakpoint_count = breakpoint_count;
 }
@@ -653,8 +652,7 @@ start_rbreak_breakpoints (void)
 /* Called at the end of an "rbreak" command to record the last
    breakpoint made.  */
 
-void
-end_rbreak_breakpoints (void)
+scoped_rbreak_breakpoints::~scoped_rbreak_breakpoints ()
 {
   prev_breakpoint_count = rbreak_start_breakpoint_count;
 }
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 9a9433bd66..69b63ea932 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1601,10 +1601,18 @@ extern VEC(breakpoint_p) *static_tracepoints_here (CORE_ADDR addr);
    that each command is suitable for tracepoint command list.  */
 extern void check_tracepoint_command (char *line, void *closure);
 
-/* Call at the start and end of an "rbreak" command to register
-   breakpoint numbers for a later "commands" command.  */
-extern void start_rbreak_breakpoints (void);
-extern void end_rbreak_breakpoints (void);
+/* Create an instance of this to start registering breakpoint numbers
+   for a later "commands" command.  */
+
+class scoped_rbreak_breakpoints
+{
+public:
+
+  scoped_rbreak_breakpoints ();
+  ~scoped_rbreak_breakpoints ();
+
+  DISABLE_COPY_AND_ASSIGN (scoped_rbreak_breakpoints);
+};
 
 /* Breakpoint iterator function.
 
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 16a6b2eb6f..25b3de0dfb 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -4506,20 +4506,10 @@ rbreak_command_wrapper (char *regexp, int from_tty)
   rbreak_command (regexp, from_tty);
 }
 
-/* A cleanup function that calls end_rbreak_breakpoints.  */
-
-static void
-do_end_rbreak_breakpoints (void *ignore)
-{
-  end_rbreak_breakpoints ();
-}
-
 static void
 rbreak_command (char *regexp, int from_tty)
 {
-  struct cleanup *old_chain;
-  char *string = NULL;
-  int len = 0;
+  std::string string;
   const char **files = NULL;
   const char *file_name;
   int nfiles = 0;
@@ -4550,8 +4540,7 @@ rbreak_command (char *regexp, int from_tty)
 						       FUNCTIONS_DOMAIN,
 						       nfiles, files);
 
-  start_rbreak_breakpoints ();
-  old_chain = make_cleanup (do_end_rbreak_breakpoints, NULL);
+  scoped_rbreak_breakpoints finalize;
   for (const symbol_search &p : symbols)
     {
       if (p.msymbol.minsym == NULL)
@@ -4559,20 +4548,9 @@ rbreak_command (char *regexp, int from_tty)
 	  struct symtab *symtab = symbol_symtab (p.symbol);
 	  const char *fullname = symtab_to_fullname (symtab);
 
-	  int newlen = (strlen (fullname)
-			+ strlen (SYMBOL_LINKAGE_NAME (p.symbol))
-			+ 4);
-
-	  if (newlen > len)
-	    {
-	      string = (char *) xrealloc (string, newlen);
-	      len = newlen;
-	    }
-	  strcpy (string, fullname);
-	  strcat (string, ":'");
-	  strcat (string, SYMBOL_LINKAGE_NAME (p.symbol));
-	  strcat (string, "'");
-	  break_command (string, from_tty);
+	  string = (std::string (fullname) + ":'"
+		    + SYMBOL_LINKAGE_NAME (p.symbol) + "'");
+	  break_command (&string[0], from_tty);
 	  print_symbol_info (FUNCTIONS_DOMAIN,
 			     p.symbol,
 			     p.block,
@@ -4580,24 +4558,14 @@ rbreak_command (char *regexp, int from_tty)
 	}
       else
 	{
-	  int newlen = (strlen (MSYMBOL_LINKAGE_NAME (p.msymbol.minsym)) + 3);
-
-	  if (newlen > len)
-	    {
-	      string = (char *) xrealloc (string, newlen);
-	      len = newlen;
-	    }
-	  strcpy (string, "'");
-	  strcat (string, MSYMBOL_LINKAGE_NAME (p.msymbol.minsym));
-	  strcat (string, "'");
+	  string = (std::string ("'")
+		    + MSYMBOL_LINKAGE_NAME (p.msymbol.minsym) + "'");
 
-	  break_command (string, from_tty);
+	  break_command (&string[0], from_tty);
 	  printf_filtered ("<function, no debug info> %s;\n",
 			   MSYMBOL_PRINT_NAME (p.msymbol.minsym));
 	}
     }
-
-  do_cleanups (old_chain);
 }
 \f
 

Re: [RFA 06/13] Replace start_rbreak_breakpoints and end_rbreak_breakpoints

[Simon Marchi]

On 2017-11-03 12:58, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
> Simon> Your patch LGTM.  But how is the "string" string freed in
> Simon> rbreak_command?
> 
> It is leaked, which apparently is a regression introduced by an earlier
> patch of mine.  Sorry about that!  I've updated the patch as appended 
> to
> fix this problem.
> 
> Tom
> 
> commit 391e4d7aa45fc4947632aa06a7aa462eeef3377c
> Author: Tom Tromey <tom@tromey.com>
> Date:   Wed Nov 1 09:00:09 2017 -0600
> 
>     Replace start_rbreak_breakpoints and end_rbreak_breakpoints
> 
>     This replaces start_rbreak_breakpoints and end_rbreak_breakpoints 
> with
>     a new scoped class.  This allows the removal of a cleanup.
> 
>     This also fixes an earlier memory leak regression, by changing
>     "string" to be a std::string.

LGTM, though I would suggest using string_print, I think it would be 
more readable.

Simon

Re: [RFA 06/13] Replace start_rbreak_breakpoints and end_rbreak_breakpoints

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> This also fixes an earlier memory leak regression, by changing
>> "string" to be a std::string.

Simon> LGTM, though I would suggest using string_print, I think it would be
Simon> more readable.

I made this change.

Tom

[RFA 01/13] Replace really_free_pendings with a scoped_ class

[Tom Tromey]

This introduces scoped_free_pendings, and changes users of
really_free_pendings to use it instead, removing some clenaups.

I tried to examine the affected code to ensure there aren't dangling
cleanups in the vicinity.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* dwarf2read.c (process_full_comp_unit, process_full_type_unit):
	Use scoped_free_pendings.
	* dbxread.c (dbx_symfile_read, dbx_psymtab_to_symtab_1): Use
	scoped_free_pendings.
	* xcoffread.c (xcoff_psymtab_to_symtab_1): Use scoped_free_pendings.
	(xcoff_initial_scan): Likewise.
	* buildsym.c (reset_symtab_globals): Update comment.
	(scoped_free_pendings): Rename from really_free_pendings.
	(prepare_for_building): Update comment.
	(buildsym_init): Likewise.
	* buildsym.h (class scoped_free_pendings): New class.
	(really_free_pendings): Don't declare.
---
 gdb/ChangeLog    | 15 +++++++++++++++
 gdb/buildsym.c   | 26 ++++++++++----------------
 gdb/buildsym.h   | 10 +++++++++-
 gdb/dbxread.c    | 10 ++--------
 gdb/dwarf2read.c | 12 ++++--------
 gdb/xcoffread.c  | 10 ++--------
 6 files changed, 42 insertions(+), 41 deletions(-)

diff --git a/gdb/buildsym.c b/gdb/buildsym.c
index 07bfbd5038..d07bfb3d13 100644
--- a/gdb/buildsym.c
+++ b/gdb/buildsym.c
@@ -26,11 +26,10 @@
    The basic way this module is used is as follows:
 
    buildsym_init ();
-   cleanups = make_cleanup (really_free_pendings, NULL);
+   scoped_free_pendings free_pending;
    cust = start_symtab (...);
    ... read debug info ...
    cust = end_symtab (...);
-   do_cleanups (cleanups);
 
    The compunit symtab pointer ("cust") is returned from both start_symtab
    and end_symtab to simplify the debug info readers.
@@ -42,31 +41,28 @@
    Reading DWARF Type Units is another variation:
 
    buildsym_init ();
-   cleanups = make_cleanup (really_free_pendings, NULL);
+   scoped_free_pendings free_pending;
    cust = start_symtab (...);
    ... read debug info ...
    cust = end_expandable_symtab (...);
-   do_cleanups (cleanups);
 
    And then reading subsequent Type Units within the containing "Comp Unit"
    will use a second flow:
 
    buildsym_init ();
-   cleanups = make_cleanup (really_free_pendings, NULL);
+   scoped_free_pendings free_pending;
    cust = restart_symtab (...);
    ... read debug info ...
    cust = augment_type_symtab (...);
-   do_cleanups (cleanups);
 
    dbxread.c and xcoffread.c use another variation:
 
    buildsym_init ();
-   cleanups = make_cleanup (really_free_pendings, NULL);
+   scoped_free_pendings free_pending;
    cust = start_symtab (...);
    ... read debug info ...
    cust = end_symtab (...);
    ... start_symtab + read + end_symtab repeated ...
-   do_cleanups (cleanups);
 */
 
 #include "defs.h"
@@ -269,15 +265,13 @@ find_symbol_in_list (struct pending *list, char *name, int length)
   return (NULL);
 }
 
-/* At end of reading syms, or in case of quit, ensure everything associated
-   with building symtabs is freed.  This is intended to be registered as a
-   cleanup before doing psymtab->symtab expansion.
+/* At end of reading syms, or in case of quit, ensure everything
+   associated with building symtabs is freed.
 
    N.B. This is *not* intended to be used when building psymtabs.  Some debug
    info readers call this anyway, which is harmless if confusing.  */
 
-void
-really_free_pendings (void *dummy)
+scoped_free_pendings::~scoped_free_pendings ()
 {
   struct pending *next, *next1;
 
@@ -1028,7 +1022,7 @@ prepare_for_building (const char *name, CORE_ADDR start_addr)
   context_stack_depth = 0;
 
   /* These should have been reset either by successful completion of building
-     a symtab, or by the really_free_pendings cleanup.  */
+     a symtab, or by the scoped_free_pendings destructor.  */
   gdb_assert (file_symbols == NULL);
   gdb_assert (global_symbols == NULL);
   gdb_assert (global_using_directives == NULL);
@@ -1169,7 +1163,7 @@ watch_main_source_file_lossage (void)
 /* Reset state after a successful building of a symtab.
    This exists because dbxread.c and xcoffread.c can call
    start_symtab+end_symtab multiple times after one call to buildsym_init,
-   and before the really_free_pendings cleanup is called.
+   and before the scoped_free_pendings destructor is called.
    We keep the free_pendings list around for dbx/xcoff sake.  */
 
 static void
@@ -1753,7 +1747,7 @@ buildsym_init (void)
       context_stack = XNEWVEC (struct context_stack, context_stack_size);
     }
 
-  /* Ensure the really_free_pendings cleanup was called after
+  /* Ensure the scoped_free_pendings destructor was called after
      the last time.  */
   gdb_assert (free_pendings == NULL);
   gdb_assert (pending_blocks == NULL);
diff --git a/gdb/buildsym.h b/gdb/buildsym.h
index 60109a0da2..accb1f0347 100644
--- a/gdb/buildsym.h
+++ b/gdb/buildsym.h
@@ -212,7 +212,15 @@ extern struct block *finish_block (struct symbol *symbol,
 extern void record_block_range (struct block *,
                                 CORE_ADDR start, CORE_ADDR end_inclusive);
 
-extern void really_free_pendings (void *dummy);
+class scoped_free_pendings
+{
+public:
+
+  scoped_free_pendings () = default;
+  ~scoped_free_pendings ();
+
+  DISABLE_COPY_AND_ASSIGN (scoped_free_pendings);
+};
 
 extern void start_subfile (const char *name);
 
diff --git a/gdb/dbxread.c b/gdb/dbxread.c
index ce00962809..1618a56136 100644
--- a/gdb/dbxread.c
+++ b/gdb/dbxread.c
@@ -516,7 +516,6 @@ dbx_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
 {
   bfd *sym_bfd;
   int val;
-  struct cleanup *back_to;
 
   sym_bfd = objfile->obfd;
 
@@ -539,7 +538,7 @@ dbx_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
   symbol_table_offset = DBX_SYMTAB_OFFSET (objfile);
 
   free_pending_blocks ();
-  back_to = make_cleanup (really_free_pendings, 0);
+  scoped_free_pendings free_pending;
 
   minimal_symbol_reader reader (objfile);
 
@@ -551,8 +550,6 @@ dbx_symfile_read (struct objfile *objfile, symfile_add_flags symfile_flags)
      minimal symbols for this objfile.  */
 
   reader.install ();
-
-  do_cleanups (back_to);
 }
 
 /* Initialize anything that needs initializing when a completely new
@@ -2186,7 +2183,6 @@ dbx_end_psymtab (struct objfile *objfile, struct partial_symtab *pst,
 static void
 dbx_psymtab_to_symtab_1 (struct objfile *objfile, struct partial_symtab *pst)
 {
-  struct cleanup *old_chain;
   int i;
 
   if (pst->readin)
@@ -2220,15 +2216,13 @@ dbx_psymtab_to_symtab_1 (struct objfile *objfile, struct partial_symtab *pst)
       /* Init stuff necessary for reading in symbols */
       stabsread_init ();
       buildsym_init ();
-      old_chain = make_cleanup (really_free_pendings, 0);
+      scoped_free_pendings free_pending;
       file_string_table_offset = FILE_STRING_OFFSET (pst);
       symbol_size = SYMBOL_SIZE (pst);
 
       /* Read in this file's symbols.  */
       bfd_seek (objfile->obfd, SYMBOL_OFFSET (pst), SEEK_SET);
       read_ofile_symtab (objfile, pst);
-
-      do_cleanups (old_chain);
     }
 
   pst->readin = 1;
diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
index 686fa3f3d3..433a9272b8 100644
--- a/gdb/dwarf2read.c
+++ b/gdb/dwarf2read.c
@@ -8317,7 +8317,7 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
   struct gdbarch *gdbarch = get_objfile_arch (objfile);
   CORE_ADDR lowpc, highpc;
   struct compunit_symtab *cust;
-  struct cleanup *back_to, *delayed_list_cleanup;
+  struct cleanup *delayed_list_cleanup;
   CORE_ADDR baseaddr;
   struct block *static_block;
   CORE_ADDR addr;
@@ -8325,7 +8325,7 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
   baseaddr = ANOFFSET (objfile->section_offsets, SECT_OFF_TEXT (objfile));
 
   buildsym_init ();
-  back_to = make_cleanup (really_free_pendings, NULL);
+  scoped_free_pendings free_pending;
   delayed_list_cleanup = make_cleanup (free_delayed_list, cu);
 
   cu->list_in_scope = &file_symbols;
@@ -8407,8 +8407,6 @@ process_full_comp_unit (struct dwarf2_per_cu_data *per_cu,
 
   /* Push it for inclusion processing later.  */
   VEC_safe_push (dwarf2_per_cu_ptr, dwarf2_per_objfile->just_read_cus, per_cu);
-
-  do_cleanups (back_to);
 }
 
 /* Generate full symbol information for type unit PER_CU, whose DIEs have
@@ -8421,14 +8419,14 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
   struct dwarf2_cu *cu = per_cu->cu;
   struct objfile *objfile = per_cu->objfile;
   struct compunit_symtab *cust;
-  struct cleanup *back_to, *delayed_list_cleanup;
+  struct cleanup *delayed_list_cleanup;
   struct signatured_type *sig_type;
 
   gdb_assert (per_cu->is_debug_types);
   sig_type = (struct signatured_type *) per_cu;
 
   buildsym_init ();
-  back_to = make_cleanup (really_free_pendings, NULL);
+  scoped_free_pendings free_pending;
   delayed_list_cleanup = make_cleanup (free_delayed_list, cu);
 
   cu->list_in_scope = &file_symbols;
@@ -8483,8 +8481,6 @@ process_full_type_unit (struct dwarf2_per_cu_data *per_cu,
       pst->compunit_symtab = cust;
       pst->readin = 1;
     }
-
-  do_cleanups (back_to);
 }
 
 /* Process an imported unit DIE.  */
diff --git a/gdb/xcoffread.c b/gdb/xcoffread.c
index ea11b3f966..2e4f30f291 100644
--- a/gdb/xcoffread.c
+++ b/gdb/xcoffread.c
@@ -1833,7 +1833,6 @@ find_linenos (struct bfd *abfd, struct bfd_section *asect, void *vpinfo)
 static void
 xcoff_psymtab_to_symtab_1 (struct objfile *objfile, struct partial_symtab *pst)
 {
-  struct cleanup *old_chain;
   int i;
 
   if (!pst)
@@ -1870,11 +1869,9 @@ xcoff_psymtab_to_symtab_1 (struct objfile *objfile, struct partial_symtab *pst)
       /* Init stuff necessary for reading in symbols.  */
       stabsread_init ();
       buildsym_init ();
-      old_chain = make_cleanup (really_free_pendings, 0);
 
+      scoped_free_pendings free_pending;
       read_xcoff_symtab (objfile, pst);
-
-      do_cleanups (old_chain);
     }
 
   pst->readin = 1;
@@ -2950,7 +2947,6 @@ xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
 {
   bfd *abfd;
   int val;
-  struct cleanup *back_to;
   int num_symbols;		/* # of symbols */
   file_ptr symtab_offset;	/* symbol table and */
   file_ptr stringtab_offset;	/* string table file offsets */
@@ -3027,8 +3023,8 @@ xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
     init_psymbol_list (objfile, num_symbols);
 
   free_pending_blocks ();
-  back_to = make_cleanup (really_free_pendings, 0);
 
+  scoped_free_pendings free_pending;
   minimal_symbol_reader reader (objfile);
 
   /* Now that the symbol table data of the executable file are all in core,
@@ -3047,8 +3043,6 @@ xcoff_initial_scan (struct objfile *objfile, symfile_add_flags symfile_flags)
     dwarf2_build_psymtabs (objfile);
 
   dwarf2_build_frame_info (objfile);
-
-  do_cleanups (back_to);
 }
 \f
 static void
-- 
2.13.6

[RFA 13/13] Use std::vector in h8300-tdep.c

[Tom Tromey]

This changes h8300-tdep.c to use std::vector, allowing the removal of
a cleanup.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* h8300-tdep.c (h8300_push_dummy_call): Use std::vector.
---
 gdb/ChangeLog    |  4 ++++
 gdb/h8300-tdep.c | 16 ++++++----------
 2 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
index 011afcaba4..b9936c0283 100644
--- a/gdb/h8300-tdep.c
+++ b/gdb/h8300-tdep.c
@@ -662,18 +662,16 @@ h8300_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 
   for (argument = 0; argument < nargs; argument++)
     {
-      struct cleanup *back_to;
       struct type *type = value_type (args[argument]);
       int len = TYPE_LENGTH (type);
       char *contents = (char *) value_contents (args[argument]);
 
       /* Pad the argument appropriately.  */
       int padded_len = align_up (len, wordsize);
-      gdb_byte *padded = (gdb_byte *) xmalloc (padded_len);
-      back_to = make_cleanup (xfree, padded);
+      std::vector<gdb_byte> padded (padded_len);
 
-      memset (padded, 0, padded_len);
-      memcpy (len < wordsize ? padded + padded_len - len : padded,
+      memcpy ((len < wordsize ? padded.data () + padded_len - len
+	       : padded.data ()),
 	      contents, len);
 
       /* Could the argument fit in the remaining registers?  */
@@ -684,7 +682,7 @@ h8300_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	  if (len > wordsize && len % wordsize)
 	    {
 	      /* I feel so unclean.  */
-	      write_memory (sp + stack_offset, padded, padded_len);
+	      write_memory (sp + stack_offset, padded.data (), padded_len);
 	      stack_offset += padded_len;
 
 	      /* That's right --- even though we passed the argument
@@ -702,7 +700,7 @@ h8300_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
 	      for (offset = 0; offset < padded_len; offset += wordsize)
 		{
 		  ULONGEST word
-		    = extract_unsigned_integer (padded + offset,
+		    = extract_unsigned_integer (&padded[offset],
 						wordsize, byte_order);
 		  regcache_cooked_write_unsigned (regcache, reg++, word);
 		}
@@ -711,15 +709,13 @@ h8300_push_dummy_call (struct gdbarch *gdbarch, struct value *function,
       else
 	{
 	  /* It doesn't fit in registers!  Onto the stack it goes.  */
-	  write_memory (sp + stack_offset, padded, padded_len);
+	  write_memory (sp + stack_offset, padded.data (), padded_len);
 	  stack_offset += padded_len;
 
 	  /* Once one argument has spilled onto the stack, all
 	     subsequent arguments go on the stack.  */
 	  reg = E_ARGLAST_REGNUM + 1;
 	}
-
-      do_cleanups (back_to);
     }
 
   /* Store return address.  */
-- 
2.13.6

Re: [RFA 13/13] Use std::vector in h8300-tdep.c

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> This changes h8300-tdep.c to use std::vector, allowing the removal of
> a cleanup.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* h8300-tdep.c (h8300_push_dummy_call): Use std::vector.
> ---
>  gdb/ChangeLog    |  4 ++++
>  gdb/h8300-tdep.c | 16 ++++++----------
>  2 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/gdb/h8300-tdep.c b/gdb/h8300-tdep.c
> index 011afcaba4..b9936c0283 100644
> --- a/gdb/h8300-tdep.c
> +++ b/gdb/h8300-tdep.c
> @@ -662,18 +662,16 @@ h8300_push_dummy_call (struct gdbarch *gdbarch,
> struct value *function,
> 
>    for (argument = 0; argument < nargs; argument++)
>      {
> -      struct cleanup *back_to;
>        struct type *type = value_type (args[argument]);
>        int len = TYPE_LENGTH (type);
>        char *contents = (char *) value_contents (args[argument]);
> 
>        /* Pad the argument appropriately.  */
>        int padded_len = align_up (len, wordsize);
> -      gdb_byte *padded = (gdb_byte *) xmalloc (padded_len);
> -      back_to = make_cleanup (xfree, padded);
> +      std::vector<gdb_byte> padded (padded_len);

I would suggest adding a comment that std::vector is used (and not 
gdb::byte_vector) because we want it to be zero-ed out.  Otherwise 
someone will feel the urge to change it to gdb::byte_vector some day :).

Simon

Re: [RFA 13/13] Use std::vector in h8300-tdep.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> I would suggest adding a comment that std::vector is used (and not
Simon> gdb::byte_vector) because we want it to be zero-ed out.  Otherwise
Simon> someone will feel the urge to change it to gdb::byte_vector some day
Simon> :).

I did this.

Tom

[RFA 12/13] Introduce gdb_breakpoint_up

[Tom Tromey]

This introduces gdb_breakpoint_up, a unique_ptr typedef that owns a
breakpoint.  It then changes set_momentary_breakpoint to return a
gdb_breakpoint_up and fixes up the fallout.  This then allows the
removal of make_cleanup_delete_breakpoint.

Once breakpoints are fully C++-ified, this typedef can be removed in
favor of a plain std::unique_ptr.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* breakpoint.c (set_momentary_breakpoint): Return
	gdb_breakpoint_up.
	(until_break_command): Update.
	(new_until_break_fsm): Change argument types to
	gdb_breakpoint_up.
	(set_momentary_breakpoint_at_pc): Return gdb_breakpoint_up.
	(do_delete_breakpoint_cleanup, make_cleanup_delete_breakpoint):
	Remove.
	* infcmd.c (finish_forward): Update.
	* breakpoint.h (set_momentary_breakpoint)
	(set_momentary_breakpoint_at_pc): Return gdb_breakpoint_up.
	(make_cleanup_delete_breakpoint): Remove.
	(struct gdb_breakpoint_deleter): New.
	(gdb_breakpoint_up): New typedef.
	* infrun.c (insert_step_resume_breakpoint_at_sal_1): Update.
	(insert_exception_resume_breakpoint): Update.
	(insert_exception_resume_from_probe): Update.
	(insert_longjmp_resume_breakpoint): Update.
	* arm-linux-tdep.c (arm_linux_copy_svc): Update.
	* elfread.c (elf_gnu_ifunc_resolver_stop): Update.
	* infcall.c (call_function_by_hand_dummy): Update
---
 gdb/ChangeLog        | 24 ++++++++++++++++++++++++
 gdb/arm-linux-tdep.c |  2 +-
 gdb/breakpoint.c     | 35 +++++++++++------------------------
 gdb/breakpoint.h     | 21 +++++++++++++++------
 gdb/elfread.c        |  7 ++++---
 gdb/infcall.c        |  5 +++--
 gdb/infcmd.c         |  2 +-
 gdb/infrun.c         |  9 +++++----
 8 files changed, 64 insertions(+), 41 deletions(-)

diff --git a/gdb/arm-linux-tdep.c b/gdb/arm-linux-tdep.c
index 525474c4db..7769c9c2e2 100644
--- a/gdb/arm-linux-tdep.c
+++ b/gdb/arm-linux-tdep.c
@@ -1020,7 +1020,7 @@ arm_linux_copy_svc (struct gdbarch *gdbarch, struct regcache *regs,
 	{
 	  inferior_thread ()->control.step_resume_breakpoint
 	    = set_momentary_breakpoint (gdbarch, sal, get_frame_id (frame),
-					bp_step_resume);
+					bp_step_resume).release ();
 
 	  /* set_momentary_breakpoint invalidates FRAME.  */
 	  frame = NULL;
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 61e41283b1..7237e81b41 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -8582,7 +8582,7 @@ new_single_step_breakpoint (int thread, struct gdbarch *gdbarch)
    SAL.  If FRAME_ID is valid, the breakpoint is restricted to that
    frame.  */
 
-struct breakpoint *
+gdb_breakpoint_up
 set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
 			  struct frame_id frame_id, enum bptype type)
 {
@@ -8605,7 +8605,7 @@ set_momentary_breakpoint (struct gdbarch *gdbarch, struct symtab_and_line sal,
 
   update_global_location_list_nothrow (UGLL_MAY_INSERT);
 
-  return b;
+  return gdb_breakpoint_up (b);
 }
 
 /* Make a momentary breakpoint based on the master breakpoint ORIG.
@@ -8658,7 +8658,7 @@ clone_momentary_breakpoint (struct breakpoint *orig)
   return momentary_breakpoint_from_master (orig, orig->type, orig->ops, 0);
 }
 
-struct breakpoint *
+gdb_breakpoint_up
 set_momentary_breakpoint_at_pc (struct gdbarch *gdbarch, CORE_ADDR pc,
 				enum bptype type)
 {
@@ -11146,8 +11146,8 @@ static struct thread_fsm_ops until_break_fsm_ops =
 
 static struct until_break_fsm *
 new_until_break_fsm (struct interp *cmd_interp, int thread,
-		     struct breakpoint *location_breakpoint,
-		     struct breakpoint *caller_breakpoint)
+		     gdb_breakpoint_up &&location_breakpoint,
+		     gdb_breakpoint_up &&caller_breakpoint)
 {
   struct until_break_fsm *sm;
 
@@ -11155,8 +11155,8 @@ new_until_break_fsm (struct interp *cmd_interp, int thread,
   thread_fsm_ctor (&sm->thread_fsm, &until_break_fsm_ops, cmd_interp);
 
   sm->thread = thread;
-  sm->location_breakpoint = location_breakpoint;
-  sm->caller_breakpoint = caller_breakpoint;
+  sm->location_breakpoint = location_breakpoint.release ();
+  sm->caller_breakpoint = caller_breakpoint.release ();
 
   return sm;
 }
@@ -11219,8 +11219,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
   struct gdbarch *frame_gdbarch;
   struct frame_id stack_frame_id;
   struct frame_id caller_frame_id;
-  struct breakpoint *location_breakpoint;
-  struct breakpoint *caller_breakpoint = NULL;
   struct cleanup *old_chain;
   int thread;
   struct thread_info *tp;
@@ -11269,6 +11267,7 @@ until_break_command (const char *arg, int from_tty, int anywhere)
   /* Keep within the current frame, or in frames called by the current
      one.  */
 
+  gdb_breakpoint_up caller_breakpoint;
   if (frame_id_p (caller_frame_id))
     {
       struct symtab_and_line sal2;
@@ -11281,7 +11280,6 @@ until_break_command (const char *arg, int from_tty, int anywhere)
 						    sal2,
 						    caller_frame_id,
 						    bp_until);
-      make_cleanup_delete_breakpoint (caller_breakpoint);
 
       set_longjmp_breakpoint (tp, caller_frame_id);
       make_cleanup (delete_longjmp_breakpoint_cleanup, &thread);
@@ -11290,6 +11288,7 @@ until_break_command (const char *arg, int from_tty, int anywhere)
   /* set_momentary_breakpoint could invalidate FRAME.  */
   frame = NULL;
 
+  gdb_breakpoint_up location_breakpoint;
   if (anywhere)
     /* If the user told us to continue until a specified location,
        we don't specify a frame at which we need to stop.  */
@@ -11300,10 +11299,10 @@ until_break_command (const char *arg, int from_tty, int anywhere)
        only at the very same frame.  */
     location_breakpoint = set_momentary_breakpoint (frame_gdbarch, sal,
 						    stack_frame_id, bp_until);
-  make_cleanup_delete_breakpoint (location_breakpoint);
 
   sm = new_until_break_fsm (command_interp (), tp->global_num,
-			    location_breakpoint, caller_breakpoint);
+			    std::move (location_breakpoint),
+			    std::move (caller_breakpoint));
   tp->thread_fsm = &sm->thread_fsm;
 
   discard_cleanups (old_chain);
@@ -13366,18 +13365,6 @@ delete_breakpoint (struct breakpoint *bpt)
   delete bpt;
 }
 
-static void
-do_delete_breakpoint_cleanup (void *b)
-{
-  delete_breakpoint ((struct breakpoint *) b);
-}
-
-struct cleanup *
-make_cleanup_delete_breakpoint (struct breakpoint *b)
-{
-  return make_cleanup (do_delete_breakpoint_cleanup, b);
-}
-
 /* Iterator function to call a user-provided callback function once
    for each of B and its related breakpoints.  */
 
diff --git a/gdb/breakpoint.h b/gdb/breakpoint.h
index 69b63ea932..494d05aa7d 100644
--- a/gdb/breakpoint.h
+++ b/gdb/breakpoint.h
@@ -1223,10 +1223,23 @@ extern void breakpoint_re_set (void);
 
 extern void breakpoint_re_set_thread (struct breakpoint *);
 
-extern struct breakpoint *set_momentary_breakpoint
+extern void delete_breakpoint (struct breakpoint *);
+
+struct gdb_breakpoint_deleter
+{
+  void operator() (struct breakpoint *b) const
+  {
+    delete_breakpoint (b);
+  }
+};
+
+typedef std::unique_ptr<struct breakpoint, gdb_breakpoint_deleter>
+    gdb_breakpoint_up;
+
+extern gdb_breakpoint_up set_momentary_breakpoint
   (struct gdbarch *, struct symtab_and_line, struct frame_id, enum bptype);
 
-extern struct breakpoint *set_momentary_breakpoint_at_pc
+extern gdb_breakpoint_up set_momentary_breakpoint_at_pc
   (struct gdbarch *, CORE_ADDR pc, enum bptype type);
 
 extern struct breakpoint *clone_momentary_breakpoint (struct breakpoint *bpkt);
@@ -1235,10 +1248,6 @@ extern void set_ignore_count (int, int, int);
 
 extern void breakpoint_init_inferior (enum inf_context);
 
-extern struct cleanup *make_cleanup_delete_breakpoint (struct breakpoint *);
-
-extern void delete_breakpoint (struct breakpoint *);
-
 extern void breakpoint_auto_delete (bpstat);
 
 typedef void (*walk_bp_location_callback) (struct bp_location *, void *);
diff --git a/gdb/elfread.c b/gdb/elfread.c
index 4e110716c3..cdef5a8c59 100644
--- a/gdb/elfread.c
+++ b/gdb/elfread.c
@@ -923,9 +923,10 @@ elf_gnu_ifunc_resolver_stop (struct breakpoint *b)
       sal.pc = prev_pc;
       sal.section = find_pc_overlay (sal.pc);
       sal.explicit_pc = 1;
-      b_return = set_momentary_breakpoint (get_frame_arch (prev_frame), sal,
-					   prev_frame_id,
-					   bp_gnu_ifunc_resolver_return);
+      b_return
+	= set_momentary_breakpoint (get_frame_arch (prev_frame), sal,
+				    prev_frame_id,
+				    bp_gnu_ifunc_resolver_return).release ();
 
       /* set_momentary_breakpoint invalidates PREV_FRAME.  */
       prev_frame = NULL;
diff --git a/gdb/infcall.c b/gdb/infcall.c
index 03749f3dc2..b06758085d 100644
--- a/gdb/infcall.c
+++ b/gdb/infcall.c
@@ -1100,8 +1100,9 @@ call_function_by_hand_dummy (struct value *function,
     /* Sanity.  The exact same SP value is returned by
        PUSH_DUMMY_CALL, saved as the dummy-frame TOS, and used by
        dummy_id to form the frame ID's stack address.  */
-    breakpoint *bpt = set_momentary_breakpoint (gdbarch, sal,
-						dummy_id, bp_call_dummy);
+    breakpoint *bpt
+      = set_momentary_breakpoint (gdbarch, sal,
+				  dummy_id, bp_call_dummy).release ();
 
     /* set_momentary_breakpoint invalidates FRAME.  */
     frame = NULL;
diff --git a/gdb/infcmd.c b/gdb/infcmd.c
index 669631bdb4..49349c169e 100644
--- a/gdb/infcmd.c
+++ b/gdb/infcmd.c
@@ -1986,7 +1986,7 @@ finish_forward (struct finish_command_fsm *sm, struct frame_info *frame)
 
   sm->breakpoint = set_momentary_breakpoint (gdbarch, sal,
 					     get_stack_frame_id (frame),
-					     bp_finish);
+					     bp_finish).release ();
 
   /* set_momentary_breakpoint invalidates FRAME.  */
   frame = NULL;
diff --git a/gdb/infrun.c b/gdb/infrun.c
index ef5a505d89..d425664957 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -7402,7 +7402,7 @@ insert_step_resume_breakpoint_at_sal_1 (struct gdbarch *gdbarch,
 			paddress (gdbarch, sr_sal.pc));
 
   inferior_thread ()->control.step_resume_breakpoint
-    = set_momentary_breakpoint (gdbarch, sr_sal, sr_id, sr_type);
+    = set_momentary_breakpoint (gdbarch, sr_sal, sr_id, sr_type).release ();
 }
 
 void
@@ -7491,7 +7491,7 @@ insert_longjmp_resume_breakpoint (struct gdbarch *gdbarch, CORE_ADDR pc)
 			paddress (gdbarch, pc));
 
   inferior_thread ()->control.exception_resume_breakpoint =
-    set_momentary_breakpoint_at_pc (gdbarch, pc, bp_longjmp_resume);
+    set_momentary_breakpoint_at_pc (gdbarch, pc, bp_longjmp_resume).release ();
 }
 
 /* Insert an exception resume breakpoint.  TP is the thread throwing
@@ -7526,7 +7526,8 @@ insert_exception_resume_breakpoint (struct thread_info *tp,
 				(unsigned long) handler);
 
 	  bp = set_momentary_breakpoint_at_pc (get_frame_arch (frame),
-					       handler, bp_exception_resume);
+					       handler,
+					       bp_exception_resume).release ();
 
 	  /* set_momentary_breakpoint_at_pc invalidates FRAME.  */
 	  frame = NULL;
@@ -7567,7 +7568,7 @@ insert_exception_resume_from_probe (struct thread_info *tp,
 				  handler));
 
   bp = set_momentary_breakpoint_at_pc (get_frame_arch (frame),
-				       handler, bp_exception_resume);
+				       handler, bp_exception_resume).release ();
   bp->thread = tp->global_num;
   inferior_thread ()->control.exception_resume_breakpoint = bp;
 }
-- 
2.13.6

Re: [RFA 12/13] Introduce gdb_breakpoint_up

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> This introduces gdb_breakpoint_up, a unique_ptr typedef that owns a
> breakpoint.  It then changes set_momentary_breakpoint to return a
> gdb_breakpoint_up and fixes up the fallout.  This then allows the
> removal of make_cleanup_delete_breakpoint.
> 
> Once breakpoints are fully C++-ified, this typedef can be removed in
> favor of a plain std::unique_ptr.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* breakpoint.c (set_momentary_breakpoint): Return
> 	gdb_breakpoint_up.
> 	(until_break_command): Update.
> 	(new_until_break_fsm): Change argument types to
> 	gdb_breakpoint_up.
> 	(set_momentary_breakpoint_at_pc): Return gdb_breakpoint_up.
> 	(do_delete_breakpoint_cleanup, make_cleanup_delete_breakpoint):
> 	Remove.
> 	* infcmd.c (finish_forward): Update.
> 	* breakpoint.h (set_momentary_breakpoint)
> 	(set_momentary_breakpoint_at_pc): Return gdb_breakpoint_up.
> 	(make_cleanup_delete_breakpoint): Remove.
> 	(struct gdb_breakpoint_deleter): New.
> 	(gdb_breakpoint_up): New typedef.
> 	* infrun.c (insert_step_resume_breakpoint_at_sal_1): Update.
> 	(insert_exception_resume_breakpoint): Update.
> 	(insert_exception_resume_from_probe): Update.
> 	(insert_longjmp_resume_breakpoint): Update.
> 	* arm-linux-tdep.c (arm_linux_copy_svc): Update.
> 	* elfread.c (elf_gnu_ifunc_resolver_stop): Update.
> 	* infcall.c (call_function_by_hand_dummy): Update

Just a tiny nit, why not name the type "breakpoint_up"?  So far the 
convention has been ${type_name}_up.

Otherwise, LGTM.

Simon

Re: [RFA 12/13] Introduce gdb_breakpoint_up

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Just a tiny nit, why not name the type "breakpoint_up"?  So far the
Simon> convention has been ${type_name}_up.

I made this change.

Tom

[RFA 05/13] Remove directive-searched cleanups

[Tom Tromey]

This removes a few cleanups related to the "searched" field in
struct using_direct, replacing these with scoped_restore.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* cp-namespace.c (reset_directive_searched): Remove.
	(cp_lookup_symbol_via_imports): Use scoped_restore.
	* cp-support.c (reset_directive_searched): Remove.
	(make_symbol_overload_list_using): Use scoped_restore.
	* d-namespace.c (d_lookup_symbol_imports): Use scoped_restore.
	(reset_directive_searched): Remove.
---
 gdb/ChangeLog      |  9 +++++++++
 gdb/cp-namespace.c | 25 +++----------------------
 gdb/cp-support.c   | 19 ++-----------------
 gdb/d-namespace.c  | 26 +++-----------------------
 4 files changed, 17 insertions(+), 62 deletions(-)

diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
index 214b7e1cf4..d8817c0372 100644
--- a/gdb/cp-namespace.c
+++ b/gdb/cp-namespace.c
@@ -338,15 +338,6 @@ cp_lookup_symbol_in_namespace (const char *the_namespace, const char *name,
   return sym;
 }
 
-/* Used for cleanups to reset the "searched" flag in case of an error.  */
-
-static void
-reset_directive_searched (void *data)
-{
-  struct using_direct *direct = (struct using_direct *) data;
-  direct->searched = 0;
-}
-
 /* Search for NAME by applying all import statements belonging to
    BLOCK which are applicable in SCOPE.  If DECLARATION_ONLY the
    search is restricted to using declarations.
@@ -388,7 +379,6 @@ cp_lookup_symbol_via_imports (const char *scope,
   struct block_symbol sym;
   int len;
   int directive_match;
-  struct cleanup *searched_cleanup;
 
   sym.symbol = NULL;
   sym.block = NULL;
@@ -425,9 +415,8 @@ cp_lookup_symbol_via_imports (const char *scope,
 	{
 	  /* Mark this import as searched so that the recursive call
 	     does not search it again.  */
-	  current->searched = 1;
-	  searched_cleanup = make_cleanup (reset_directive_searched,
-					   current);
+	  scoped_restore reset_directive_searched
+	    = make_scoped_restore (&current->searched, 1);
 
 	  /* If there is an import of a single declaration, compare the
 	     imported declaration (after optional renaming by its alias)
@@ -446,9 +435,6 @@ cp_lookup_symbol_via_imports (const char *scope,
 	     search of this import is complete.  */
 	  if (declaration_only || sym.symbol != NULL || current->declaration)
 	    {
-	      current->searched = 0;
-	      discard_cleanups (searched_cleanup);
-
 	      if (sym.symbol != NULL)
 		return sym;
 
@@ -460,10 +446,7 @@ cp_lookup_symbol_via_imports (const char *scope,
 	    if (strcmp (name, *excludep) == 0)
 	      break;
 	  if (*excludep)
-	    {
-	      discard_cleanups (searched_cleanup);
-	      continue;
-	    }
+	    continue;
 
 	  if (current->alias != NULL
 	      && strcmp (name, current->alias) == 0)
@@ -484,8 +467,6 @@ cp_lookup_symbol_via_imports (const char *scope,
 						  name, block,
 						  domain, 1, 0, 0);
 	    }
-	  current->searched = 0;
-	  discard_cleanups (searched_cleanup);
 
 	  if (sym.symbol != NULL)
 	    return sym;
diff --git a/gdb/cp-support.c b/gdb/cp-support.c
index 37b2b4af97..817de59f12 100644
--- a/gdb/cp-support.c
+++ b/gdb/cp-support.c
@@ -1286,16 +1286,6 @@ make_symbol_overload_list_adl (struct type **arg_types, int nargs,
   return sym_return_val;
 }
 
-/* Used for cleanups to reset the "searched" flag in case of an
-   error.  */
-
-static void
-reset_directive_searched (void *data)
-{
-  struct using_direct *direct = (struct using_direct *) data;
-  direct->searched = 0;
-}
-
 /* This applies the using directives to add namespaces to search in,
    and then searches for overloads in all of those namespaces.  It
    adds the symbols found to sym_return_val.  Arguments are as in
@@ -1332,16 +1322,11 @@ make_symbol_overload_list_using (const char *func_name,
 	  {
 	    /* Mark this import as searched so that the recursive call
 	       does not search it again.  */
-	    struct cleanup *old_chain;
-	    current->searched = 1;
-	    old_chain = make_cleanup (reset_directive_searched,
-				      current);
+	    scoped_restore reset_directive_searched
+	      = make_scoped_restore (&current->searched, 1);
 
 	    make_symbol_overload_list_using (func_name,
 					     current->import_src);
-
-	    current->searched = 0;
-	    discard_cleanups (old_chain);
 	  }
       }
 
diff --git a/gdb/d-namespace.c b/gdb/d-namespace.c
index bc791f70dc..6842a29fac 100644
--- a/gdb/d-namespace.c
+++ b/gdb/d-namespace.c
@@ -356,16 +356,6 @@ d_lookup_nested_symbol (struct type *parent_type,
     }
 }
 
-/* Used for cleanups to reset the "searched" flag incase
-   of an error.  */
-
-static void
-reset_directive_searched (void *data)
-{
-  struct using_direct *direct = (struct using_direct *) data;
-  direct->searched = 0;
-}
-
 /* Search for NAME by applying all import statements belonging to
    BLOCK which are applicable in SCOPE.  */
 
@@ -376,7 +366,6 @@ d_lookup_symbol_imports (const char *scope, const char *name,
 {
   struct using_direct *current;
   struct block_symbol sym;
-  struct cleanup *searched_cleanup;
 
   /* First, try to find the symbol in the given module.  */
   sym = d_lookup_symbol_in_module (scope, name, block, domain, 1);
@@ -399,9 +388,8 @@ d_lookup_symbol_imports (const char *scope, const char *name,
 	{
 	  /* Mark this import as searched so that the recursive call
 	     does not search it again.  */
-	  current->searched = 1;
-	  searched_cleanup = make_cleanup (reset_directive_searched,
-					   current);
+	  scoped_restore restore_searched
+	    = make_scoped_restore (&current->searched, 1);
 
 	  /* If there is an import of a single declaration, compare the
 	     imported declaration (after optional renaming by its alias)
@@ -419,9 +407,6 @@ d_lookup_symbol_imports (const char *scope, const char *name,
 	     declaration, the search of this import is complete.  */
 	  if (sym.symbol != NULL || current->declaration)
 	    {
-	      current->searched = 0;
-	      discard_cleanups (searched_cleanup);
-
 	      if (sym.symbol != NULL)
 		return sym;
 
@@ -433,10 +418,7 @@ d_lookup_symbol_imports (const char *scope, const char *name,
 	    if (strcmp (name, *excludep) == 0)
 	      break;
 	  if (*excludep)
-	    {
-	      discard_cleanups (searched_cleanup);
-	      continue;
-	    }
+	    continue;
 
 	  /* If the import statement is creating an alias.  */
 	  if (current->alias != NULL)
@@ -476,8 +458,6 @@ d_lookup_symbol_imports (const char *scope, const char *name,
 	      sym = d_lookup_symbol_in_module (current->import_src,
 					       name, block, domain, 1);
 	    }
-	  current->searched = 0;
-	  discard_cleanups (searched_cleanup);
 
 	  if (sym.symbol != NULL)
 	    return sym;
-- 
2.13.6

Re: [RFA 05/13] Remove directive-searched cleanups

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> This removes a few cleanups related to the "searched" field in
> struct using_direct, replacing these with scoped_restore.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* cp-namespace.c (reset_directive_searched): Remove.
> 	(cp_lookup_symbol_via_imports): Use scoped_restore.
> 	* cp-support.c (reset_directive_searched): Remove.
> 	(make_symbol_overload_list_using): Use scoped_restore.
> 	* d-namespace.c (d_lookup_symbol_imports): Use scoped_restore.
> 	(reset_directive_searched): Remove.
> ---
>  gdb/ChangeLog      |  9 +++++++++
>  gdb/cp-namespace.c | 25 +++----------------------
>  gdb/cp-support.c   | 19 ++-----------------
>  gdb/d-namespace.c  | 26 +++-----------------------
>  4 files changed, 17 insertions(+), 62 deletions(-)
> 
> diff --git a/gdb/cp-namespace.c b/gdb/cp-namespace.c
> index 214b7e1cf4..d8817c0372 100644
> --- a/gdb/cp-namespace.c
> +++ b/gdb/cp-namespace.c
> @@ -338,15 +338,6 @@ cp_lookup_symbol_in_namespace (const char
> *the_namespace, const char *name,
>    return sym;
>  }
> 
> -/* Used for cleanups to reset the "searched" flag in case of an error. 
>  */
> -
> -static void
> -reset_directive_searched (void *data)
> -{
> -  struct using_direct *direct = (struct using_direct *) data;
> -  direct->searched = 0;
> -}
> -
>  /* Search for NAME by applying all import statements belonging to
>     BLOCK which are applicable in SCOPE.  If DECLARATION_ONLY the
>     search is restricted to using declarations.
> @@ -388,7 +379,6 @@ cp_lookup_symbol_via_imports (const char *scope,
>    struct block_symbol sym;
>    int len;
>    int directive_match;
> -  struct cleanup *searched_cleanup;
> 
>    sym.symbol = NULL;
>    sym.block = NULL;
> @@ -425,9 +415,8 @@ cp_lookup_symbol_via_imports (const char *scope,
>  	{
>  	  /* Mark this import as searched so that the recursive call
>  	     does not search it again.  */
> -	  current->searched = 1;
> -	  searched_cleanup = make_cleanup (reset_directive_searched,
> -					   current);
> +	  scoped_restore reset_directive_searched
> +	    = make_scoped_restore (&current->searched, 1);
> 
>  	  /* If there is an import of a single declaration, compare the
>  	     imported declaration (after optional renaming by its alias)
> @@ -446,9 +435,6 @@ cp_lookup_symbol_via_imports (const char *scope,
>  	     search of this import is complete.  */
>  	  if (declaration_only || sym.symbol != NULL || current->declaration)
>  	    {
> -	      current->searched = 0;
> -	      discard_cleanups (searched_cleanup);
> -
>  	      if (sym.symbol != NULL)
>  		return sym;
> 
> @@ -460,10 +446,7 @@ cp_lookup_symbol_via_imports (const char *scope,
>  	    if (strcmp (name, *excludep) == 0)
>  	      break;
>  	  if (*excludep)
> -	    {
> -	      discard_cleanups (searched_cleanup);
> -	      continue;
> -	    }
> +	    continue;

In this case, the cleanup is discarded.  Shouldn't the same thing happen 
with the scoped_restore?  Or was it an error in the original code, and 
we always want to reset searched?

There's the same case in d-namespace.c.

Simon

Re: [RFA 05/13] Remove directive-searched cleanups

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

>> if (*excludep)
>> -	    {
>> -	      discard_cleanups (searched_cleanup);
>> -	      continue;
>> -	    }
>> +	    continue;

Simon> In this case, the cleanup is discarded.  Shouldn't the same thing
Simon> happen with the scoped_restore?  Or was it an error in the original
Simon> code, and we always want to reset searched?

Simon> There's the same case in d-namespace.c.

I'm afraid I hadn't noticed this.  Ouch.

However, I think it has to be a latent bug, because it will result in
the using_direct's searched flag being permanently set -- nothing else
will ever clear it.

Tom

Re: [RFA 05/13] Remove directive-searched cleanups

[Simon Marchi]

On 2017-11-03 12:42, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:
> 
>>> if (*excludep)
>>> -	    {
>>> -	      discard_cleanups (searched_cleanup);
>>> -	      continue;
>>> -	    }
>>> +	    continue;
> 
> Simon> In this case, the cleanup is discarded.  Shouldn't the same 
> thing
> Simon> happen with the scoped_restore?  Or was it an error in the 
> original
> Simon> code, and we always want to reset searched?
> 
> Simon> There's the same case in d-namespace.c.
> 
> I'm afraid I hadn't noticed this.  Ouch.
> 
> However, I think it has to be a latent bug, because it will result in
> the using_direct's searched flag being permanently set -- nothing else
> will ever clear it.
> 
> Tom

That's what my assumption was.  From what I understand, we want to set 
the flag while we recurse and always restore it when we return.

Simon

[RFA 07/13] Use gdb::def_vector in sparc64-tdep.c

[Tom Tromey]

This removes a cleanup from sparc64-tdep.c, replacing it with
gdb::def_vector.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* sparc64-tdep.c (do_examine): Use gdb::def_vector.
---
 gdb/ChangeLog      |  4 ++++
 gdb/sparc64-tdep.c | 10 +++-------
 2 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index a756834cba..eebfbdf96f 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -415,21 +415,17 @@ static void
 do_examine (CORE_ADDR start, int bcnt)
 {
   CORE_ADDR vaddr = adi_normalize_address (start);
-  struct cleanup *cleanup;
 
   CORE_ADDR vstart = adi_align_address (vaddr);
   int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
-  unsigned char *buf = (unsigned char *) xmalloc (cnt);
-  cleanup = make_cleanup (xfree, buf);
-  int read_cnt = adi_read_versions (vstart, cnt, buf);
+  gdb::def_vector<unsigned char> buf (cnt);
+  int read_cnt = adi_read_versions (vstart, cnt, buf.data ());
   if (read_cnt == -1)
     error (_("No ADI information"));
   else if (read_cnt < cnt)
     error(_("No ADI information at %s"), paddress (target_gdbarch (), vaddr));
 
-  adi_print_versions (vstart, cnt, buf);
-
-  do_cleanups (cleanup);
+  adi_print_versions (vstart, cnt, buf.data ());
 }
 
 static void
-- 
2.13.6

Re: [RFA 07/13] Use gdb::def_vector in sparc64-tdep.c

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> This removes a cleanup from sparc64-tdep.c, replacing it with
> gdb::def_vector.
> 
> gdb/ChangeLog
> 2017-11-02  Tom Tromey  <tom@tromey.com>
> 
> 	* sparc64-tdep.c (do_examine): Use gdb::def_vector.
> ---
>  gdb/ChangeLog      |  4 ++++
>  gdb/sparc64-tdep.c | 10 +++-------
>  2 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
> index a756834cba..eebfbdf96f 100644
> --- a/gdb/sparc64-tdep.c
> +++ b/gdb/sparc64-tdep.c
> @@ -415,21 +415,17 @@ static void
>  do_examine (CORE_ADDR start, int bcnt)
>  {
>    CORE_ADDR vaddr = adi_normalize_address (start);
> -  struct cleanup *cleanup;
> 
>    CORE_ADDR vstart = adi_align_address (vaddr);
>    int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
> -  unsigned char *buf = (unsigned char *) xmalloc (cnt);
> -  cleanup = make_cleanup (xfree, buf);
> -  int read_cnt = adi_read_versions (vstart, cnt, buf);
> +  gdb::def_vector<unsigned char> buf (cnt);
> +  int read_cnt = adi_read_versions (vstart, cnt, buf.data ());
>    if (read_cnt == -1)
>      error (_("No ADI information"));
>    else if (read_cnt < cnt)
>      error(_("No ADI information at %s"), paddress (target_gdbarch (), 
> vaddr));
> 
> -  adi_print_versions (vstart, cnt, buf);
> -
> -  do_cleanups (cleanup);
> +  adi_print_versions (vstart, cnt, buf.data ());
>  }
> 
>  static void

It seems to me like the code doesn't use this as text, but binary data.  
So it should probably have used gdb_byte in the first place.  I would 
then suggest using gdb::byte_vector (if you agree that gdb_byte should 
be used here).

Simon

Re: [RFA 07/13] Use gdb::def_vector in sparc64-tdep.c

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> It seems to me like the code doesn't use this as text, but binary
Simon> data.  So it should probably have used gdb_byte in the first place.  I
Simon> would then suggest using gdb::byte_vector (if you agree that gdb_byte
Simon> should be used here).

For consistency I changed the callees as well.

Tom

commit 0cd1a0cc08838934771cb8c4aa556dde7ee66f01
Author: Tom Tromey <tom@tromey.com>
Date:   Wed Nov 1 16:37:27 2017 -0600

    Use gdb::def_vector in sparc64-tdep.c
    
    This removes a cleanup from sparc64-tdep.c, replacing it with
    gdb::def_vector.
    
    gdb/ChangeLog
    2017-11-03  Tom Tromey  <tom@tromey.com>
    
            * sparc64-tdep.c (do_examine): Use gdb::def_vector.
            (adi_read_versions): Change "tags" to "gdb_byte *".
            (adi_print_versions): Likewise.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 56b7e903bc..8f663938cb 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-11-03  Tom Tromey  <tom@tromey.com>
 
+	* sparc64-tdep.c (do_examine): Use gdb::def_vector.
+	(adi_read_versions): Change "tags" to "gdb_byte *".
+	(adi_print_versions): Likewise.
+
+2017-11-03  Tom Tromey  <tom@tromey.com>
+
 	* breakpoint.c
 	(scoped_rbreak_breakpoints::scoped_rbreak_breakpoints): Rename
 	from start_rbreak_breakpoints.
diff --git a/gdb/sparc64-tdep.c b/gdb/sparc64-tdep.c
index a756834cba..eade0461d5 100644
--- a/gdb/sparc64-tdep.c
+++ b/gdb/sparc64-tdep.c
@@ -341,7 +341,7 @@ adi_is_addr_mapped (CORE_ADDR vaddr, size_t cnt)
    for "SIZE" number of bytes.  */
 
 static int
-adi_read_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
+adi_read_versions (CORE_ADDR vaddr, size_t size, gdb_byte *tags)
 {
   int fd = adi_tag_fd ();
   if (fd == -1)
@@ -383,7 +383,7 @@ adi_write_versions (CORE_ADDR vaddr, size_t size, unsigned char *tags)
    at "VADDR" with number of "CNT".  */
 
 static void
-adi_print_versions (CORE_ADDR vaddr, size_t cnt, unsigned char *tags)
+adi_print_versions (CORE_ADDR vaddr, size_t cnt, gdb_byte *tags)
 {
   int v_idx = 0;
   const int maxelts = 8;  /* # of elements per line */
@@ -415,21 +415,17 @@ static void
 do_examine (CORE_ADDR start, int bcnt)
 {
   CORE_ADDR vaddr = adi_normalize_address (start);
-  struct cleanup *cleanup;
 
   CORE_ADDR vstart = adi_align_address (vaddr);
   int cnt = adi_convert_byte_count (vaddr, bcnt, vstart);
-  unsigned char *buf = (unsigned char *) xmalloc (cnt);
-  cleanup = make_cleanup (xfree, buf);
-  int read_cnt = adi_read_versions (vstart, cnt, buf);
+  gdb::def_vector<gdb_byte> buf (cnt);
+  int read_cnt = adi_read_versions (vstart, cnt, buf.data ());
   if (read_cnt == -1)
     error (_("No ADI information"));
   else if (read_cnt < cnt)
     error(_("No ADI information at %s"), paddress (target_gdbarch (), vaddr));
 
-  adi_print_versions (vstart, cnt, buf);
-
-  do_cleanups (cleanup);
+  adi_print_versions (vstart, cnt, buf.data ());
 }
 
 static void

[RFA 04/13] Use unique_xmalloc_ptr in find_separate_debug_file_by_debuglink

[Tom Tromey]

This changes find_separate_debug_file_by_debuglink to use
unique_xmalloc_ptr, removing some cleanups.

gdb/ChangeLog
2017-11-02  Tom Tromey  <tom@tromey.com>

	* symfile.c (find_separate_debug_file_by_debuglink): Use
	unique_xmalloc_ptr.
---
 gdb/ChangeLog |  5 +++++
 gdb/symfile.c | 36 ++++++++++++++----------------------
 2 files changed, 19 insertions(+), 22 deletions(-)

diff --git a/gdb/symfile.c b/gdb/symfile.c
index 69358e44ce..fb63441ac0 100644
--- a/gdb/symfile.c
+++ b/gdb/symfile.c
@@ -1545,13 +1545,11 @@ terminate_after_last_dir_separator (char *path)
 char *
 find_separate_debug_file_by_debuglink (struct objfile *objfile)
 {
-  char *debuglink;
-  char *dir, *canon_dir;
   char *debugfile;
   unsigned long crc32;
-  struct cleanup *cleanups;
 
-  debuglink = bfd_get_debug_link_info (objfile->obfd, &crc32);
+  gdb::unique_xmalloc_ptr<char> debuglink
+    (bfd_get_debug_link_info (objfile->obfd, &crc32));
 
   if (debuglink == NULL)
     {
@@ -1560,15 +1558,12 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
       return NULL;
     }
 
-  cleanups = make_cleanup (xfree, debuglink);
-  dir = xstrdup (objfile_name (objfile));
-  make_cleanup (xfree, dir);
-  terminate_after_last_dir_separator (dir);
-  canon_dir = lrealpath (dir);
+  std::string dir = objfile_name (objfile);
+  terminate_after_last_dir_separator (&dir[0]);
+  gdb::unique_xmalloc_ptr<char> canon_dir (lrealpath (dir.c_str ()));
 
-  debugfile = find_separate_debug_file (dir, canon_dir, debuglink,
-					crc32, objfile);
-  xfree (canon_dir);
+  debugfile = find_separate_debug_file (dir.c_str (), canon_dir.get (),
+					debuglink.get (), crc32, objfile);
 
   if (debugfile == NULL)
     {
@@ -1580,19 +1575,17 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
       if (lstat (objfile_name (objfile), &st_buf) == 0
 	  && S_ISLNK (st_buf.st_mode))
 	{
-	  char *symlink_dir;
-
-	  symlink_dir = lrealpath (objfile_name (objfile));
+	  gdb::unique_xmalloc_ptr<char> symlink_dir
+	    (lrealpath (objfile_name (objfile)));
 	  if (symlink_dir != NULL)
 	    {
-	      make_cleanup (xfree, symlink_dir);
-	      terminate_after_last_dir_separator (symlink_dir);
-	      if (strcmp (dir, symlink_dir) != 0)
+	      terminate_after_last_dir_separator (symlink_dir.get ());
+	      if (strcmp (dir.c_str (), symlink_dir.get ()) != 0)
 		{
 		  /* Different directory, so try using it.  */
-		  debugfile = find_separate_debug_file (symlink_dir,
-							symlink_dir,
-							debuglink,
+		  debugfile = find_separate_debug_file (symlink_dir.get (),
+							symlink_dir.get (),
+							debuglink.get (),
 							crc32,
 							objfile);
 		}
@@ -1600,7 +1593,6 @@ find_separate_debug_file_by_debuglink (struct objfile *objfile)
 	}
     }
 
-  do_cleanups (cleanups);
   return debugfile;
 }
 
-- 
2.13.6

Re: [RFA 04/13] Use unique_xmalloc_ptr in find_separate_debug_file_by_debuglink

[Simon Marchi]

On 2017-11-02 18:36, Tom Tromey wrote:
> @@ -1580,19 +1575,17 @@ find_separate_debug_file_by_debuglink (struct
> objfile *objfile)
>        if (lstat (objfile_name (objfile), &st_buf) == 0
>  	  && S_ISLNK (st_buf.st_mode))
>  	{
> -	  char *symlink_dir;
> -
> -	  symlink_dir = lrealpath (objfile_name (objfile));
> +	  gdb::unique_xmalloc_ptr<char> symlink_dir
> +	    (lrealpath (objfile_name (objfile)));
>  	  if (symlink_dir != NULL)
>  	    {
> -	      make_cleanup (xfree, symlink_dir);
> -	      terminate_after_last_dir_separator (symlink_dir);
> -	      if (strcmp (dir, symlink_dir) != 0)
> +	      terminate_after_last_dir_separator (symlink_dir.get ());
> +	      if (strcmp (dir.c_str (), symlink_dir.get ()) != 0)

Just one tiny nit: when possible, I like to replace strcmp with 
std::string's operator==

   if (dir == symlink_dir.get ())

but I'll leave it up to you.

Simon

Re: [RFA 04/13] Use unique_xmalloc_ptr in find_separate_debug_file_by_debuglink

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Just one tiny nit: when possible, I like to replace strcmp with
Simon> std::string's operator==
Simon>   if (dir == symlink_dir.get ())
Simon> but I'll leave it up to you.

Good idea, I made this change; though I think it should be != due to:

> +	      if (strcmp (dir.c_str (), symlink_dir.get ()) != 0)

Tom

Re: [RFA 00/13] more cleanup removal

[Simon Marchi]

On 2017-11-02 18:35, Tom Tromey wrote:
> This series removes some more cleanups.  Most of the patches are
> straightforward.
> 
> Tested by the buildbot.  However, the buildbot doesn't really test
> some of these changes (e.g, h8300-tdep.c); so be sure to examine these
> more carefully.
> 
> Tom

Thanks!  The patches I did not reply too LGTM.

Simon

Re: [RFA 00/13] more cleanup removal

[Tom Tromey]

>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Thanks!  The patches I did not reply too LGTM.

Thanks.

I've made all the requested changes, and I sent the updated series
through the buildbot again, so I am checking it in.

Tom