[RFA 3/9] Return unique_xmalloc_ptr from macro scope functions

[RFA 3/9] Return unique_xmalloc_ptr from macro scope functions

[Tom Tromey]

This changes the macro scope functions (sal_macro_scope,
user_macro_scope, and default_macro_scope) to return a
unique_xmalloc_ptr, then fixes up the users.  This allowed for the
removal of several cleanups.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* symtab.c (default_collect_symbol_completion_matches_break_on):
	Use unique_xmalloc_ptr.
	* macroscope.h: (sal_macro_scope, user_macro_scope)
	(default_macro_scope): Return unique_xmalloc_ptr.
	* macroscope.c (sal_macro_scope, user_macro_scope)
	(default_macro_scope): Return unique_xmalloc_ptr.
	* macroexp.h (macro_expand, macro_expand_once): Return
	unique_xmalloc_ptr.
	* macroexp.c (macro_expand, macro_expand_once): Return
	unique_xmalloc_ptr.
	* macrocmd.c (macro_expand_command, macro_expand_once_command)
	(info_macro_command, info_macros_command): Use
	unique_xmalloc_ptr.
	* compile/compile-c-support.c (write_macro_definitions): Use
	unique_xmalloc_ptr.
	* c-exp.y (c_parse): Use unique_xmalloc_ptr.
---
 gdb/ChangeLog                   | 19 +++++++++++++++++++
 gdb/c-exp.y                     | 24 +++++++++++-------------
 gdb/compile/compile-c-support.c |  2 +-
 gdb/macrocmd.c                  | 38 ++++++++++----------------------------
 gdb/macroexp.c                  |  6 +++---
 gdb/macroexp.h                  | 12 ++++++------
 gdb/macroscope.c                | 15 ++++++---------
 gdb/macroscope.h                | 15 ++++++---------
 gdb/symtab.c                    |  9 +++------
 9 files changed, 65 insertions(+), 75 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 00ed7e78c2..abb6126864 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,22 @@
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
+	* symtab.c (default_collect_symbol_completion_matches_break_on):
+	Use unique_xmalloc_ptr.
+	* macroscope.h: (sal_macro_scope, user_macro_scope)
+	(default_macro_scope): Return unique_xmalloc_ptr.
+	* macroscope.c (sal_macro_scope, user_macro_scope)
+	(default_macro_scope): Return unique_xmalloc_ptr.
+	* macroexp.h (macro_expand, macro_expand_once): Return
+	unique_xmalloc_ptr.
+	* macroexp.c (macro_expand, macro_expand_once): Return
+	unique_xmalloc_ptr.
+	* macrocmd.c (macro_expand_command, macro_expand_once_command)
+	(info_macro_command, info_macros_command): Use
+	unique_xmalloc_ptr.
+	* compile/compile-c-support.c (write_macro_definitions): Use
+	unique_xmalloc_ptr.
+	* c-exp.y (c_parse): Use unique_xmalloc_ptr.
+
 2018-02-07  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* value.c (value_static_field): Assign field type instead of
diff --git a/gdb/c-exp.y b/gdb/c-exp.y
index 8f0aa00f52..8dc3c068a5 100644
--- a/gdb/c-exp.y
+++ b/gdb/c-exp.y
@@ -3220,26 +3220,24 @@ c_parse (struct parser_state *par_state)
   gdb_assert (par_state != NULL);
   pstate = par_state;
 
-  /* Note that parsing (within yyparse) freely installs cleanups
-     assuming they'll be run here (below).  */
-
-  back_to = make_cleanup (free_current_contents, &expression_macro_scope);
-
-  /* Set up the scope for macro expansion.  */
-  expression_macro_scope = NULL;
+  gdb::unique_xmalloc_ptr<struct macro_scope> macro_scope;
 
   if (expression_context_block)
-    expression_macro_scope
-      = sal_macro_scope (find_pc_line (expression_context_pc, 0));
+    macro_scope = sal_macro_scope (find_pc_line (expression_context_pc, 0));
   else
-    expression_macro_scope = default_macro_scope ();
-  if (! expression_macro_scope)
-    expression_macro_scope = user_macro_scope ();
+    macro_scope = default_macro_scope ();
+  if (! macro_scope)
+    macro_scope = user_macro_scope ();
+
+  scoped_restore restore_macro_scope
+    = make_scoped_restore (&expression_macro_scope, macro_scope.get ());
 
   /* Initialize macro expansion code.  */
   obstack_init (&expansion_obstack);
   gdb_assert (! macro_original_text);
-  make_cleanup (scan_macro_cleanup, 0);
+  /* Note that parsing (within yyparse) freely installs cleanups
+     assuming they'll be run here (below).  */
+  back_to = make_cleanup (scan_macro_cleanup, 0);
 
   scoped_restore restore_yydebug = make_scoped_restore (&yydebug,
 							parser_debug);
diff --git a/gdb/compile/compile-c-support.c b/gdb/compile/compile-c-support.c
index 006d3f811c..e694648288 100644
--- a/gdb/compile/compile-c-support.c
+++ b/gdb/compile/compile-c-support.c
@@ -157,7 +157,7 @@ static void
 write_macro_definitions (const struct block *block, CORE_ADDR pc,
 			 struct ui_file *file)
 {
-  struct macro_scope *scope;
+  gdb::unique_xmalloc_ptr<struct macro_scope> scope;
 
   if (block != NULL)
     scope = sal_macro_scope (find_pc_line (pc, 0));
diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index e90d78724d..13fd95d7da 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -55,11 +55,8 @@ macro_inform_no_debuginfo (void)
 static void
 macro_expand_command (const char *exp, int from_tty)
 {
-  struct macro_scope *ms = NULL;
-  char *expanded = NULL;
-  struct cleanup *cleanup_chain = make_cleanup (free_current_contents, &ms);
-
-  make_cleanup (free_current_contents, &expanded);
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms;
+  gdb::unique_xmalloc_ptr<char> expanded;
 
   /* You know, when the user doesn't specify any expression, it would be
      really cool if this defaulted to the last expression evaluated.
@@ -74,26 +71,21 @@ macro_expand_command (const char *exp, int from_tty)
   ms = default_macro_scope ();
   if (ms)
     {
-      expanded = macro_expand (exp, standard_macro_lookup, ms);
+      expanded = macro_expand (exp, standard_macro_lookup, ms.get ());
       fputs_filtered ("expands to: ", gdb_stdout);
-      fputs_filtered (expanded, gdb_stdout);
+      fputs_filtered (expanded.get (), gdb_stdout);
       fputs_filtered ("\n", gdb_stdout);
     }
   else
     macro_inform_no_debuginfo ();
-
-  do_cleanups (cleanup_chain);
-  return;
 }
 
 
 static void
 macro_expand_once_command (const char *exp, int from_tty)
 {
-  struct macro_scope *ms = NULL;
-  char *expanded = NULL;
-  struct cleanup *cleanup_chain = make_cleanup (free_current_contents, &ms);
-  make_cleanup (free_current_contents, &expanded);
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms;
+  gdb::unique_xmalloc_ptr<char> expanded;
 
   /* You know, when the user doesn't specify any expression, it would be
      really cool if this defaulted to the last expression evaluated.
@@ -108,16 +100,13 @@ macro_expand_once_command (const char *exp, int from_tty)
   ms = default_macro_scope ();
   if (ms)
     {
-      expanded = macro_expand_once (exp, standard_macro_lookup, ms);
+      expanded = macro_expand_once (exp, standard_macro_lookup, ms.get ());
       fputs_filtered ("expands to: ", gdb_stdout);
-      fputs_filtered (expanded, gdb_stdout);
+      fputs_filtered (expanded.get (), gdb_stdout);
       fputs_filtered ("\n", gdb_stdout);
     }
   else
     macro_inform_no_debuginfo ();
-
-  do_cleanups (cleanup_chain);
-  return;
 }
 
 /*  Outputs the include path of a macro starting at FILE and LINE to STREAM.
@@ -190,8 +179,7 @@ print_macro_definition (const char *name,
 static void
 info_macro_command (const char *args, int from_tty)
 {
-  struct macro_scope *ms = NULL;
-  struct cleanup *cleanup_chain;
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms;
   const char *name;
   int show_all_macros_named = 0;
   const char *arg_start = args;
@@ -228,7 +216,6 @@ info_macro_command (const char *args, int from_tty)
 	     "whose definition you want to see."));
 
   ms = default_macro_scope ();
-  cleanup_chain = make_cleanup (free_current_contents, &ms);
 
   if (! ms)
     macro_inform_no_debuginfo ();
@@ -263,16 +250,13 @@ info_macro_command (const char *args, int from_tty)
           show_pp_source_pos (gdb_stdout, ms->file, ms->line);
 	}
     }
-
-  do_cleanups (cleanup_chain);
 }
 
 /* Implementation of the "info macros" command. */
 static void
 info_macros_command (const char *args, int from_tty)
 {
-  struct macro_scope *ms = NULL;
-  struct cleanup *cleanup_chain = make_cleanup (free_current_contents, &ms);
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms;
 
   if (args == NULL)
     ms = default_macro_scope ();
@@ -289,8 +273,6 @@ info_macros_command (const char *args, int from_tty)
     macro_inform_no_debuginfo ();
   else
     macro_for_each_in_scope (ms->file, ms->line, print_macro_definition);
-
-  do_cleanups (cleanup_chain);
 }
 
 \f
diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index 8a2df1edff..aefb13a9d6 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -1485,7 +1485,7 @@ scan (struct macro_buffer *dest,
 }
 
 
-char *
+gdb::unique_xmalloc_ptr<char>
 macro_expand (const char *source,
               macro_lookup_ftype *lookup_func,
               void *lookup_func_baton)
@@ -1504,11 +1504,11 @@ macro_expand (const char *source,
   appendc (&dest, '\0');
 
   discard_cleanups (back_to);
-  return dest.text;
+  return gdb::unique_xmalloc_ptr<char> (dest.text);
 }
 
 
-char *
+gdb::unique_xmalloc_ptr<char>
 macro_expand_once (const char *source,
                    macro_lookup_ftype *lookup_func,
                    void *lookup_func_baton)
diff --git a/gdb/macroexp.h b/gdb/macroexp.h
index f7c74116ce..8d3e3f010f 100644
--- a/gdb/macroexp.h
+++ b/gdb/macroexp.h
@@ -37,9 +37,9 @@ typedef struct macro_definition *(macro_lookup_ftype) (const char *name,
    preprocessor definitions.  SOURCE is a null-terminated string.  The
    result is a null-terminated string, allocated using xmalloc; it is
    the caller's responsibility to free it.  */
-char *macro_expand (const char *source,
-                    macro_lookup_ftype *lookup_func,
-                    void *lookup_func_baton);
+gdb::unique_xmalloc_ptr<char> macro_expand (const char *source,
+					    macro_lookup_ftype *lookup_func,
+					    void *lookup_func_baton);
 
 
 /* Expand all preprocessor macro references that appear explicitly in
@@ -49,9 +49,9 @@ char *macro_expand (const char *source,
    SOURCE is a null-terminated string.  The result is a
    null-terminated string, allocated using xmalloc; it is the caller's
    responsibility to free it.  */
-char *macro_expand_once (const char *source,
-                         macro_lookup_ftype *lookup_func,
-                         void *lookup_func_baton);
+gdb::unique_xmalloc_ptr<char> macro_expand_once (const char *source,
+						 macro_lookup_ftype *lookup_func,
+						 void *lookup_func_baton);
 
 
 /* If the null-terminated string pointed to by *LEXPTR begins with a
diff --git a/gdb/macroscope.c b/gdb/macroscope.c
index a94492011d..23d2e50fdd 100644
--- a/gdb/macroscope.c
+++ b/gdb/macroscope.c
@@ -35,11 +35,10 @@
 struct macro_table *macro_user_macros;
 
 
-struct macro_scope *
+gdb::unique_xmalloc_ptr<struct macro_scope>
 sal_macro_scope (struct symtab_and_line sal)
 {
   struct macro_source_file *main_file, *inclusion;
-  struct macro_scope *ms;
   struct compunit_symtab *cust;
 
   if (sal.symtab == NULL)
@@ -48,7 +47,7 @@ sal_macro_scope (struct symtab_and_line sal)
   if (COMPUNIT_MACRO_TABLE (cust) == NULL)
     return NULL;
 
-  ms = XNEW (struct macro_scope);
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms (XNEW (struct macro_scope));
 
   main_file = macro_main (COMPUNIT_MACRO_TABLE (cust));
   inclusion = macro_lookup_inclusion (main_file, sal.symtab->filename);
@@ -87,22 +86,20 @@ sal_macro_scope (struct symtab_and_line sal)
 }
 
 
-struct macro_scope *
+gdb::unique_xmalloc_ptr<struct macro_scope>
 user_macro_scope (void)
 {
-  struct macro_scope *ms;
-
-  ms = XNEW (struct macro_scope);
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms (XNEW (struct macro_scope));
   ms->file = macro_main (macro_user_macros);
   ms->line = -1;
   return ms;
 }
 
-struct macro_scope *
+gdb::unique_xmalloc_ptr<struct macro_scope>
 default_macro_scope (void)
 {
   struct symtab_and_line sal;
-  struct macro_scope *ms;
+  gdb::unique_xmalloc_ptr<struct macro_scope> ms;
   struct frame_info *frame;
   CORE_ADDR pc;
 
diff --git a/gdb/macroscope.h b/gdb/macroscope.h
index 3bd1c9e817..00fa98ab6e 100644
--- a/gdb/macroscope.h
+++ b/gdb/macroscope.h
@@ -39,13 +39,13 @@ struct macro_scope {
 /* Return a `struct macro_scope' object corresponding to the symtab
    and line given in SAL.  If we have no macro information for that
    location, or if SAL's pc is zero, return zero.  */
-struct macro_scope *sal_macro_scope (struct symtab_and_line sal);
+gdb::unique_xmalloc_ptr<struct macro_scope> sal_macro_scope
+    (struct symtab_and_line sal);
 
 
 /* Return a `struct macro_scope' object representing just the
-   user-defined macros.  The result is allocated using xmalloc; the
-   caller is responsible for freeing it.  */
-struct macro_scope *user_macro_scope (void);
+   user-defined macros.  */
+gdb::unique_xmalloc_ptr<struct macro_scope> user_macro_scope (void);
 
 /* Return a `struct macro_scope' object describing the scope the `macro
    expand' and `macro expand-once' commands should use for looking up
@@ -53,11 +53,8 @@ struct macro_scope *user_macro_scope (void);
    its PC; otherwise, this is the last listing position.
 
    If we have no macro information for the current location, return
-   the user macro scope.
-
-   The object returned is allocated using xmalloc; the caller is
-   responsible for freeing it.  */
-struct macro_scope *default_macro_scope (void);
+   the user macro scope.  */
+gdb::unique_xmalloc_ptr<struct macro_scope> default_macro_scope (void);
 
 
 /* Look up the definition of the macro named NAME in scope at the source
diff --git a/gdb/symtab.c b/gdb/symtab.c
index 81f4fc9ec5..0fd3f3a30f 100644
--- a/gdb/symtab.c
+++ b/gdb/symtab.c
@@ -5154,7 +5154,7 @@ default_collect_symbol_completion_matches_break_on
   if (current_language->la_macro_expansion == macro_expansion_c
       && code == TYPE_CODE_UNDEF)
     {
-      struct macro_scope *scope;
+      gdb::unique_xmalloc_ptr<struct macro_scope> scope;
 
       /* This adds a macro's name to the current completion list.  */
       auto add_macro_name = [&] (const char *macro_name,
@@ -5175,11 +5175,8 @@ default_collect_symbol_completion_matches_break_on
 	 completion time.  */
       scope = default_macro_scope ();
       if (scope)
-	{
-	  macro_for_each_in_scope (scope->file, scope->line,
-				   add_macro_name);
-	  xfree (scope);
-	}
+	macro_for_each_in_scope (scope->file, scope->line,
+				 add_macro_name);
 
       /* User-defined macros are always visible.  */
       macro_for_each (macro_user_macros, add_macro_name);
-- 
2.13.6

[RFA 5/9] Use std::string in maybe_expand

[Tom Tromey]

This patch changes maybe_expand to use std::string rather than an
explicit malloc and a cleanup.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* macroexp.c (maybe_expand): Use std::string.
---
 gdb/ChangeLog  |  4 ++++
 gdb/macroexp.c | 20 ++++++--------------
 2 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 33f7e5f3d0..f33e3a6f7f 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* macroexp.c (maybe_expand): Use std::string.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* macroexp.c (struct macro_buffer): Add initializers for some
 	members.
 	(init_buffer, init_shared_buffer, free_buffer)
diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index 02cf26ff73..1fa37d2875 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -1347,28 +1347,20 @@ maybe_expand (struct macro_buffer *dest,
     {
       /* Make a null-terminated copy of it, since that's what our
          lookup function expects.  */
-      char *id = (char *) xmalloc (src_first->len + 1);
-      struct cleanup *back_to = make_cleanup (xfree, id);
+      std::string id (src_first->text, src_first->len);
 
-      memcpy (id, src_first->text, src_first->len);
-      id[src_first->len] = 0;
-          
       /* If we're currently re-scanning the result of expanding
          this macro, don't expand it again.  */
-      if (! currently_rescanning (no_loop, id))
+      if (! currently_rescanning (no_loop, id.c_str ()))
         {
           /* Does this identifier have a macro definition in scope?  */
-          struct macro_definition *def = lookup_func (id, lookup_baton);
+          struct macro_definition *def = lookup_func (id.c_str (),
+						      lookup_baton);
 
-          if (def && expand (id, def, dest, src_rest, no_loop,
+          if (def && expand (id.c_str (), def, dest, src_rest, no_loop,
                              lookup_func, lookup_baton))
-            {
-              do_cleanups (back_to);
-              return 1;
-            }
+	    return 1;
         }
-
-      do_cleanups (back_to);
     }
 
   return 0;
-- 
2.13.6

[RFA 1/9] Remove a cleanup from gdbserver

[Tom Tromey]

This removes a cleanup from gdbserver's set_raw_breakpoint_at,
replacing it with unique_xmalloc_ptr.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* mem-break.c (set_raw_breakpoint_at): Use
	gdb::unique_xmalloc_ptr.
---
 gdb/gdbserver/ChangeLog   |  5 +++++
 gdb/gdbserver/mem-break.c | 11 ++++++-----
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 9559e63f92..902ee7c7fd 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,3 +1,8 @@
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
+	* mem-break.c (set_raw_breakpoint_at): Use
+	gdb::unique_xmalloc_ptr.
+
 2018-01-30  Pedro Alves  <palves@redhat.com>
 
 	PR gdb/13211
diff --git a/gdb/gdbserver/mem-break.c b/gdb/gdbserver/mem-break.c
index bb385733fe..4c10fbba2c 100644
--- a/gdb/gdbserver/mem-break.c
+++ b/gdb/gdbserver/mem-break.c
@@ -431,7 +431,6 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
 {
   struct process_info *proc = current_process ();
   struct raw_breakpoint *bp;
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
 
   if (type == raw_bkpt_type_sw || type == raw_bkpt_type_hw)
     {
@@ -450,13 +449,14 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
   else
     bp = find_raw_breakpoint_at (where, type, kind);
 
+  gdb::unique_xmalloc_ptr<struct raw_breakpoint> bp_holder;
   if (bp == NULL)
     {
-      bp = XCNEW (struct raw_breakpoint);
+      bp_holder.reset (XCNEW (struct raw_breakpoint));
+      bp = bp_holder.get ();
       bp->pc = where;
       bp->kind = kind;
       bp->raw_type = type;
-      make_cleanup (xfree, bp);
     }
 
   if (!bp->inserted)
@@ -468,14 +468,15 @@ set_raw_breakpoint_at (enum raw_bkpt_type type, CORE_ADDR where, int kind,
 	    debug_printf ("Failed to insert breakpoint at 0x%s (%d).\n",
 			  paddress (where), *err);
 
-	  do_cleanups (old_chain);
 	  return NULL;
 	}
 
       bp->inserted = 1;
     }
 
-  discard_cleanups (old_chain);
+  /* If the breakpoint was allocated above, we know we want to keep it
+     now.  */
+  bp_holder.release ();
 
   /* Link the breakpoint in, if this is the first reference.  */
   if (++bp->refcount == 1)
-- 
2.13.6

[RFA 7/9] Use std::vector in find_source_lines

[Tom Tromey]

This replaces an explicit malloc and a cleanup with a std::vector.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* source.c (find_source_lines): Use std::vector.
---
 gdb/ChangeLog |  4 ++++
 gdb/source.c  | 20 ++++++++------------
 2 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f0ec664d23..8bd04e47c7 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* source.c (find_source_lines): Use std::vector.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* macrocmd.c (struct temporary_macro_definition): New.
 	(macro_define_command): Use temporary_macro_definition.  Remove
 	cleanups.
diff --git a/gdb/source.c b/gdb/source.c
index 1f136f4cfb..0e871be8ca 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1186,7 +1186,7 @@ void
 find_source_lines (struct symtab *s, int desc)
 {
   struct stat st;
-  char *data, *p, *end;
+  char *p, *end;
   int nlines = 0;
   int lines_allocated = 1000;
   int *line_charpos;
@@ -1207,23 +1207,20 @@ find_source_lines (struct symtab *s, int desc)
     warning (_("Source file is more recent than executable."));
 
   {
-    struct cleanup *old_cleanups;
-
     /* st_size might be a large type, but we only support source files whose 
        size fits in an int.  */
     size = (int) st.st_size;
 
-    /* Use malloc, not alloca, because this may be pretty large, and we may
-       run into various kinds of limits on stack size.  */
-    data = (char *) xmalloc (size);
-    old_cleanups = make_cleanup (xfree, data);
+    /* Use the heap, not the stack, because this may be pretty large,
+       and we may run into various kinds of limits on stack size.  */
+    std::vector<char> data (size);
 
     /* Reassign `size' to result of read for systems where \r\n -> \n.  */
-    size = myread (desc, data, size);
+    size = myread (desc, data.data (), size);
     if (size < 0)
       perror_with_name (symtab_to_filename_for_display (s));
-    end = data + size;
-    p = data;
+    end = &data[size];
+    p = &data[0];
     line_charpos[0] = 0;
     nlines = 1;
     while (p != end)
@@ -1239,10 +1236,9 @@ find_source_lines (struct symtab *s, int desc)
 		  (int *) xrealloc ((char *) line_charpos,
 				    sizeof (int) * lines_allocated);
 	      }
-	    line_charpos[nlines++] = p - data;
+	    line_charpos[nlines++] = p - data.data ();
 	  }
       }
-    do_cleanups (old_cleanups);
   }
 
   s->nlines = nlines;
-- 
2.13.6

[RFA 8/9] Use unique_xmalloc_ptr in build_id_to_debug_bfd

[Tom Tromey]

This changes build_id_to_debug_bfd to use a unique_xmalloc_ptr,
removing a cleanup.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* build-id.c (build_id_to_debug_bfd): Use unique_xmalloc_ptr.
---
 gdb/ChangeLog  | 4 ++++
 gdb/build-id.c | 9 +++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 8bd04e47c7..84115ab5a9 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* build-id.c (build_id_to_debug_bfd): Use unique_xmalloc_ptr.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* source.c (find_source_lines): Use std::vector.
 
 2018-02-07  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/build-id.c b/gdb/build-id.c
index 945da4f3cf..685a2b0370 100644
--- a/gdb/build-id.c
+++ b/gdb/build-id.c
@@ -95,8 +95,6 @@ build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id)
       const gdb_byte *data = build_id;
       size_t size = build_id_len;
       char *s;
-      char *filename = NULL;
-      struct cleanup *inner;
 
       memcpy (link, debugdir, debugdir_len);
       s = &link[debugdir_len];
@@ -116,16 +114,15 @@ build_id_to_debug_bfd (size_t build_id_len, const bfd_byte *build_id)
 	printf_unfiltered (_("  Trying %s\n"), link);
 
       /* lrealpath() is expensive even for the usually non-existent files.  */
+      gdb::unique_xmalloc_ptr<char> filename;
       if (access (link, F_OK) == 0)
-	filename = lrealpath (link);
+	filename.reset (lrealpath (link));
 
       if (filename == NULL)
 	continue;
 
       /* We expect to be silent on the non-existing files.  */
-      inner = make_cleanup (xfree, filename);
-      abfd = gdb_bfd_open (filename, gnutarget, -1);
-      do_cleanups (inner);
+      abfd = gdb_bfd_open (filename.get (), gnutarget, -1);
 
       if (abfd == NULL)
 	continue;
-- 
2.13.6

[RFA 0/9] more cleanup removal

[Tom Tromey]

This series removes a number of cleanups, replacing them with
unique_xmalloc_ptr or other things, as appropriate.

Regression tested by the buildbot.

Tom

[RFA 9/9] Remove cleanups from solib.c

[Tom Tromey]

This removes a couple of cleanups from solib.c, replacing one with
std::string and another with unique_xmalloc_ptr.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* solib.c (solib_find_1): Use std::string.
	(solib_bfd_fopen): Use unique_xmalloc_ptr.
---
 gdb/ChangeLog |  5 +++++
 gdb/solib.c   | 14 ++++++--------
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 84115ab5a9..ee1339b251 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* solib.c (solib_find_1): Use std::string.
+	(solib_bfd_fopen): Use unique_xmalloc_ptr.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* build-id.c (build_id_to_debug_bfd): Use unique_xmalloc_ptr.
 
 2018-02-07  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/solib.c b/gdb/solib.c
index 3dba5eaa8a..40fdfc59e6 100644
--- a/gdb/solib.c
+++ b/gdb/solib.c
@@ -155,8 +155,7 @@ solib_find_1 (const char *in_pathname, int *fd, int is_solib)
   int found_file = -1;
   char *temp_pathname = NULL;
   const char *fskind = effective_target_file_system_kind ();
-  struct cleanup *old_chain = make_cleanup (null_cleanup, NULL);
-  char *sysroot = gdb_sysroot;
+  const char *sysroot = gdb_sysroot;
   int prefix_len, orig_prefix_len;
 
   /* If the absolute prefix starts with "target:" but the filesystem
@@ -174,12 +173,13 @@ solib_find_1 (const char *in_pathname, int *fd, int is_solib)
   while (prefix_len > 0 && IS_DIR_SEPARATOR (sysroot[prefix_len - 1]))
     prefix_len--;
 
+  std::string sysroot_holder;
   if (prefix_len == 0)
     sysroot = NULL;
   else if (prefix_len != orig_prefix_len)
     {
-      sysroot = savestring (sysroot, prefix_len);
-      make_cleanup (xfree, sysroot);
+      sysroot_holder = std::string (sysroot, prefix_len);
+      sysroot = sysroot_holder.c_str ();
     }
 
   /* If we're on a non-DOS-based system, backslashes won't be
@@ -251,7 +251,6 @@ solib_find_1 (const char *in_pathname, int *fd, int is_solib)
     {
       if (fd != NULL)
 	*fd = -1;
-      do_cleanups (old_chain);
       return temp_pathname;
     }
 
@@ -300,8 +299,6 @@ solib_find_1 (const char *in_pathname, int *fd, int is_solib)
 	}
     }
 
-  do_cleanups (old_chain);
-
   /* We try to find the library in various ways.  After each attempt,
      either found_file >= 0 and temp_pathname is a malloc'd string, or
      found_file < 0 and temp_pathname does not point to storage that
@@ -480,7 +477,8 @@ solib_bfd_fopen (char *pathname, int fd)
 
   if (abfd == NULL)
     {
-      make_cleanup (xfree, pathname);
+      /* Arrange to free PATHNAME when the error is thrown.  */
+      gdb::unique_xmalloc_ptr<char> free_pathname (pathname);
       error (_("Could not open `%s' as an executable file: %s"),
 	     pathname, bfd_errmsg (bfd_get_error ()));
     }
-- 
2.13.6

[RFA 2/9] Remove make_cleanup_restore_current_thread from gdbserver

[Tom Tromey]

This removes make_cleanup_restore_current_thread from gdbserver,
replacing it with a use of scoped_restore.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* linux-low.c (install_software_single_step_breakpoints): Use
	make_scoped_restore.
	* inferiors.c (make_cleanup_restore_current_thread): Remove.
	(do_restore_current_thread_cleanup): Remove.
	* gdbthread.h (make_cleanup_restore_current_thread): Don't
	declare.
---
 gdb/gdbserver/ChangeLog   |  9 +++++++++
 gdb/gdbserver/gdbthread.h |  3 ---
 gdb/gdbserver/inferiors.c | 12 ------------
 gdb/gdbserver/linux-low.c |  6 +++---
 4 files changed, 12 insertions(+), 18 deletions(-)

diff --git a/gdb/gdbserver/ChangeLog b/gdb/gdbserver/ChangeLog
index 902ee7c7fd..0213edb1ee 100644
--- a/gdb/gdbserver/ChangeLog
+++ b/gdb/gdbserver/ChangeLog
@@ -1,5 +1,14 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* linux-low.c (install_software_single_step_breakpoints): Use
+	make_scoped_restore.
+	* inferiors.c (make_cleanup_restore_current_thread): Remove.
+	(do_restore_current_thread_cleanup): Remove.
+	* gdbthread.h (make_cleanup_restore_current_thread): Don't
+	declare.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* mem-break.c (set_raw_breakpoint_at): Use
 	gdb::unique_xmalloc_ptr.
 
diff --git a/gdb/gdbserver/gdbthread.h b/gdb/gdbserver/gdbthread.h
index 9a8c72e780..0edf870c56 100644
--- a/gdb/gdbserver/gdbthread.h
+++ b/gdb/gdbserver/gdbthread.h
@@ -224,7 +224,4 @@ lwpid_of (const thread_info *thread)
   return thread->id.lwp ();
 }
 
-/* Create a cleanup to restore current_thread.  */
-struct cleanup *make_cleanup_restore_current_thread (void);
-
 #endif /* GDB_THREAD_H */
diff --git a/gdb/gdbserver/inferiors.c b/gdb/gdbserver/inferiors.c
index 600bf201bf..ad3df648f5 100644
--- a/gdb/gdbserver/inferiors.c
+++ b/gdb/gdbserver/inferiors.c
@@ -213,18 +213,6 @@ current_process (void)
   return get_thread_process (current_thread);
 }
 
-static void
-do_restore_current_thread_cleanup (void *arg)
-{
-  current_thread = (struct thread_info *) arg;
-}
-
-struct cleanup *
-make_cleanup_restore_current_thread (void)
-{
-  return make_cleanup (do_restore_current_thread_cleanup, current_thread);
-}
-
 /* See common/common-gdbthread.h.  */
 
 void
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index 38142bba03..b6b660ea96 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -50,6 +50,7 @@
 #include "common-inferior.h"
 #include "nat/fork-inferior.h"
 #include "environ.h"
+#include "common/scoped_restore.h"
 #ifndef ELFMAG0
 /* Don't include <linux/elf.h> here.  If it got included by gdb_proc_service.h
    then ELFMAG0 will have been defined.  If it didn't get included by
@@ -4207,15 +4208,14 @@ install_software_single_step_breakpoints (struct lwp_info *lwp)
 {
   struct thread_info *thread = get_lwp_thread (lwp);
   struct regcache *regcache = get_thread_regcache (thread, 1);
-  struct cleanup *old_chain = make_cleanup_restore_current_thread ();
+
+  scoped_restore save_current_thread = make_scoped_restore (&current_thread);
 
   current_thread = thread;
   std::vector<CORE_ADDR> next_pcs = the_low_target.get_next_pcs (regcache);
 
   for (CORE_ADDR pc : next_pcs)
     set_single_step_breakpoint (pc, current_ptid);
-
-  do_cleanups (old_chain);
 }
 
 /* Single step via hardware or software single step.
-- 
2.13.6

[RFA 6/9] Remove cleanups from macro_define_command

[Tom Tromey]

This removes cleanups from macro_define_command, by introducing a new
struct temporary_macro_definition that cleans up after itself.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* macrocmd.c (struct temporary_macro_definition): New.
	(macro_define_command): Use temporary_macro_definition.  Remove
	cleanups.
	(free_macro_definition_ptr): Remove.
---
 gdb/ChangeLog  |  7 +++++++
 gdb/macrocmd.c | 42 +++++++++++++++++++++---------------------
 2 files changed, 28 insertions(+), 21 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index f33e3a6f7f..f0ec664d23 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* macrocmd.c (struct temporary_macro_definition): New.
+	(macro_define_command): Use temporary_macro_definition.  Remove
+	cleanups.
+	(free_macro_definition_ptr): Remove.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* macroexp.c (maybe_expand): Use std::string.
 
 2018-02-07  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/macrocmd.c b/gdb/macrocmd.c
index 13fd95d7da..e6cf921a1b 100644
--- a/gdb/macrocmd.c
+++ b/gdb/macrocmd.c
@@ -324,35 +324,37 @@ extract_identifier (const char **expp, int is_parameter)
   return result;
 }
 
-/* Helper function to clean up a temporarily-constructed macro object.
-   This assumes that the contents were all allocated with xmalloc.  */
-static void
-free_macro_definition_ptr (void *ptr)
+struct temporary_macro_definition : public macro_definition
 {
-  int i;
-  struct macro_definition *loc = (struct macro_definition *) ptr;
-
-  for (i = 0; i < loc->argc; ++i)
-    xfree ((char *) loc->argv[i]);
-  xfree ((char *) loc->argv);
-  /* Note that the 'replacement' field is not allocated.  */
-}
+  temporary_macro_definition ()
+  {
+    table = nullptr;
+    kind = macro_object_like;
+    argc = 0;
+    argv = nullptr;
+    replacement = nullptr;
+  }
+
+  ~temporary_macro_definition ()
+  {
+    int i;
+
+    for (i = 0; i < argc; ++i)
+      xfree ((char *) argv[i]);
+    xfree ((char *) argv);
+    /* Note that the 'replacement' field is not allocated.  */
+  }
+};
 
 static void
 macro_define_command (const char *exp, int from_tty)
 {
-  struct macro_definition new_macro;
+  temporary_macro_definition new_macro;
   char *name = NULL;
-  struct cleanup *cleanup_chain;
 
   if (!exp)
     error (_("usage: macro define NAME[(ARGUMENT-LIST)] [REPLACEMENT-LIST]"));
 
-  cleanup_chain = make_cleanup (free_macro_definition_ptr, &new_macro);
-  make_cleanup (free_current_contents, &name);
-
-  memset (&new_macro, 0, sizeof (struct macro_definition));
-
   skip_ws (&exp);
   name = extract_identifier (&exp, 0);
   if (! name)
@@ -415,8 +417,6 @@ macro_define_command (const char *exp, int from_tty)
       skip_ws (&exp);
       macro_define_object (macro_main (macro_user_macros), -1, name, exp);
     }
-
-  do_cleanups (cleanup_chain);
 }
 
 
-- 
2.13.6

[RFA 4/9] Class-ify macro_buffer

[Tom Tromey]

This patch changes macro_buffer to be a bit more of a C++ class,
adding constructors, a destructor, and some members.  Then this is
used to remove various cleanups in macroexp.c.

2018-02-07  Tom Tromey  <tom@tromey.com>

	* macroexp.c (struct macro_buffer): Add initializers for some
	members.
	(init_buffer, init_shared_buffer, free_buffer)
	(free_buffer_return_text): Remove.
	(macro_buffer): New constructors.
	(~macro_buffer): New destructor.
	(macro_buffer::set_shared): New method.
	(macro_buffer::resize_buffer, macro_buffer::appendc)
	(macro_buffer::appendmem): Now methods, not free functions.
	(set_token, append_tokens_without_splicing, stringify)
	(macro_stringify): Update.
	(gather_arguments): Change return type.  Remove argc_p argument,
	add args_ptr argument.  Use std::vector.
	(substitute_args): Remove argc argument.  Accept std::vector.
	(expand): Update.  Use std::vector.
	(scan, macro_expand, macro_expand_next): Update.
---
 gdb/ChangeLog  |  19 +++
 gdb/macroexp.c | 406 +++++++++++++++++++++++----------------------------------
 2 files changed, 185 insertions(+), 240 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index abb6126864..33f7e5f3d0 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,24 @@
 2018-02-07  Tom Tromey  <tom@tromey.com>
 
+	* macroexp.c (struct macro_buffer): Add initializers for some
+	members.
+	(init_buffer, init_shared_buffer, free_buffer)
+	(free_buffer_return_text): Remove.
+	(macro_buffer): New constructors.
+	(~macro_buffer): New destructor.
+	(macro_buffer::set_shared): New method.
+	(macro_buffer::resize_buffer, macro_buffer::appendc)
+	(macro_buffer::appendmem): Now methods, not free functions.
+	(set_token, append_tokens_without_splicing, stringify)
+	(macro_stringify): Update.
+	(gather_arguments): Change return type.  Remove argc_p argument,
+	add args_ptr argument.  Use std::vector.
+	(substitute_args): Remove argc argument.  Accept std::vector.
+	(expand): Update.  Use std::vector.
+	(scan, macro_expand, macro_expand_next): Update.
+
+2018-02-07  Tom Tromey  <tom@tromey.com>
+
 	* symtab.c (default_collect_symbol_completion_matches_break_on):
 	Use unique_xmalloc_ptr.
 	* macroscope.h: (sal_macro_scope, user_macro_scope)
diff --git a/gdb/macroexp.c b/gdb/macroexp.c
index aefb13a9d6..02cf26ff73 100644
--- a/gdb/macroexp.c
+++ b/gdb/macroexp.c
@@ -65,121 +65,110 @@ struct macro_buffer
      no token abutting the end of TEXT (it's just whitespace), and
      again, we set this equal to LEN.  We set this to -1 if we don't
      know the nature of TEXT.  */
-  int last_token;
+  int last_token = -1;
 
   /* If this buffer is holding the result from get_token, then this 
      is non-zero if it is an identifier token, zero otherwise.  */
-  int is_identifier;
-};
-
-
-/* Set the macro buffer *B to the empty string, guessing that its
-   final contents will fit in N bytes.  (It'll get resized if it
-   doesn't, so the guess doesn't have to be right.)  Allocate the
-   initial storage with xmalloc.  */
-static void
-init_buffer (struct macro_buffer *b, int n)
-{
-  b->size = n;
-  if (n > 0)
-    b->text = (char *) xmalloc (n);
-  else
-    b->text = NULL;
-  b->len = 0;
-  b->shared = false;
-  b->last_token = -1;
-}
-
-
-/* Set the macro buffer *BUF to refer to the LEN bytes at ADDR, as a
-   shared substring.  */
-
-static void
-init_shared_buffer (struct macro_buffer *buf, const char *addr, int len)
-{
-  /* The function accept a "const char *" addr so that clients can
-     pass in string literals without casts.  */
-  buf->text = (char *) addr;
-  buf->len = len;
-  buf->shared = true;
-  buf->size = 0;
-  buf->last_token = -1;
-}
+  int is_identifier = 0;
 
 
-/* Free the text of the buffer B.  Raise an error if B is shared.  */
-static void
-free_buffer (struct macro_buffer *b)
-{
-  gdb_assert (! b->shared);
-  if (b->size)
-    xfree (b->text);
-}
+  macro_buffer ()
+    : text (NULL),
+      len (0),
+      size (0),
+      shared (false)
+  {
+  }
 
-/* Like free_buffer, but return the text as an xstrdup()d string.
-   This only exists to try to make the API relatively clean.  */
+  /* Set the macro buffer to the empty string, guessing that its
+     final contents will fit in N bytes.  (It'll get resized if it
+     doesn't, so the guess doesn't have to be right.)  Allocate the
+     initial storage with xmalloc.  */
+  explicit macro_buffer (int n)
+    : len (0),
+      size (n),
+      shared (false)
+  {
+    if (n > 0)
+      text = (char *) xmalloc (n);
+    else
+      text = NULL;
+  }
 
-static char *
-free_buffer_return_text (struct macro_buffer *b)
-{
-  gdb_assert (! b->shared);
-  gdb_assert (b->size);
-  /* Nothing to do.  */
-  return b->text;
-}
+  /* Set the macro buffer to refer to the LEN bytes at ADDR, as a
+     shared substring.  */
+  macro_buffer (const char *addr, int len)
+  {
+    set_shared (addr, len);
+  }
 
-/* A cleanup function for macro buffers.  */
-static void
-cleanup_macro_buffer (void *untyped_buf)
-{
-  free_buffer ((struct macro_buffer *) untyped_buf);
-}
+  /* Set the macro buffer to refer to the LEN bytes at ADDR, as a
+     shared substring.  */
+  void set_shared (const char *addr, int len_)
+  {
+    text = (char *) addr;
+    len = len_;
+    size = 0;
+    shared = true;
+  }
 
+  ~macro_buffer ()
+  {
+    if (! shared && size)
+      xfree (text);
+  }
 
-/* Resize the buffer B to be at least N bytes long.  Raise an error if
-   B shouldn't be resized.  */
-static void
-resize_buffer (struct macro_buffer *b, int n)
-{
-  /* We shouldn't be trying to resize shared strings.  */
-  gdb_assert (! b->shared);
-  
-  if (b->size == 0)
-    b->size = n;
-  else
-    while (b->size <= n)
-      b->size *= 2;
+  /* Release the text of the buffer to the caller, which is now
+     responsible for freeing it.  */
+  char *release ()
+  {
+    gdb_assert (! shared);
+    gdb_assert (size);
+    char *result = text;
+    text = NULL;
+    return result;
+  }
 
-  b->text = (char *) xrealloc (b->text, b->size);
-}
+  /* Resize the buffer to be at least N bytes long.  Raise an error if
+     the buffer shouldn't be resized.  */
+  void resize_buffer (int n)
+  {
+    /* We shouldn't be trying to resize shared strings.  */
+    gdb_assert (! shared);
 
+    if (size == 0)
+      size = n;
+    else
+      while (size <= n)
+	size *= 2;
 
-/* Append the character C to the buffer B.  */
-static void
-appendc (struct macro_buffer *b, int c)
-{
-  int new_len = b->len + 1;
+    text = (char *) xrealloc (text, size);
+  }
 
-  if (new_len > b->size)
-    resize_buffer (b, new_len);
+  /* Append the character C to the buffer.  */
+  void appendc (int c)
+  {
+    int new_len = len + 1;
 
-  b->text[b->len] = c;
-  b->len = new_len;
-}
+    if (new_len > size)
+      resize_buffer (new_len);
 
+    text[len] = c;
+    len = new_len;
+  }
 
-/* Append the LEN bytes at ADDR to the buffer B.  */
-static void
-appendmem (struct macro_buffer *b, const char *addr, int len)
-{
-  int new_len = b->len + len;
+  /* Append the COUNT bytes at ADDR to the buffer.  */
+  void appendmem (const char *addr, int count)
+  {
+    int new_len = len + count;
 
-  if (new_len > b->size)
-    resize_buffer (b, new_len);
+    if (new_len > size)
+      resize_buffer (new_len);
 
-  memcpy (b->text + b->len, addr, len);
-  b->len = new_len;
-}
+    memcpy (text + len, addr, count);
+    len = new_len;
+  }
+};
 
 
 \f
@@ -216,7 +205,7 @@ macro_is_identifier_nondigit (int c)
 static void
 set_token (struct macro_buffer *tok, char *start, char *end)
 {
-  init_shared_buffer (tok, start, end - start);
+  tok->set_shared (start, end - start);
   tok->last_token = 0;
 
   /* Presumed; get_identifier may overwrite this.  */
@@ -596,7 +585,7 @@ append_tokens_without_splicing (struct macro_buffer *dest,
   
   /* First, just try appending the two, and call get_token to see if
      we got a splice.  */
-  appendmem (dest, src->text, src->len);
+  dest->appendmem (src->text, src->len);
 
   /* If DEST originally had no token abutting its end, then we can't
      have spliced anything, so we're done.  */
@@ -608,9 +597,8 @@ append_tokens_without_splicing (struct macro_buffer *dest,
 
   /* Set DEST_TAIL to point to the last token in DEST, followed by
      all the stuff we just appended.  */
-  init_shared_buffer (&dest_tail,
-                      dest->text + dest->last_token,
-                      dest->len - dest->last_token);
+  dest_tail.set_shared (dest->text + dest->last_token,
+			dest->len - dest->last_token);
 
   /* Re-parse DEST's last token.  We know that DEST used to contain
      at least one token, so if it doesn't contain any after the
@@ -630,12 +618,11 @@ append_tokens_without_splicing (struct macro_buffer *dest,
      its original length and try again, but separate the texts with a
      space.  */
   dest->len = original_dest_len;
-  appendc (dest, ' ');
-  appendmem (dest, src->text, src->len);
+  dest->appendc (' ');
+  dest->appendmem (src->text, src->len);
 
-  init_shared_buffer (&dest_tail,
-                      dest->text + dest->last_token,
-                      dest->len - dest->last_token);
+  dest_tail.set_shared (dest->text + dest->last_token,
+			dest->len - dest->last_token);
 
   /* Try to re-parse DEST's last token, as above.  */
   if (get_token (&new_token, &dest_tail)
@@ -671,7 +658,7 @@ stringify (struct macro_buffer *dest, const char *arg, int len)
     --len;
 
   /* Insert the string.  */
-  appendc (dest, '"');
+  dest->appendc ('"');
   while (len > 0)
     {
       /* We could try to handle strange cases here, like control
@@ -679,7 +666,7 @@ stringify (struct macro_buffer *dest, const char *arg, int len)
       if (macro_is_whitespace (*arg))
 	{
 	  /* Replace a sequence of whitespace with a single space.  */
-	  appendc (dest, ' ');
+	  dest->appendc (' ');
 	  while (len > 1 && macro_is_whitespace (arg[1]))
 	    {
 	      ++arg;
@@ -688,15 +675,15 @@ stringify (struct macro_buffer *dest, const char *arg, int len)
 	}
       else if (*arg == '\\' || *arg == '"')
 	{
-	  appendc (dest, '\\');
-	  appendc (dest, *arg);
+	  dest->appendc ('\\');
+	  dest->appendc (*arg);
 	}
       else
-	appendc (dest, *arg);
+	dest->appendc (*arg);
       ++arg;
       --len;
     }
-  appendc (dest, '"');
+  dest->appendc ('"');
   dest->last_token = dest->len;
 }
 
@@ -705,14 +692,13 @@ stringify (struct macro_buffer *dest, const char *arg, int len)
 char *
 macro_stringify (const char *str)
 {
-  struct macro_buffer buffer;
   int len = strlen (str);
+  struct macro_buffer buffer (len);
 
-  init_buffer (&buffer, len);
   stringify (&buffer, str, len);
-  appendc (&buffer, '\0');
+  buffer.appendc ('\0');
 
-  return free_buffer_return_text (&buffer);
+  return buffer.release ();
 }
 
 \f
@@ -759,22 +745,19 @@ currently_rescanning (struct macro_name_list *list, const char *name)
    baz).
 
    If SRC doesn't start with an open paren ( token at all, return
-   zero, leave SRC unchanged, and don't set *ARGC_P to anything.
+   false, leave SRC unchanged, and don't set *ARGS_PTR to anything.
 
    If SRC doesn't contain a properly terminated argument list, then
    raise an error.
-   
+
    For a variadic macro, NARGS holds the number of formal arguments to
    the macro.  For a GNU-style variadic macro, this should be the
    number of named arguments.  For a non-variadic macro, NARGS should
    be -1.
 
-   Otherwise, return a pointer to the first element of an array of
-   macro buffers referring to the argument texts, and set *ARGC_P to
-   the number of arguments we found --- the number of elements in the
-   array.  The macro buffers share their text with SRC, and their
-   last_token fields are initialized.  The array is allocated with
-   xmalloc, and the caller is responsible for freeing it.
+   Otherwise, return true and set *ARGS_PTR to a vector of macro
+   buffers referring to the argument texts.  The macro buffers share
+   their text with SRC, and their last_token fields are initialized.
 
    NOTE WELL: if SRC starts with a open paren ( token followed
    immediately by a close paren ) token (e.g., the invocation looks
@@ -787,53 +770,36 @@ currently_rescanning (struct macro_name_list *list, const char *name)
    Consume the tokens from SRC; after this call, SRC contains the text
    following the invocation.  */
 
-static struct macro_buffer *
-gather_arguments (const char *name, struct macro_buffer *src,
-		  int nargs, int *argc_p)
+static bool
+gather_arguments (const char *name, struct macro_buffer *src, int nargs,
+		  std::vector<struct macro_buffer> *args_ptr)
 {
   struct macro_buffer tok;
-  int args_len, args_size;
-  struct macro_buffer *args = NULL;
-  struct cleanup *back_to = make_cleanup (free_current_contents, &args);
+  std::vector<struct macro_buffer> args;
 
   /* Does SRC start with an opening paren token?  Read from a copy of
      SRC, so SRC itself is unaffected if we don't find an opening
      paren.  */
   {
-    struct macro_buffer temp;
-
-    init_shared_buffer (&temp, src->text, src->len);
+    struct macro_buffer temp (src->text, src->len);
 
     if (! get_token (&tok, &temp)
         || tok.len != 1
         || tok.text[0] != '(')
-      {
-        discard_cleanups (back_to);
-        return 0;
-      }
+      return false;
   }
 
   /* Consume SRC's opening paren.  */
   get_token (&tok, src);
 
-  args_len = 0;
-  args_size = 6;
-  args = XNEWVEC (struct macro_buffer, args_size);
-
   for (;;)
     {
       struct macro_buffer *arg;
       int depth;
 
-      /* Make sure we have room for the next argument.  */
-      if (args_len >= args_size)
-        {
-          args_size *= 2;
-          args = XRESIZEVEC (struct macro_buffer, args, args_size);
-        }
-
       /* Initialize the next argument.  */
-      arg = &args[args_len++];
+      args.emplace_back ();
+      arg = &args.back ();
       set_token (arg, src->text, src->text);
 
       /* Gather the argument's tokens.  */
@@ -842,7 +808,7 @@ gather_arguments (const char *name, struct macro_buffer *src,
         {
           if (! get_token (&tok, src))
             error (_("Malformed argument list for macro `%s'."), name);
-      
+
           /* Is tok an opening paren?  */
           if (tok.len == 1 && tok.text[0] == '(')
             depth++;
@@ -856,22 +822,15 @@ gather_arguments (const char *name, struct macro_buffer *src,
                 {
 		  /* In the varargs case, the last argument may be
 		     missing.  Add an empty argument in this case.  */
-		  if (nargs != -1 && args_len == nargs - 1)
+		  if (nargs != -1 && args.size () == nargs - 1)
 		    {
-		      /* Make sure we have room for the argument.  */
-		      if (args_len >= args_size)
-			{
-			  args_size++;
-			  args = XRESIZEVEC (struct macro_buffer, args,
-					     args_size);
-			}
-		      arg = &args[args_len++];
+		      args.emplace_back ();
+		      arg = &args.back ();
 		      set_token (arg, src->text, src->text);
 		    }
 
-                  discard_cleanups (back_to);
-                  *argc_p = args_len;
-                  return args;
+		  *args_ptr = std::move (args);
+		  return true;
                 }
 
               depth--;
@@ -882,7 +841,7 @@ gather_arguments (const char *name, struct macro_buffer *src,
              variadic macro and we are computing the last argument, we
              want to include the comma and remaining tokens.  */
           else if (tok.len == 1 && tok.text[0] == ',' && depth == 0
-		   && (nargs == -1 || args_len < nargs))
+		   && (nargs == -1 || args.size () < nargs))
             break;
 
           /* Extend the current argument to enclose this token.  If
@@ -971,7 +930,7 @@ get_next_token_for_substitution (struct macro_buffer *replacement_list,
 }
 
 /* Given the macro definition DEF, being invoked with the actual
-   arguments given by ARGC and ARGV, substitute the arguments into the
+   arguments given by ARGV, substitute the arguments into the
    replacement list, and store the result in DEST.
 
    IS_VARARGS should be true if DEF is a varargs macro.  In this case,
@@ -986,16 +945,14 @@ get_next_token_for_substitution (struct macro_buffer *replacement_list,
    NO_LOOP.  */
 
 static void
-substitute_args (struct macro_buffer *dest, 
+substitute_args (struct macro_buffer *dest,
                  struct macro_definition *def,
 		 int is_varargs, const struct macro_buffer *va_arg_name,
-                 int argc, struct macro_buffer *argv,
+		 const std::vector<struct macro_buffer> &argv,
                  struct macro_name_list *no_loop,
                  macro_lookup_ftype *lookup_func,
                  void *lookup_baton)
 {
-  /* A macro buffer for the macro's replacement list.  */
-  struct macro_buffer replacement_list;
   /* The token we are currently considering.  */
   struct macro_buffer tok;
   /* The replacement list's pointer from just before TOK was lexed.  */
@@ -1008,8 +965,9 @@ substitute_args (struct macro_buffer *dest,
      lexed.  */
   char *lookahead_rl_start;
 
-  init_shared_buffer (&replacement_list, def->replacement,
-                      strlen (def->replacement));
+  /* A macro buffer for the macro's replacement list.  */
+  struct macro_buffer replacement_list (def->replacement,
+					strlen (def->replacement));
 
   gdb_assert (dest->len == 0);
   dest->last_token = 0;
@@ -1066,7 +1024,7 @@ substitute_args (struct macro_buffer *dest,
 
 	  /* If __VA_ARGS__ is empty, then drop the contents of
 	     __VA_OPT__.  */
-	  if (argv[argc - 1].len == 0)
+	  if (argv.back ().len == 0)
 	    continue;
 	}
       else if (token_is_vaopt)
@@ -1082,7 +1040,7 @@ substitute_args (struct macro_buffer *dest,
          that to DEST.  */
       if (tok.text > original_rl_start)
         {
-          appendmem (dest, original_rl_start, tok.text - original_rl_start);
+          dest->appendmem (original_rl_start, tok.text - original_rl_start);
           dest->last_token = dest->len;
         }
 
@@ -1135,9 +1093,9 @@ substitute_args (struct macro_buffer *dest,
 					def->argc, def->argv);
 
 	      if (arg != -1)
-		appendmem (dest, argv[arg].text, argv[arg].len);
+		dest->appendmem (argv[arg].text, argv[arg].len);
 	      else
-		appendmem (dest, tok.text, tok.len);
+		dest->appendmem (tok.text, tok.len);
 	    }
 
 	  /* Apply a possible sequence of ## operators.  */
@@ -1160,8 +1118,8 @@ substitute_args (struct macro_buffer *dest,
 		  if (! (is_varargs
 			 && tok.len == va_arg_name->len
 			 && !memcmp (tok.text, va_arg_name->text, tok.len)
-			 && argv[argc - 1].len == 0))
-		    appendmem (dest, ",", 1);
+			 && argv.back ().len == 0))
+		    dest->appendmem (",", 1);
 		  prev_was_comma = 0;
 		}
 
@@ -1175,9 +1133,9 @@ substitute_args (struct macro_buffer *dest,
 					    def->argc, def->argv);
 
 		  if (arg != -1)
-		    appendmem (dest, argv[arg].text, argv[arg].len);
+		    dest->appendmem (argv[arg].text, argv[arg].len);
 		  else
-		    appendmem (dest, tok.text, tok.len);
+		    dest->appendmem (tok.text, tok.len);
 		}
 
 	      /* Now read another token.  If it is another splice, we
@@ -1198,7 +1156,7 @@ substitute_args (struct macro_buffer *dest,
 	  if (prev_was_comma)
 	    {
 	      /* We saw a comma.  Insert it now.  */
-	      appendmem (dest, ",", 1);
+	      dest->appendmem (",", 1);
 	    }
 
           dest->last_token = dest->len;
@@ -1221,14 +1179,12 @@ substitute_args (struct macro_buffer *dest,
 
 	  if (arg != -1)
 	    {
-	      struct macro_buffer arg_src;
-
 	      /* Expand any macro invocations in the argument text,
 		 and append the result to dest.  Remember that scan
 		 mutates its source, so we need to scan a new buffer
 		 referring to the argument's text, not the argument
 		 itself.  */
-	      init_shared_buffer (&arg_src, argv[arg].text, argv[arg].len);
+	      struct macro_buffer arg_src (argv[arg].text, argv[arg].len);
 	      scan (dest, &arg_src, no_loop, lookup_func, lookup_baton);
 	      substituted = 1;
 	    }
@@ -1275,22 +1231,15 @@ expand (const char *id,
   /* What kind of macro are we expanding?  */
   if (def->kind == macro_object_like)
     {
-      struct macro_buffer replacement_list;
-
-      init_shared_buffer (&replacement_list, def->replacement,
-                          strlen (def->replacement));
+      struct macro_buffer replacement_list (def->replacement,
+					    strlen (def->replacement));
 
       scan (dest, &replacement_list, &new_no_loop, lookup_func, lookup_baton);
       return 1;
     }
   else if (def->kind == macro_function_like)
     {
-      struct cleanup *back_to = make_cleanup (null_cleanup, 0);
-      int argc = 0;
-      struct macro_buffer *argv = NULL;
-      struct macro_buffer substituted;
-      struct macro_buffer substituted_src;
-      struct macro_buffer va_arg_name = {0};
+      struct macro_buffer va_arg_name;
       int is_varargs = 0;
 
       if (def->argc >= 1)
@@ -1299,8 +1248,7 @@ expand (const char *id,
 	    {
 	      /* In C99-style varargs, substitution is done using
 		 __VA_ARGS__.  */
-	      init_shared_buffer (&va_arg_name, "__VA_ARGS__",
-				  strlen ("__VA_ARGS__"));
+	      va_arg_name.set_shared ("__VA_ARGS__", strlen ("__VA_ARGS__"));
 	      is_varargs = 1;
 	    }
 	  else
@@ -1313,43 +1261,36 @@ expand (const char *id,
 		  /* In GNU-style varargs, the name of the
 		     substitution parameter is the name of the formal
 		     argument without the "...".  */
-		  init_shared_buffer (&va_arg_name,
-				      def->argv[def->argc - 1],
-				      len - 3);
+		  va_arg_name.set_shared (def->argv[def->argc - 1], len - 3);
 		  is_varargs = 1;
 		}
 	    }
 	}
 
-      make_cleanup (free_current_contents, &argv);
-      argv = gather_arguments (id, src, is_varargs ? def->argc : -1,
-			       &argc);
-
+      std::vector<struct macro_buffer> argv;
       /* If we couldn't find any argument list, then we don't expand
          this macro.  */
-      if (! argv)
-        {
-          do_cleanups (back_to);
-          return 0;
-        }
+      if (!gather_arguments (id, src, is_varargs ? def->argc : -1,
+			     &argv))
+	return 0;
 
       /* Check that we're passing an acceptable number of arguments for
          this macro.  */
-      if (argc != def->argc)
+      if (argv.size () != def->argc)
         {
-	  if (is_varargs && argc >= def->argc - 1)
+	  if (is_varargs && argv.size () >= def->argc - 1)
 	    {
 	      /* Ok.  */
 	    }
           /* Remember that a sequence of tokens like "foo()" is a
              valid invocation of a macro expecting either zero or one
              arguments.  */
-          else if (! (argc == 1
+          else if (! (argv.size () == 1
 		      && argv[0].len == 0
 		      && def->argc == 0))
             error (_("Wrong number of arguments to macro `%s' "
                    "(expected %d, got %d)."),
-                   id, def->argc, argc);
+                   id, def->argc, int (argv.size ()));
         }
 
       /* Note that we don't expand macro invocations in the arguments
@@ -1358,10 +1299,9 @@ expand (const char *id,
          splicing operator "##" don't get macro references expanded,
          so we can't really tell whether it's appropriate to macro-
          expand an argument until we see how it's being used.  */
-      init_buffer (&substituted, 0);
-      make_cleanup (cleanup_macro_buffer, &substituted);
+      struct macro_buffer substituted (0);
       substitute_args (&substituted, def, is_varargs, &va_arg_name,
-		       argc, argv, no_loop, lookup_func, lookup_baton);
+		       argv, no_loop, lookup_func, lookup_baton);
 
       /* Now `substituted' is the macro's replacement list, with all
          argument values substituted into it properly.  Re-scan it for
@@ -1373,11 +1313,9 @@ expand (const char *id,
          text pointer around, and we still need to be able to find
          `substituted's original text buffer after scanning it so we
          can free it.  */
-      init_shared_buffer (&substituted_src, substituted.text, substituted.len);
+      struct macro_buffer substituted_src (substituted.text, substituted.len);
       scan (dest, &substituted_src, &new_no_loop, lookup_func, lookup_baton);
 
-      do_cleanups (back_to);
-
       return 1;
     }
   else
@@ -1465,7 +1403,7 @@ scan (struct macro_buffer *dest,
          that to DEST.  */
       if (tok.text > original_src_start)
         {
-          appendmem (dest, original_src_start, tok.text - original_src_start);
+          dest->appendmem (original_src_start, tok.text - original_src_start);
           dest->last_token = dest->len;
         }
 
@@ -1479,7 +1417,7 @@ scan (struct macro_buffer *dest,
      src, copy it to dest.  */
   if (src->len)
     {
-      appendmem (dest, src->text, src->len);
+      dest->appendmem (src->text, src->len);
       dest->last_token = dest->len;
     }
 }
@@ -1490,21 +1428,16 @@ macro_expand (const char *source,
               macro_lookup_ftype *lookup_func,
               void *lookup_func_baton)
 {
-  struct macro_buffer src, dest;
-  struct cleanup *back_to;
-
-  init_shared_buffer (&src, source, strlen (source));
+  struct macro_buffer src (source, strlen (source));
 
-  init_buffer (&dest, 0);
+  struct macro_buffer dest (0);
   dest.last_token = 0;
-  back_to = make_cleanup (cleanup_macro_buffer, &dest);
 
   scan (&dest, &src, 0, lookup_func, lookup_func_baton);
 
-  appendc (&dest, '\0');
+  dest.appendc ('\0');
 
-  discard_cleanups (back_to);
-  return gdb::unique_xmalloc_ptr<char> (dest.text);
+  return gdb::unique_xmalloc_ptr<char> (dest.release ());
 }
 
 
@@ -1522,23 +1455,18 @@ macro_expand_next (const char **lexptr,
                    macro_lookup_ftype *lookup_func,
                    void *lookup_baton)
 {
-  struct macro_buffer src, dest, tok;
-  struct cleanup *back_to;
+  struct macro_buffer tok;
 
   /* Set up SRC to refer to the input text, pointed to by *lexptr.  */
-  init_shared_buffer (&src, *lexptr, strlen (*lexptr));
+  struct macro_buffer src (*lexptr, strlen (*lexptr));
 
   /* Set up DEST to receive the expansion, if there is one.  */
-  init_buffer (&dest, 0);
+  struct macro_buffer dest (0);
   dest.last_token = 0;
-  back_to = make_cleanup (cleanup_macro_buffer, &dest);
 
   /* Get the text's first preprocessing token.  */
   if (! get_token (&tok, &src))
-    {
-      do_cleanups (back_to);
-      return 0;
-    }
+    return 0;
 
   /* If it's a macro invocation, expand it.  */
   if (maybe_expand (&dest, &tok, &src, 0, lookup_func, lookup_baton))
@@ -1546,15 +1474,13 @@ macro_expand_next (const char **lexptr,
       /* It was a macro invocation!  Package up the expansion as a
          null-terminated string and return it.  Set *lexptr to the
          start of the next token in the input.  */
-      appendc (&dest, '\0');
-      discard_cleanups (back_to);
+      dest.appendc ('\0');
       *lexptr = src.text;
-      return dest.text;
+      return dest.release ();
     }
   else
     {
       /* It wasn't a macro invocation.  */
-      do_cleanups (back_to);
       return 0;
     }
 }
-- 
2.13.6

Re: [RFA 0/9] more cleanup removal

[Pedro Alves]

Hi Tom,

Yay, cleanups-- :-)

On 02/07/2018 10:04 PM, Tom Tromey wrote:
> This series removes a number of cleanups, replacing them with
> unique_xmalloc_ptr or other things, as appropriate.
> 
> Regression tested by the buildbot.

In patch #7 you can use gdb::def_vector<char> instead, I think.

Otherwise LGTM.  Feel free to push.

Thanks,
Pedro Alves

Re: [RFA 0/9] more cleanup removal

[Tom Tromey]

>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:

Pedro> In patch #7 you can use gdb::def_vector<char> instead, I think.

Thanks - I had already forgotten about this.
I've made the change.

Tom

Re: [RFA 7/9] Use std::vector in find_source_lines

[Tom Tromey]

>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:

Tom> +    end = &data[size];

Andre pointed out that this is possibly fishy, so I think perhaps the
appended should go in.  Ok?

Tom

commit 4e40d3c76f0e14aea171c9ef7447c78defee8a74
Author: Tom Tromey <tom@tromey.com>
Date:   Fri Feb 9 05:58:46 2018 -0700

    Don't reference past the end of the vector
    
    An earlier change made find_source_lines read:
    
        end = &data[size];
    
    However, since 'size' is the size of the vector, this seems fishy.
    More obviously ok is to compute the end of the data directly:
    
        end = data.data () + size;
    
    2018-02-09  Tom Tromey  <tom@tromey.com>
    
            * source.c (find_source_lines): Don't reference past the end of
            the vector.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3e5b324472..d07da95027 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2018-02-09  Tom Tromey  <tom@tromey.com>
+
+	* source.c (find_source_lines): Don't reference past the end of
+	the vector.
+
 2018-02-09  Joel Brobecker  <brobecker@adacore.com>
 
 	* NEWS <Changes in GDB 8.1>: Clarify that "rbreak" is a new
diff --git a/gdb/source.c b/gdb/source.c
index 9eec58febd..009bec5285 100644
--- a/gdb/source.c
+++ b/gdb/source.c
@@ -1219,7 +1219,7 @@ find_source_lines (struct symtab *s, int desc)
     size = myread (desc, data.data (), size);
     if (size < 0)
       perror_with_name (symtab_to_filename_for_display (s));
-    end = &data[size];
+    end = data.data () + size;
     p = &data[0];
     line_charpos[0] = 0;
     nlines = 1;

Re: [RFA 7/9] Use std::vector in find_source_lines

[Pedro Alves]

On 02/09/2018 01:00 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom Tromey <tom@tromey.com> writes:
> 
> Tom> +    end = &data[size];
> 
> Andre pointed out that this is possibly fishy, so I think perhaps the
> appended should go in.  Ok?

OK.

Thanks,
Pedro Alves