more cleanup removal, particularly in MI

more cleanup removal, particularly in MI

[Tom Tromey]

This series removes more cleanups.

The first patch is generic, but the remaining patches focus on
removing cleanups from MI.  It doesn't completely remove cleanups from
MI, but does make significant progress toward that goal.

Regression tested on the buildbot.

Tom

[RFA 09/11] Use std::set in mi-main.c

[Tom Tromey]

Change a couple of spots in mi-main.c to use std::set.  This
simplifies the code and removes some cleanups.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (struct print_one_inferior_data) <inferiors>: Now a
	'std::set *'.
	(print_one_inferior): Update.
	(free_vector_of_ints): Remove.
	(list_available_thread_groups): Change "ids" to std::set.
	(mi_cmd_list_thread_groups): Update.
	(struct collect_cores_data) <core>: Now a std::set.
	(collect_cores): Update.
	(unique): Remove.
	(print_one_inferior): Update.
---
 gdb/ChangeLog    | 13 ++++++++++
 gdb/mi/mi-main.c | 77 +++++++++++++-------------------------------------------
 2 files changed, 31 insertions(+), 59 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 6368f41..54928fc 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,18 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (struct print_one_inferior_data) <inferiors>: Now a
+	'std::set *'.
+	(print_one_inferior): Update.
+	(free_vector_of_ints): Remove.
+	(list_available_thread_groups): Change "ids" to std::set.
+	(mi_cmd_list_thread_groups): Update.
+	(struct collect_cores_data) <core>: Now a std::set.
+	(collect_cores): Update.
+	(unique): Remove.
+	(print_one_inferior): Update.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (mi_execute_cli_command): Use unique_xmalloc_ptr.
 	(mi_execute_async_cli_command): Likewise.
 	(mi_cmd_trace_frame_collected): Use std::string.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0147fb9..3d73446 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -62,6 +62,8 @@
 #include <chrono>
 #include "progspace-and-thread.h"
 #include "common/rsp-low.h"
+#include <algorithm>
+#include <set>
 
 enum
   {
@@ -604,8 +606,7 @@ mi_cmd_thread_info (const char *command, char **argv, int argc)
 struct collect_cores_data
 {
   int pid;
-
-  VEC (int) *cores;
+  std::set<int> cores;
 };
 
 static int
@@ -618,27 +619,16 @@ collect_cores (struct thread_info *ti, void *xdata)
       int core = target_core_of_thread (ti->ptid);
 
       if (core != -1)
-	VEC_safe_push (int, data->cores, core);
+	data->cores.insert (core);
     }
 
   return 0;
 }
 
-static int *
-unique (int *b, int *e)
-{
-  int *d = b;
-
-  while (++b != e)
-    if (*d != *b)
-      *++d = *b;
-  return ++d;
-}
-
 struct print_one_inferior_data
 {
   int recurse;
-  VEC (int) *inferiors;
+  const std::set<int> *inferiors;
 };
 
 static int
@@ -648,10 +638,9 @@ print_one_inferior (struct inferior *inferior, void *xdata)
     = (struct print_one_inferior_data *) xdata;
   struct ui_out *uiout = current_uiout;
 
-  if (VEC_empty (int, top_data->inferiors)
-      || bsearch (&(inferior->pid), VEC_address (int, top_data->inferiors),
-		  VEC_length (int, top_data->inferiors), sizeof (int),
-		  compare_positive_ints))
+  if (top_data->inferiors->empty ()
+      || (top_data->inferiors->find (inferior->pid)
+	  != top_data->inferiors->end ()))
     {
       struct collect_cores_data data;
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
@@ -670,28 +659,18 @@ print_one_inferior (struct inferior *inferior, void *xdata)
 			       inferior->pspace->pspace_exec_filename);
 	}
 
-      data.cores = 0;
       if (inferior->pid != 0)
 	{
 	  data.pid = inferior->pid;
 	  iterate_over_threads (collect_cores, &data);
 	}
 
-      if (!VEC_empty (int, data.cores))
+      if (!data.cores.empty ())
 	{
-	  int *b, *e;
 	  ui_out_emit_list list_emitter (uiout, "cores");
 
-	  qsort (VEC_address (int, data.cores),
-		 VEC_length (int, data.cores), sizeof (int),
-		 compare_positive_ints);
-
-	  b = VEC_address (int, data.cores);
-	  e = b + VEC_length (int, data.cores);
-	  e = unique (b, e);
-
-	  for (; b != e; ++b)
-	    uiout->field_int (NULL, *b);
+	  for (int b : data.cores)
+	    uiout->field_int (NULL, b);
 	}
 
       if (top_data->recurse)
@@ -717,14 +696,6 @@ output_cores (struct ui_out *uiout, const char *field_name, const char *xcores)
 }
 
 static void
-free_vector_of_ints (void *xvector)
-{
-  VEC (int) **vector = (VEC (int) **) xvector;
-
-  VEC_free (int, *vector);
-}
-
-static void
 do_nothing (splay_tree_key k)
 {
 }
@@ -755,7 +726,7 @@ free_splay_tree (void *xt)
 }
 
 static void
-list_available_thread_groups (VEC (int) *ids, int recurse)
+list_available_thread_groups (const std::set<int> &ids, int recurse)
 {
   struct osdata *data;
   struct osdata_item *item;
@@ -824,12 +795,9 @@ list_available_thread_groups (VEC (int) *ids, int recurse)
       /* At present, the target will return all available processes
 	 and if information about specific ones was required, we filter
 	 undesired processes here.  */
-      if (ids && bsearch (&pid_i, VEC_address (int, ids),
-			  VEC_length (int, ids),
-			  sizeof (int), compare_positive_ints) == NULL)
+      if (!ids.empty () && ids.find (pid_i) != ids.end ())
 	continue;
 
-
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
 
       uiout->field_fmt ("id", "%s", pid);
@@ -875,10 +843,9 @@ void
 mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
 {
   struct ui_out *uiout = current_uiout;
-  struct cleanup *back_to;
   int available = 0;
   int recurse = 0;
-  VEC (int) *ids = 0;
+  std::set<int> ids;
 
   enum opt
   {
@@ -930,23 +897,17 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
 
       if (*end != '\0')
 	error (_("invalid syntax of group id '%s'"), argv[oind]);
-      VEC_safe_push (int, ids, inf);
+      ids.insert (inf);
     }
-  if (VEC_length (int, ids) > 1)
-    qsort (VEC_address (int, ids),
-	   VEC_length (int, ids),
-	   sizeof (int), compare_positive_ints);
-
-  back_to = make_cleanup (free_vector_of_ints, &ids);
 
   if (available)
     {
       list_available_thread_groups (ids, recurse);
     }
-  else if (VEC_length (int, ids) == 1)
+  else if (ids.size () == 1)
     {
       /* Local thread groups, single id.  */
-      int id = *VEC_address (int, ids);
+      int id = *(ids.begin ());
       struct inferior *inf = find_inferior_id (id);
 
       if (!inf)
@@ -959,7 +920,7 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
       struct print_one_inferior_data data;
 
       data.recurse = recurse;
-      data.inferiors = ids;
+      data.inferiors = &ids;
 
       /* Local thread groups.  Either no explicit ids -- and we
 	 print everything, or several explicit ids.  In both cases,
@@ -969,8 +930,6 @@ mi_cmd_list_thread_groups (const char *command, char **argv, int argc)
       update_thread_list ();
       iterate_over_inferiors (print_one_inferior, &data);
     }
-
-  do_cleanups (back_to);
 }
 
 void
-- 
2.9.4

Re: [RFA 09/11] Use std::set in mi-main.c

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change a couple of spots in mi-main.c to use std::set.  This
> simplifies the code and removes some cleanups.

std::set always gives me pause.  For small objects like int,
and when the use case is insertion phase + lookup phase + discard set,
unsorted inserting into a vector, sorting, and then binary searching
the vector for lookuups is very likely to have better performance, for
cache locality reasons, and also because fewer allocations (with
std::set being a node-based container...)

But it's likely that in this case it doesn't really matter, so let's
go with the simplicity argument.

(At some point I may propose some data structure on top of
std::vector for use cases like this.)

Patch is OK as is.

Thanks,
Pedro Alves

Re: [RFA 09/11] Use std::set in mi-main.c

[Simon Marchi]

On 2017-09-28 12:10, Pedro Alves wrote:
> On 09/12/2017 07:57 PM, Tom Tromey wrote:
>> Change a couple of spots in mi-main.c to use std::set.  This
>> simplifies the code and removes some cleanups.
> 
> std::set always gives me pause.  For small objects like int,
> and when the use case is insertion phase + lookup phase + discard set,
> unsorted inserting into a vector, sorting, and then binary searching
> the vector for lookuups is very likely to have better performance, for
> cache locality reasons, and also because fewer allocations (with
> std::set being a node-based container...)
> 
> But it's likely that in this case it doesn't really matter, so let's
> go with the simplicity argument.
> 
> (At some point I may propose some data structure on top of
> std::vector for use cases like this.)

We have discussed something like this with Sergio in this thread:

https://sourceware.org/ml/gdb-patches/2017-07/msg00434.html

(you can ctrl-f "boilerplate")

We managed to sneak in an std::set while you were not looking/on 
vacation :).  The idea was that the std::set interface really helped to 
keep the code clean and short, and that we could later implement 
something with the same behavior and interface as std::set, but based on 
a vector.  It's a bit different than what you propose, because to be a 
drop-in replacement for a set, we would always keep the vector sorted 
(insert the new elements where they belong).  IIUC, what you propose is 
to insert everything and then sort it.  I think the latter has a better 
worst-case performance (sorting at the end is n*log(n)) compared to the 
former (insertion in reverse order will become n^2).  I don't think it 
matters much though, since this data structure would be explicitly for 
relatively small amount of items.

I am not sure if the reasoning is different for a set of string (as in 
common/environ.{h,c}).  I assume it would be similar, since the actual 
string object is rather small, and when inserting a string in the 
middle, the following strings will be moved one position and not copied 
(I haven't verified though).

Simon

Re: [RFA 09/11] Use std::set in mi-main.c

[Simon Marchi]

On 2017-09-12 20:57, Tom Tromey wrote:
>  static void
> -list_available_thread_groups (VEC (int) *ids, int recurse)
> +list_available_thread_groups (const std::set<int> &ids, int recurse)
>  {
>    struct osdata *data;
>    struct osdata_item *item;
> @@ -824,12 +795,9 @@ list_available_thread_groups (VEC (int) *ids, int 
> recurse)
>        /* At present, the target will return all available processes
>  	 and if information about specific ones was required, we filter
>  	 undesired processes here.  */
> -      if (ids && bsearch (&pid_i, VEC_address (int, ids),
> -			  VEC_length (int, ids),
> -			  sizeof (int), compare_positive_ints) == NULL)
> +      if (!ids.empty () && ids.find (pid_i) != ids.end ())

I think the condition is the wrong way, it should be == and not !=.  It 
probably means we don't have a test for this feature.

Simon

Re: [RFA 09/11] Use std::set in mi-main.c

[Tom Tromey]

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

Simon> On 2017-09-12 20:57, Tom Tromey wrote:
>> static void
>> -list_available_thread_groups (VEC (int) *ids, int recurse)
>> +list_available_thread_groups (const std::set<int> &ids, int recurse)
>> {
>> struct osdata *data;
>> struct osdata_item *item;
>> @@ -824,12 +795,9 @@ list_available_thread_groups (VEC (int) *ids,
>> int recurse)
>> /* At present, the target will return all available processes
>> and if information about specific ones was required, we filter
>> undesired processes here.  */
>> -      if (ids && bsearch (&pid_i, VEC_address (int, ids),
>> -			  VEC_length (int, ids),
>> -			  sizeof (int), compare_positive_ints) == NULL)
>> +      if (!ids.empty () && ids.find (pid_i) != ids.end ())

Simon> I think the condition is the wrong way, it should be == and not !=.
Simon> It probably means we don't have a test for this feature.

Yes, I think you're right.  Changing it to == doesn't affect the gdb.mi
test results for me.

Tom

[RFA 11/11] Change captured_mi_execute_command to use scoped_restore

[Tom Tromey]

Change captured_mi_execute_command to use a scoped_restore, removing a
cleanup.  The old code copied the current token, but I don't believe
that is necessary.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (captured_mi_execute_command): Use scope_restore.
---
 gdb/ChangeLog    | 4 ++++
 gdb/mi/mi-main.c | 9 +++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index ee965c4..773f35d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (captured_mi_execute_command): Use scope_restore.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Update.
 	* ada-lang.h (struct ada_exc_info): Remove typedef.  Declare
 	operator< and operator==.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 3d73446..3e5eca2 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1870,20 +1870,19 @@ mi_cmd_remove_inferior (const char *command, char **argv, int argc)
    Return <0 for error; >=0 for ok.
 
    args->action will tell mi_execute_command what action
-   to perfrom after the given command has executed (display/suppress
+   to perform after the given command has executed (display/suppress
    prompt, display error).  */
 
 static void
 captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
 {
   struct mi_interp *mi = (struct mi_interp *) command_interp ();
-  struct cleanup *cleanup;
 
   if (do_timings)
     current_command_ts = context->cmd_start;
 
-  current_token = xstrdup (context->token);
-  cleanup = make_cleanup (free_current_contents, &current_token);
+  scoped_restore save_token = make_scoped_restore (&current_token,
+						   context->token);
 
   running_result_record_printed = 0;
   mi_proceeded = 0;
@@ -1959,8 +1958,6 @@ captured_mi_execute_command (struct ui_out *uiout, struct mi_parse *context)
 	break;
       }
     }
-
-  do_cleanups (cleanup);
 }
 
 /* Print a gdb exception to the MI output stream.  */
-- 
2.9.4

Re: [RFA 11/11] Change captured_mi_execute_command to use scoped_restore

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change captured_mi_execute_command to use a scoped_restore, removing a
> cleanup.  The old code copied the current token, but I don't believe
> that is necessary.
> 

I tried to see if it was safe, and I think it is.  I wondered whether
the command to be executed could itself run an MI command with a token,
and whether that'd clobber the original token.  Like, e.g. (contrived):

 321-interpreter-exec mi "123-thread-info"

we currently get:

 123^done,threads=[]
 321^done
 (gdb)

Looks like that's OK because we create a separate parse object for each
of the commands.

> ChangeLog
> 2017-09-12  Tom Tromey  <tom@tromey.com>
> 
> 	* mi/mi-main.c (captured_mi_execute_command): Use scope_restore.

OK.

Thanks,
Pedro Alves

[RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes

[Tom Tromey]

This changes mi_cmd_data_write_memory_bytes to use gdb::byte_vector,
removing some cleanups.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
	gdb::byte_vector.
---
 gdb/ChangeLog    |  5 +++++
 gdb/mi/mi-main.c | 20 +++++++-------------
 2 files changed, 12 insertions(+), 13 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 47632fa..7158579 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
+	gdb::byte_vector.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* thread.c (gdb_list_thread_ids, gdb_thread_select): Change
 	error_message to std::string.
 	* mi/mi-main.c (mi_cmd_thread_select): Use std::string.
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index d01f578..c8c4a97 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -1710,11 +1710,8 @@ mi_cmd_data_write_memory_bytes (const char *command, char **argv, int argc)
 {
   CORE_ADDR addr;
   char *cdata;
-  gdb_byte *data;
-  gdb_byte *databuf;
   size_t len_hex, len_bytes, len_units, i, steps, remaining_units;
   long int count_units;
-  struct cleanup *back_to;
   int unit_size;
 
   if (argc != 2 && argc != 3)
@@ -1738,8 +1735,7 @@ mi_cmd_data_write_memory_bytes (const char *command, char **argv, int argc)
   else
     count_units = len_units;
 
-  databuf = XNEWVEC (gdb_byte, len_bytes);
-  back_to = make_cleanup (xfree, databuf);
+  gdb::byte_vector databuf (len_bytes);
 
   for (i = 0; i < len_bytes; ++i)
     {
@@ -1749,34 +1745,32 @@ mi_cmd_data_write_memory_bytes (const char *command, char **argv, int argc)
       databuf[i] = (gdb_byte) x;
     }
 
+  gdb::byte_vector data;
   if (len_units < count_units)
     {
       /* Pattern is made of less units than count:
          repeat pattern to fill memory.  */
-      data = (gdb_byte *) xmalloc (count_units * unit_size);
-      make_cleanup (xfree, data);
+      data = gdb::byte_vector (count_units * unit_size);
 
       /* Number of times the pattern is entirely repeated.  */
       steps = count_units / len_units;
       /* Number of remaining addressable memory units.  */
       remaining_units = count_units % len_units;
       for (i = 0; i < steps; i++)
-        memcpy (data + i * len_bytes, databuf, len_bytes);
+        memcpy (&data[i * len_bytes], &databuf[0], len_bytes);
 
       if (remaining_units > 0)
-        memcpy (data + steps * len_bytes, databuf,
+        memcpy (&data[steps * len_bytes], &databuf[0],
 		remaining_units * unit_size);
     }
   else
     {
       /* Pattern is longer than or equal to count:
          just copy count addressable memory units.  */
-      data = databuf;
+      data = std::move (databuf);
     }
 
-  write_memory_with_notification (addr, data, count_units);
-
-  do_cleanups (back_to);
+  write_memory_with_notification (addr, data.data (), count_units);
 }
 
 void
-- 
2.9.4

Re: [RFA 07/11] Use gdb::byte_vector in mi_cmd_data_write_memory_bytes

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This changes mi_cmd_data_write_memory_bytes to use gdb::byte_vector,
> removing some cleanups.

Thanks.

>  
> +  gdb::byte_vector data;
>    if (len_units < count_units)
>      {
>        /* Pattern is made of less units than count:
>           repeat pattern to fill memory.  */
> -      data = (gdb_byte *) xmalloc (count_units * unit_size);
> -      make_cleanup (xfree, data);
> +      data = gdb::byte_vector (count_units * unit_size);

I think I'd have written instead:

  data.resize (count_units * unit_size)

But OK either way.

Thanks,
Pedro Alves

[RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c

[Tom Tromey]

Change a couple of spots in mi-main.c to use std::string or
unique_xmalloc_ptr.  unique_xmalloc_ptr is used here where the string
is writeable; I generally prefer to pretend that string is read-only,
perhaps not for a good resaon.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-main.c (mi_execute_cli_command): Use unique_xmalloc_ptr.
	(mi_execute_async_cli_command): Likewise.
	(mi_cmd_trace_frame_collected): Use std::string.
---
 gdb/ChangeLog    |  6 ++++++
 gdb/mi/mi-main.c | 40 +++++++++++-----------------------------
 2 files changed, 17 insertions(+), 29 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 7158579..6368f41 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (mi_execute_cli_command): Use unique_xmalloc_ptr.
+	(mi_execute_async_cli_command): Likewise.
+	(mi_cmd_trace_frame_collected): Use std::string.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
 	gdb::byte_vector.
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index c8c4a97..0147fb9 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2264,41 +2264,31 @@ mi_execute_cli_command (const char *cmd, int args_p, const char *args)
 {
   if (cmd != 0)
     {
-      struct cleanup *old_cleanups;
-      char *run;
+      gdb::unique_xmalloc_ptr<char> run;
 
       if (args_p)
-	run = xstrprintf ("%s %s", cmd, args);
+	run.reset (xstrprintf ("%s %s", cmd, args));
       else
-	run = xstrdup (cmd);
+	run.reset (xstrdup (cmd));
       if (mi_debug_p)
 	/* FIXME: gdb_???? */
 	fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
-			    cmd, run);
-      old_cleanups = make_cleanup (xfree, run);
-      execute_command (run, 0 /* from_tty */ );
-      do_cleanups (old_cleanups);
-      return;
+			    cmd, run.get ());
+      execute_command (run.get (), 0 /* from_tty */ );
     }
 }
 
 void
 mi_execute_async_cli_command (const char *cli_command, char **argv, int argc)
 {
-  struct cleanup *old_cleanups;
-  char *run;
+  gdb::unique_xmalloc_ptr<char> run;
 
   if (mi_async_p ())
-    run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
+    run.reset (xstrprintf ("%s %s&", cli_command, argc ? *argv : ""));
   else
-    run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
-  old_cleanups = make_cleanup (xfree, run);
-
-  execute_command (run, 0 /* from_tty */ );
+    run.reset (xstrprintf ("%s %s", cli_command, argc ? *argv : ""));
 
-  /* Do this before doing any printing.  It would appear that some
-     print code leaves garbage around in the buffer.  */
-  do_cleanups (old_cleanups);
+  execute_command (run.get (), 0 /* from_tty */ );
 }
 
 void
@@ -2804,14 +2794,10 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
   {
     struct cleanup *cleanups;
     int tvar;
-    char *tsvname;
     int i;
 
     ui_out_emit_list list_emitter (uiout, "tvars");
 
-    tsvname = NULL;
-    cleanups = make_cleanup (free_current_contents, &tsvname);
-
     for (i = 0; VEC_iterate (int, tinfo->tvars, i, tvar); i++)
       {
 	struct trace_state_variable *tsv;
@@ -2822,10 +2808,8 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
 	if (tsv != NULL)
 	  {
-	    tsvname = (char *) xrealloc (tsvname, strlen (tsv->name) + 2);
-	    tsvname[0] = '$';
-	    strcpy (tsvname + 1, tsv->name);
-	    uiout->field_string ("name", tsvname);
+	    std::string tsvname = std::string ("$") + tsv->name;
+	    uiout->field_string ("name", tsvname.c_str ());
 
 	    tsv->value_known = target_get_trace_state_variable_value (tsv->number,
 								      &tsv->value);
@@ -2837,8 +2821,6 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 	    uiout->field_skip ("current");
 	  }
       }
-
-    do_cleanups (cleanups);
   }
 
   /* Memory.  */
-- 
2.9.4

Re: [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change a couple of spots in mi-main.c to use std::string or
> unique_xmalloc_ptr.  unique_xmalloc_ptr is used here where the string
> is writeable; I generally prefer to pretend that string is read-only,
> perhaps not for a good resaon.

Yes, I think not for a good reason.  :-)  You can
pass &str[0] to execute_command get access to the underlying
modifiable raw string.  See b064640146bb for example.

> diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
> index c8c4a97..0147fb9 100644
> --- a/gdb/mi/mi-main.c
> +++ b/gdb/mi/mi-main.c
> @@ -2264,41 +2264,31 @@ mi_execute_cli_command (const char *cmd, int args_p, const char *args)
>  {
>    if (cmd != 0)
>      {
> -      struct cleanup *old_cleanups;
> -      char *run;
> +      gdb::unique_xmalloc_ptr<char> run;
>  
>        if (args_p)
> -	run = xstrprintf ("%s %s", cmd, args);
> +	run.reset (xstrprintf ("%s %s", cmd, args));
>        else
> -	run = xstrdup (cmd);
> +	run.reset (xstrdup (cmd));
>        if (mi_debug_p)
>  	/* FIXME: gdb_???? */
>  	fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
> -			    cmd, run);
> -      old_cleanups = make_cleanup (xfree, run);
> -      execute_command (run, 0 /* from_tty */ );
> -      do_cleanups (old_cleanups);
> -      return;
> +			    cmd, run.get ());
> +      execute_command (run.get (), 0 /* from_tty */ );
>      }
>  }
>  
>  void
>  mi_execute_async_cli_command (const char *cli_command, char **argv, int argc)
>  {
> -  struct cleanup *old_cleanups;
> -  char *run;
> +  gdb::unique_xmalloc_ptr<char> run;
>  
>    if (mi_async_p ())
> -    run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
> +    run.reset (xstrprintf ("%s %s&", cli_command, argc ? *argv : ""));
>    else
> -    run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
> -  old_cleanups = make_cleanup (xfree, run);
> -
> -  execute_command (run, 0 /* from_tty */ );
> +    run.reset (xstrprintf ("%s %s", cli_command, argc ? *argv : ""));
>  
> -  /* Do this before doing any printing.  It would appear that some
> -     print code leaves garbage around in the buffer.  */
> -  do_cleanups (old_cleanups);
> +  execute_command (run.get (), 0 /* from_tty */ );
>  }
>  

I think the above would look better with std::string + string_printf.


>  	struct trace_state_variable *tsv;
> @@ -2822,10 +2808,8 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
>  
>  	if (tsv != NULL)
>  	  {
> -	    tsvname = (char *) xrealloc (tsvname, strlen (tsv->name) + 2);
> -	    tsvname[0] = '$';
> -	    strcpy (tsvname + 1, tsv->name);
> -	    uiout->field_string ("name", tsvname);
> +	    std::string tsvname = std::string ("$") + tsv->name;
> +	    uiout->field_string ("name", tsvname.c_str ());

How about replacing this string building + field_string with a single
call to:

	    uiout->field_fmt ("name", "$%s", tsv->name);

Thanks,
Pedro Alves

Re: [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c

[Tom Tromey]

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

Pedro> How about replacing this string building + field_string with a single
Pedro> call to:
Pedro> uiout-> field_fmt ("name", "$%s", tsv->name);

Here's an update.

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 9cd584b..1edd8ee 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,11 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-main.c (mi_execute_cli_command): Use std::string.
+	(mi_execute_async_cli_command): Likewise.
+	(mi_cmd_trace_frame_collected): Use field_fmt.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (mi_cmd_data_write_memory_bytes): Use
 	gdb::byte_vector.
 
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index e1ba8e2..2d560a4 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -2269,41 +2269,29 @@ mi_execute_cli_command (const char *cmd, int args_p, const char *args)
 {
   if (cmd != 0)
     {
-      struct cleanup *old_cleanups;
-      char *run;
+      std::string run = cmd;
 
       if (args_p)
-	run = xstrprintf ("%s %s", cmd, args);
-      else
-	run = xstrdup (cmd);
+	run = run + " " + args;
       if (mi_debug_p)
 	/* FIXME: gdb_???? */
 	fprintf_unfiltered (gdb_stdout, "cli=%s run=%s\n",
-			    cmd, run);
-      old_cleanups = make_cleanup (xfree, run);
-      execute_command (run, 0 /* from_tty */ );
-      do_cleanups (old_cleanups);
-      return;
+			    cmd, run.c_str ());
+      execute_command (&run[0], 0 /* from_tty */ );
     }
 }
 
 void
 mi_execute_async_cli_command (const char *cli_command, char **argv, int argc)
 {
-  struct cleanup *old_cleanups;
-  char *run;
+  std::string run = cli_command;
 
+  if (argc)
+    run = run + " " + *argv;
   if (mi_async_p ())
-    run = xstrprintf ("%s %s&", cli_command, argc ? *argv : "");
-  else
-    run = xstrprintf ("%s %s", cli_command, argc ? *argv : "");
-  old_cleanups = make_cleanup (xfree, run);
+    run += "&";
 
-  execute_command (run, 0 /* from_tty */ );
-
-  /* Do this before doing any printing.  It would appear that some
-     print code leaves garbage around in the buffer.  */
-  do_cleanups (old_cleanups);
+  execute_command (&run[0], 0 /* from_tty */ );
 }
 
 void
@@ -2806,14 +2794,10 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
   {
     struct cleanup *cleanups;
     int tvar;
-    char *tsvname;
     int i;
 
     ui_out_emit_list list_emitter (uiout, "tvars");
 
-    tsvname = NULL;
-    cleanups = make_cleanup (free_current_contents, &tsvname);
-
     for (i = 0; VEC_iterate (int, tinfo->tvars, i, tvar); i++)
       {
 	struct trace_state_variable *tsv;
@@ -2824,10 +2808,7 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 
 	if (tsv != NULL)
 	  {
-	    tsvname = (char *) xrealloc (tsvname, strlen (tsv->name) + 2);
-	    tsvname[0] = '$';
-	    strcpy (tsvname + 1, tsv->name);
-	    uiout->field_string ("name", tsvname);
+	    uiout->field_fmt ("name", "$%s", tsv->name);
 
 	    tsv->value_known = target_get_trace_state_variable_value (tsv->number,
 								      &tsv->value);
@@ -2839,8 +2820,6 @@ mi_cmd_trace_frame_collected (const char *command, char **argv, int argc)
 	    uiout->field_skip ("current");
 	  }
       }
-
-    do_cleanups (cleanups);
   }
 
   /* Memory.  */

Re: [RFA 08/11] Use string and unique_xmalloc_ptr in mi-main.c

[Pedro Alves]

On 09/29/2017 02:41 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> How about replacing this string building + field_string with a single
> Pedro> call to:
> Pedro> uiout-> field_fmt ("name", "$%s", tsv->name);
> 
> Here's an update.

OK.

Thanks,
Pedro Alves

[RFA 06/11] Change some gdb_* functions to use a std::string out parameter

[Tom Tromey]

This changes a few gdb_* functions to use a std::string out parameter.
Perhaps these functions should just go away entirely; I think they're
vesitiges of a now-defunct libgdb plan.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* thread.c (gdb_list_thread_ids, gdb_thread_select): Change
	error_message to std::string.
	* mi/mi-main.c (mi_cmd_thread_select): Use std::string.
	(mi_cmd_thread_list_ids): Likewise.
	* gdb.h (gdb_breakpoint_query, gdb_thread_select)
	(gdb_list_thread_ids): Update.
	* exceptions.h (catch_exceptions_with_msg): Update.
	* exceptions.c (catch_exceptions_with_msg): Change error_message
	to std::string.
	* breakpoint.c (enum gdb_rc): Change error_message to
	std::string.
---
 gdb/ChangeLog    | 14 ++++++++++++++
 gdb/breakpoint.c |  2 +-
 gdb/exceptions.c | 11 +++--------
 gdb/exceptions.h |  2 +-
 gdb/gdb.h        |  6 +++---
 gdb/mi/mi-main.c | 14 ++++----------
 gdb/thread.c     |  4 ++--
 7 files changed, 28 insertions(+), 25 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index e1c409d..47632fa 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,19 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* thread.c (gdb_list_thread_ids, gdb_thread_select): Change
+	error_message to std::string.
+	* mi/mi-main.c (mi_cmd_thread_select): Use std::string.
+	(mi_cmd_thread_list_ids): Likewise.
+	* gdb.h (gdb_breakpoint_query, gdb_thread_select)
+	(gdb_list_thread_ids): Update.
+	* exceptions.h (catch_exceptions_with_msg): Update.
+	* exceptions.c (catch_exceptions_with_msg): Change error_message
+	to std::string.
+	* breakpoint.c (enum gdb_rc): Change error_message to
+	std::string.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-parse.c (mi_parse): Remove unused declaration.
 
 2017-09-12  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c
index 235dab4..4c58a8c 100644
--- a/gdb/breakpoint.c
+++ b/gdb/breakpoint.c
@@ -6750,7 +6750,7 @@ do_captured_breakpoint_query (struct ui_out *uiout, void *data)
 
 enum gdb_rc
 gdb_breakpoint_query (struct ui_out *uiout, int bnum, 
-		      char **error_message)
+		      std::string *error_message)
 {
   struct captured_breakpoint_query_args args;
 
diff --git a/gdb/exceptions.c b/gdb/exceptions.c
index f9a80a0..0385bec 100644
--- a/gdb/exceptions.c
+++ b/gdb/exceptions.c
@@ -167,7 +167,7 @@ int
 catch_exceptions_with_msg (struct ui_out *func_uiout,
 		  	   catch_exceptions_ftype *func,
 		  	   void *func_args,
-			   char **gdberrmsg,
+			   std::string *gdberrmsg,
 		  	   return_mask mask)
 {
   struct gdb_exception exception = exception_none;
@@ -206,13 +206,8 @@ catch_exceptions_with_msg (struct ui_out *func_uiout,
       /* If caller wants a copy of the low-level error message, make
 	 one.  This is used in the case of a silent error whereby the
 	 caller may optionally want to issue the message.  */
-      if (gdberrmsg != NULL)
-	{
-	  if (exception.message != NULL)
-	    *gdberrmsg = xstrdup (exception.message);
-	  else
-	    *gdberrmsg = NULL;
-	}
+      if (gdberrmsg != NULL && exception.message != NULL)
+	*gdberrmsg = exception.message;
       return exception.reason;
     }
   return val;
diff --git a/gdb/exceptions.h b/gdb/exceptions.h
index b2cdee3..e1a517b 100644
--- a/gdb/exceptions.h
+++ b/gdb/exceptions.h
@@ -73,7 +73,7 @@ typedef void (catch_exception_ftype) (struct ui_out *ui_out, void *args);
 extern int catch_exceptions_with_msg (struct ui_out *uiout,
 			     	      catch_exceptions_ftype *func, 
 			     	      void *func_args,
-			     	      char **gdberrmsg,
+				      std::string *gdberrmsg,
 				      return_mask mask);
 
 /* If CATCH_ERRORS_FTYPE throws an error, catch_errors() returns zero
diff --git a/gdb/gdb.h b/gdb/gdb.h
index ac1e683..9403823 100644
--- a/gdb/gdb.h
+++ b/gdb/gdb.h
@@ -45,14 +45,14 @@ enum gdb_rc {
 /* Print the specified breakpoint on GDB_STDOUT.  (Eventually this
    function will ``print'' the object on ``output'').  */
 enum gdb_rc gdb_breakpoint_query (struct ui_out *uiout, int bnum,
-				  char **error_message);
+				  std::string *error_message);
 
 /* Switch thread and print notification.  */
 enum gdb_rc gdb_thread_select (struct ui_out *uiout, char *tidstr,
-			       char **error_message);
+			       std::string *error_message);
 
 /* Print a list of known thread ids.  */
 enum gdb_rc gdb_list_thread_ids (struct ui_out *uiout,
-				 char **error_message);
+				 std::string *error_message);
 
 #endif
diff --git a/gdb/mi/mi-main.c b/gdb/mi/mi-main.c
index 0ee2605..d01f578 100644
--- a/gdb/mi/mi-main.c
+++ b/gdb/mi/mi-main.c
@@ -554,7 +554,7 @@ void
 mi_cmd_thread_select (const char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
-  char *mi_error_message;
+  std::string mi_error_message;
   ptid_t previous_ptid = inferior_ptid;
 
   if (argc != 1)
@@ -564,10 +564,7 @@ mi_cmd_thread_select (const char *command, char **argv, int argc)
 
   /* If thread switch did not succeed don't notify or print.  */
   if (rc == GDB_RC_FAIL)
-    {
-      make_cleanup (xfree, mi_error_message);
-      error ("%s", mi_error_message);
-    }
+    error ("%s", mi_error_message.c_str ());
 
   print_selected_thread_frame (current_uiout,
 			       USER_SELECTED_THREAD | USER_SELECTED_FRAME);
@@ -584,7 +581,7 @@ void
 mi_cmd_thread_list_ids (const char *command, char **argv, int argc)
 {
   enum gdb_rc rc;
-  char *mi_error_message;
+  std::string mi_error_message;
 
   if (argc != 0)
     error (_("-thread-list-ids: No arguments required."));
@@ -592,10 +589,7 @@ mi_cmd_thread_list_ids (const char *command, char **argv, int argc)
   rc = gdb_list_thread_ids (current_uiout, &mi_error_message);
 
   if (rc == GDB_RC_FAIL)
-    {
-      make_cleanup (xfree, mi_error_message);
-      error ("%s", mi_error_message);
-    }
+    error ("%s", mi_error_message.c_str ());
 }
 
 void
diff --git a/gdb/thread.c b/gdb/thread.c
index 2539d43..a378c13 100644
--- a/gdb/thread.c
+++ b/gdb/thread.c
@@ -744,7 +744,7 @@ do_captured_list_thread_ids (struct ui_out *uiout, void *arg)
 /* Official gdblib interface function to get a list of thread ids and
    the total number.  */
 enum gdb_rc
-gdb_list_thread_ids (struct ui_out *uiout, char **error_message)
+gdb_list_thread_ids (struct ui_out *uiout, std::string *error_message)
 {
   if (catch_exceptions_with_msg (uiout, do_captured_list_thread_ids, NULL,
 				 error_message, RETURN_MASK_ALL) < 0)
@@ -2056,7 +2056,7 @@ print_selected_thread_frame (struct ui_out *uiout,
 }
 
 enum gdb_rc
-gdb_thread_select (struct ui_out *uiout, char *tidstr, char **error_message)
+gdb_thread_select (struct ui_out *uiout, char *tidstr, std::string *error_message)
 {
   if (catch_exceptions_with_msg (uiout, do_captured_thread_select, tidstr,
 				 error_message, RETURN_MASK_ALL) < 0)
-- 
2.9.4

Re: [RFA 06/11] Change some gdb_* functions to use a std::string out parameter

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This changes a few gdb_* functions to use a std::string out parameter.
> Perhaps these functions should just go away entirely; I think they're
> vesitiges of a now-defunct libgdb plan.

I think it's better to drop this one and go with the full removal
as done in the other thread.

Thanks,
Pedro Alves

Re: [RFA 06/11] Change some gdb_* functions to use a std::string out parameter

[Tom Tromey]

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

Pedro> On 09/12/2017 07:57 PM, Tom Tromey wrote:
>> This changes a few gdb_* functions to use a std::string out parameter.
>> Perhaps these functions should just go away entirely; I think they're
>> vesitiges of a now-defunct libgdb plan.

Pedro> I think it's better to drop this one and go with the full removal
Pedro> as done in the other thread.

I agree, I'll drop it here.

Tom

[RFA 04/11] Don't copy a string in mi_cmd_disassemble

[Tom Tromey]

This string copy in mi_cmd_disassemble seems not to be needed, so
don't do it.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-cmd-disas.c (mi_cmd_disassemble): Don't copy "oarg".
---
 gdb/ChangeLog         | 4 ++++
 gdb/mi/mi-cmd-disas.c | 6 +-----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b1fdcda..c981c7e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-cmd-disas.c (mi_cmd_disassemble): Don't copy "oarg".
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* varobj.h (varobj_gen_name): Return std::string.
 	* varobj.c (varobj_gen_name): Return std::string.
 	* mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
diff --git a/gdb/mi/mi-cmd-disas.c b/gdb/mi/mi-cmd-disas.c
index d0f9b0b..b3d6245 100644
--- a/gdb/mi/mi-cmd-disas.c
+++ b/gdb/mi/mi-cmd-disas.c
@@ -74,7 +74,6 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   int how_many = -1;
   CORE_ADDR low = 0;
   CORE_ADDR high = 0;
-  struct cleanup *cleanups = make_cleanup (null_cleanup, NULL);
 
   /* Options processing stuff.  */
   int oind = 0;
@@ -104,9 +103,8 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
       switch ((enum opt) opt)
 	{
 	case FILE_OPT:
-	  file_string = xstrdup (oarg);
+	  file_string = oarg;
 	  file_seen = 1;
-	  make_cleanup (xfree, file_string);
 	  break;
 	case LINE_OPT:
 	  line_num = atoi (oarg);
@@ -190,6 +188,4 @@ mi_cmd_disassemble (const char *command, char **argv, int argc)
   gdb_disassembly (gdbarch, uiout,
   		   disasm_flags,
 		   how_many, low, high);
-
-  do_cleanups (cleanups);
 }
-- 
2.9.4

Re: [RFA 04/11] Don't copy a string in mi_cmd_disassemble

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This string copy in mi_cmd_disassemble seems not to be needed, so
> don't do it.

I read mi-getopt.h|c and convinced myself that you are right.

OK.

Thanks,
Pedro Alves

[RFA 05/11] Remove unused declaration

[Tom Tromey]

There was a leftover cleanup declaration in mi_parse.  Remove it.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-parse.c (mi_parse): Remove unused declaration.
---
 gdb/ChangeLog     | 4 ++++
 gdb/mi/mi-parse.c | 1 -
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index c981c7e..e1c409d 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,9 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-parse.c (mi_parse): Remove unused declaration.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-cmd-disas.c (mi_cmd_disassemble): Don't copy "oarg".
 
 2017-09-12  Tom Tromey  <tom@tromey.com>
diff --git a/gdb/mi/mi-parse.c b/gdb/mi/mi-parse.c
index cf05fa0..9c1b033 100644
--- a/gdb/mi/mi-parse.c
+++ b/gdb/mi/mi-parse.c
@@ -237,7 +237,6 @@ std::unique_ptr<struct mi_parse>
 mi_parse (const char *cmd, char **token)
 {
   const char *chp;
-  struct cleanup *cleanup;
 
   std::unique_ptr<struct mi_parse> parse (new struct mi_parse);
 
-- 
2.9.4

Re: [RFA 05/11] Remove unused declaration

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> There was a leftover cleanup declaration in mi_parse.  Remove it.
> 

This is obvious.

Thanks,
Pedro Alves

[RFA 02/11] Remove cleanups from mi_cmd_break_insert_1

[Tom Tromey]

This changes mi_argv_to_format to return a string, allowing the
removal of some cleanups.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
	(mi_cmd_break_insert_1): Update.
---
 gdb/ChangeLog         |  5 +++++
 gdb/mi/mi-cmd-break.c | 17 +++++------------
 2 files changed, 10 insertions(+), 12 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index b71f23b..15a19bb8 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,10 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
+	(mi_cmd_break_insert_1): Update.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* target.h (make_scoped_defer_target_commit_resume): Update.
 	* target.c (make_scoped_defer_target_commit_resume): Rename from
 	make_cleanup_defer_target_commit_resume.  Return a
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index bae8711..4cd134c 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -86,12 +86,11 @@ setup_breakpoint_reporting (void)
 /* Convert arguments in ARGV to the string in "format",argv,argv...
    and return it.  */
 
-static char *
+static std::string
 mi_argv_to_format (char **argv, int argc)
 {
   int i;
-  struct obstack obstack;
-  char *ret;
+  auto_obstack obstack;
 
   obstack_init (&obstack);
 
@@ -152,10 +151,7 @@ mi_argv_to_format (char **argv, int argc)
     }
   obstack_1grow (&obstack, '\0');
 
-  ret = xstrdup ((const char *) obstack_finish (&obstack));
-  obstack_free (&obstack, NULL);
-
-  return ret;
+  return (const char *) obstack_finish (&obstack);
 }
 
 /* Insert breakpoint.
@@ -174,13 +170,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
-  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   enum bptype type_wanted;
   event_location_up location;
   struct breakpoint_ops *ops;
   int is_explicit = 0;
   struct explicit_location explicit_loc;
-  char *extra_string = NULL;
+  std::string extra_string;
 
   enum opt
     {
@@ -278,7 +273,6 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
 	error (_("-dprintf-insert: Missing <format>"));
 
       extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
-      make_cleanup (xfree, extra_string);
       address = argv[oind];
     }
   else
@@ -343,13 +337,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
     }
 
   create_breakpoint (get_current_arch (), location.get (), condition, thread,
-		     extra_string,
+		     extra_string.c_str (),
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
-  do_cleanups (back_to);
 }
 
 /* Implements the -break-insert command.
-- 
2.9.4

Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:

>  	* target.h (make_scoped_defer_target_commit_resume): Update.
>  	* target.c (make_scoped_defer_target_commit_resume): Rename from
>  	make_cleanup_defer_target_commit_resume.  Return a
> diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
> index bae8711..4cd134c 100644
> --- a/gdb/mi/mi-cmd-break.c
> +++ b/gdb/mi/mi-cmd-break.c
> @@ -86,12 +86,11 @@ setup_breakpoint_reporting (void)
>  /* Convert arguments in ARGV to the string in "format",argv,argv...
>     and return it.  */
>  
> -static char *
> +static std::string
>  mi_argv_to_format (char **argv, int argc)
>  {
>    int i;
> -  struct obstack obstack;
> -  char *ret;
> +  auto_obstack obstack;
>  
>    obstack_init (&obstack);

Remove the obstack_init call too.

>  
> @@ -152,10 +151,7 @@ mi_argv_to_format (char **argv, int argc)
>      }
>    obstack_1grow (&obstack, '\0');
>  
> -  ret = xstrdup ((const char *) obstack_finish (&obstack));
> -  obstack_free (&obstack, NULL);
> -
> -  return ret;
> +  return (const char *) obstack_finish (&obstack);
>  }

OK.

IMO, it'd be even better to build the std::string directly
instead of building an obstack and then dupping the string.

Thanks,
Pedro Alves

Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1

[Tom Tromey]

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

Pedro> IMO, it'd be even better to build the std::string directly
Pedro> instead of building an obstack and then dupping the string.

I made this change.
I'll send the new patch a bit later.

Tom

Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1

[Tom Tromey]

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

Pedro> IMO, it'd be even better to build the std::string directly
Pedro> instead of building an obstack and then dupping the string.

Here's the new patch.

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index d06ed22..1a58d82 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,8 @@
+2017-09-28  Tom Tromey  <tom@tromey.com>
+
+	* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
+	(mi_cmd_break_insert_1): Update.
+
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
 	* target.h (make_scoped_defer_target_commit_resume): Update.
diff --git a/gdb/mi/mi-cmd-break.c b/gdb/mi/mi-cmd-break.c
index 188e4e2..6519e29 100644
--- a/gdb/mi/mi-cmd-break.c
+++ b/gdb/mi/mi-cmd-break.c
@@ -86,76 +86,69 @@ setup_breakpoint_reporting (void)
 /* Convert arguments in ARGV to the string in "format",argv,argv...
    and return it.  */
 
-static char *
+static std::string
 mi_argv_to_format (char **argv, int argc)
 {
   int i;
-  struct obstack obstack;
-  char *ret;
-
-  obstack_init (&obstack);
+  std::string result;
 
   /* Convert ARGV[OIND + 1] to format string and save to FORMAT.  */
-  obstack_1grow (&obstack, '\"');
+  result += '\"';
   for (i = 0; i < strlen (argv[0]); i++)
     {
       switch (argv[0][i])
 	{
 	case '\\':
-	  obstack_grow (&obstack, "\\\\", 2);
+	  result += "\\\\";
 	  break;
 	case '\a':
-	  obstack_grow (&obstack, "\\a", 2);
+	  result += "\\a";
 	  break;
 	case '\b':
-	  obstack_grow (&obstack, "\\b", 2);
+	  result += "\\b";
 	  break;
 	case '\f':
-	  obstack_grow (&obstack, "\\f", 2);
+	  result += "\\f";
 	  break;
 	case '\n':
-	  obstack_grow (&obstack, "\\n", 2);
+	  result += "\\n";
 	  break;
 	case '\r':
-	  obstack_grow (&obstack, "\\r", 2);
+	  result += "\\r";
 	  break;
 	case '\t':
-	  obstack_grow (&obstack, "\\t", 2);
+	  result += "\\t";
 	  break;
 	case '\v':
-	  obstack_grow (&obstack, "\\v", 2);
+	  result += "\\v";
 	  break;
 	case '"':
-	  obstack_grow (&obstack, "\\\"", 2);
+	  result += "\\\"";
 	  break;
 	default:
 	  if (isprint (argv[0][i]))
-	    obstack_grow (&obstack, argv[0] + i, 1);
+	    result += argv[0][i];
 	  else
 	    {
 	      char tmp[5];
 
 	      xsnprintf (tmp, sizeof (tmp), "\\%o",
 			 (unsigned char) argv[0][i]);
-	      obstack_grow_str (&obstack, tmp);
+	      result += tmp;
 	    }
 	  break;
 	}
     }
-  obstack_1grow (&obstack, '\"');
+  result += '\"';
 
   /* Apply other argv to FORMAT.  */
   for (i = 1; i < argc; i++)
     {
-      obstack_1grow (&obstack, ',');
-      obstack_grow_str (&obstack, argv[i]);
+      result += ',';
+      result += argv[i];
     }
-  obstack_1grow (&obstack, '\0');
-
-  ret = xstrdup ((const char *) obstack_finish (&obstack));
-  obstack_free (&obstack, NULL);
 
-  return ret;
+  return result;
 }
 
 /* Insert breakpoint.
@@ -174,13 +167,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
   int pending = 0;
   int enabled = 1;
   int tracepoint = 0;
-  struct cleanup *back_to = make_cleanup (null_cleanup, NULL);
   enum bptype type_wanted;
   event_location_up location;
   struct breakpoint_ops *ops;
   int is_explicit = 0;
   struct explicit_location explicit_loc;
-  char *extra_string = NULL;
+  std::string extra_string;
 
   enum opt
     {
@@ -278,7 +270,6 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
 	error (_("-dprintf-insert: Missing <format>"));
 
       extra_string = mi_argv_to_format (argv + format_num, argc - format_num);
-      make_cleanup (xfree, extra_string);
       address = argv[oind];
     }
   else
@@ -343,13 +334,12 @@ mi_cmd_break_insert_1 (int dprintf, const char *command, char **argv, int argc)
     }
 
   create_breakpoint (get_current_arch (), location.get (), condition, thread,
-		     extra_string,
+		     extra_string.c_str (),
 		     0 /* condition and thread are valid.  */,
 		     temp_p, type_wanted,
 		     ignore_count,
 		     pending ? AUTO_BOOLEAN_TRUE : AUTO_BOOLEAN_FALSE,
 		     ops, 0, enabled, 0, 0);
-  do_cleanups (back_to);
 }
 
 /* Implements the -break-insert command.

Re: [RFA 02/11] Remove cleanups from mi_cmd_break_insert_1

[Pedro Alves]

On 09/29/2017 02:39 AM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> IMO, it'd be even better to build the std::string directly
> Pedro> instead of building an obstack and then dupping the string.
> 
> Here's the new patch.

OK, please push.  Thanks for doing this.

Thanks,
Pedro Alves

[RFA 10/11] Use a std::vector for ada_exceptions_list

[Tom Tromey]

Change ada_exceptions_list to return a std::vector and fix up the
users.  This allows removing a cleanup in MI.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Update.
	* ada-lang.h (struct ada_exc_info): Remove typedef.  Declare
	operator< and operator==.
	(ada_exceptions_list): Return a std::vector.
	* ada-lang.c (ada_exc_info::operator<): Rename from
	compare_ada_exception_info.
	(ada_exc_info::operator==): New.
	(sort_remove_dups_ada_exceptions_list): Change type of
	"exceptions".
	(ada_add_standard_exceptions, ada_add_exceptions_from_frame)
	(ada_add_global_exceptions): Likewise.
	(ada_exceptions_list_1): Return a std::vector.
	(ada_exceptions_list): Likewise.
---
 gdb/ChangeLog        | 16 ++++++++++
 gdb/ada-lang.c       | 88 ++++++++++++++++++++--------------------------------
 gdb/ada-lang.h       |  9 +++---
 gdb/mi/mi-cmd-info.c | 17 +++-------
 4 files changed, 60 insertions(+), 70 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 54928fc..ee965c4 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,21 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* mi/mi-cmd-info.c (mi_cmd_info_ada_exceptions): Update.
+	* ada-lang.h (struct ada_exc_info): Remove typedef.  Declare
+	operator< and operator==.
+	(ada_exceptions_list): Return a std::vector.
+	* ada-lang.c (ada_exc_info::operator<): Rename from
+	compare_ada_exception_info.
+	(ada_exc_info::operator==): New.
+	(sort_remove_dups_ada_exceptions_list): Change type of
+	"exceptions".
+	(ada_add_standard_exceptions, ada_add_exceptions_from_frame)
+	(ada_add_global_exceptions): Likewise.
+	(ada_exceptions_list_1): Return a std::vector.
+	(ada_exceptions_list): Likewise.
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-main.c (struct print_one_inferior_data) <inferiors>: Now a
 	'std::set *'.
 	(print_one_inferior): Update.
diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
index 64f1a33..cafba2d 100644
--- a/gdb/ada-lang.c
+++ b/gdb/ada-lang.c
@@ -62,6 +62,7 @@
 #include "cli/cli-utils.h"
 #include "common/function-view.h"
 #include "common/byte-vector.h"
+#include <algorithm>
 
 /* Define whether or not the C operator '/' truncates towards zero for
    differently signed operands (truncation direction is undefined in C).
@@ -13121,23 +13122,23 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
    The comparison is determined first by exception name, and then
    by exception address.  */
 
-static int
-compare_ada_exception_info (const void *a, const void *b)
+bool
+ada_exc_info::operator< (const ada_exc_info &other)
 {
-  const struct ada_exc_info *exc_a = (struct ada_exc_info *) a;
-  const struct ada_exc_info *exc_b = (struct ada_exc_info *) b;
   int result;
 
-  result = strcmp (exc_a->name, exc_b->name);
-  if (result != 0)
-    return result;
-
-  if (exc_a->addr < exc_b->addr)
-    return -1;
-  if (exc_a->addr > exc_b->addr)
-    return 1;
+  result = strcmp (name, other.name);
+  if (result < 0)
+    return true;
+  if (result == 0 && addr < other.addr)
+    return true;
+  return false;
+}
 
-  return 0;
+bool
+ada_exc_info::operator== (const ada_exc_info &other)
+{
+  return strcmp (name, other.name) == 0 && addr == other.addr;
 }
 
 /* Sort EXCEPTIONS using compare_ada_exception_info as the comparison
@@ -13146,23 +13147,12 @@ compare_ada_exception_info (const void *a, const void *b)
    All duplicates are also removed.  */
 
 static void
-sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions,
+sort_remove_dups_ada_exceptions_list (std::vector<ada_exc_info> *exceptions,
 				      int skip)
 {
-  struct ada_exc_info *to_sort
-    = VEC_address (ada_exc_info, *exceptions) + skip;
-  int to_sort_len
-    = VEC_length (ada_exc_info, *exceptions) - skip;
-  int i, j;
-
-  qsort (to_sort, to_sort_len, sizeof (struct ada_exc_info),
-	 compare_ada_exception_info);
-
-  for (i = 1, j = 1; i < to_sort_len; i++)
-    if (compare_ada_exception_info (&to_sort[i], &to_sort[j - 1]) != 0)
-      to_sort[j++] = to_sort[i];
-  to_sort_len = j;
-  VEC_truncate(ada_exc_info, *exceptions, skip + to_sort_len);
+  std::sort (exceptions->begin () + skip, exceptions->end ());
+  exceptions->erase (std::unique (exceptions->begin () + skip, exceptions->end ()),
+		     exceptions->end ());
 }
 
 /* Add all exceptions defined by the Ada standard whose name match
@@ -13177,7 +13167,7 @@ sort_remove_dups_ada_exceptions_list (VEC(ada_exc_info) **exceptions,
 
 static void
 ada_add_standard_exceptions (compiled_regex *preg,
-			     VEC(ada_exc_info) **exceptions)
+			     std::vector<ada_exc_info> *exceptions)
 {
   int i;
 
@@ -13194,7 +13184,7 @@ ada_add_standard_exceptions (compiled_regex *preg,
 	      struct ada_exc_info info
 		= {standard_exc[i], BMSYMBOL_VALUE_ADDRESS (msymbol)};
 
-	      VEC_safe_push (ada_exc_info, *exceptions, &info);
+	      exceptions->push_back (info);
 	    }
 	}
     }
@@ -13213,7 +13203,7 @@ ada_add_standard_exceptions (compiled_regex *preg,
 static void
 ada_add_exceptions_from_frame (compiled_regex *preg,
 			       struct frame_info *frame,
-			       VEC(ada_exc_info) **exceptions)
+			       std::vector<ada_exc_info> *exceptions)
 {
   const struct block *block = get_frame_block (frame, 0);
 
@@ -13236,7 +13226,7 @@ ada_add_exceptions_from_frame (compiled_regex *preg,
 		  struct ada_exc_info info = {SYMBOL_PRINT_NAME (sym),
 					      SYMBOL_VALUE_ADDRESS (sym)};
 
-		  VEC_safe_push (ada_exc_info, *exceptions, &info);
+		  exceptions->push_back (info);
 		}
 	    }
 	}
@@ -13276,7 +13266,7 @@ name_matches_regex (const char *name, compiled_regex *preg)
 
 static void
 ada_add_global_exceptions (compiled_regex *preg,
-			   VEC(ada_exc_info) **exceptions)
+			   std::vector<ada_exc_info> *exceptions)
 {
   struct objfile *objfile;
   struct compunit_symtab *s;
@@ -13311,7 +13301,7 @@ ada_add_global_exceptions (compiled_regex *preg,
 		struct ada_exc_info info
 		  = {SYMBOL_PRINT_NAME (sym), SYMBOL_VALUE_ADDRESS (sym)};
 
-		VEC_safe_push (ada_exc_info, *exceptions, &info);
+		exceptions->push_back (info);
 	      }
 	}
     }
@@ -13323,12 +13313,10 @@ ada_add_global_exceptions (compiled_regex *preg,
    If not NULL, PREG is used to filter out exceptions whose names
    do not match.  Otherwise, all exceptions are listed.  */
 
-static VEC(ada_exc_info) *
+static std::vector<ada_exc_info>
 ada_exceptions_list_1 (compiled_regex *preg)
 {
-  VEC(ada_exc_info) *result = NULL;
-  struct cleanup *old_chain
-    = make_cleanup (VEC_cleanup (ada_exc_info), &result);
+  std::vector<ada_exc_info> result;
   int prev_len;
 
   /* First, list the known standard exceptions.  These exceptions
@@ -13342,21 +13330,20 @@ ada_exceptions_list_1 (compiled_regex *preg)
 
   if (has_stack_frames ())
     {
-      prev_len = VEC_length (ada_exc_info, result);
+      prev_len = result.size ();
       ada_add_exceptions_from_frame (preg, get_selected_frame (NULL),
 				     &result);
-      if (VEC_length (ada_exc_info, result) > prev_len)
+      if (result.size () > prev_len)
 	sort_remove_dups_ada_exceptions_list (&result, prev_len);
     }
 
   /* Add all exceptions whose scope is global.  */
 
-  prev_len = VEC_length (ada_exc_info, result);
+  prev_len = result.size ();
   ada_add_global_exceptions (preg, &result);
-  if (VEC_length (ada_exc_info, result) > prev_len)
+  if (result.size () > prev_len)
     sort_remove_dups_ada_exceptions_list (&result, prev_len);
 
-  discard_cleanups (old_chain);
   return result;
 }
 
@@ -13374,7 +13361,7 @@ ada_exceptions_list_1 (compiled_regex *preg)
        alphabetical order;
      - Exceptions whose scope is global, in alphabetical order.  */
 
-VEC(ada_exc_info) *
+std::vector<ada_exc_info>
 ada_exceptions_list (const char *regexp)
 {
   if (regexp == NULL)
@@ -13389,14 +13376,9 @@ ada_exceptions_list (const char *regexp)
 static void
 info_exceptions_command (char *regexp, int from_tty)
 {
-  VEC(ada_exc_info) *exceptions;
-  struct cleanup *cleanup;
   struct gdbarch *gdbarch = get_current_arch ();
-  int ix;
-  struct ada_exc_info *info;
 
-  exceptions = ada_exceptions_list (regexp);
-  cleanup = make_cleanup (VEC_cleanup (ada_exc_info), &exceptions);
+  std::vector<ada_exc_info> exceptions = ada_exceptions_list (regexp);
 
   if (regexp != NULL)
     printf_filtered
@@ -13404,10 +13386,8 @@ info_exceptions_command (char *regexp, int from_tty)
   else
     printf_filtered (_("All defined Ada exceptions:\n"));
 
-  for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
-    printf_filtered ("%s: %s\n", info->name, paddress (gdbarch, info->addr));
-
-  do_cleanups (cleanup);
+  for (ada_exc_info &info : exceptions)
+    printf_filtered ("%s: %s\n", info.name, paddress (gdbarch, info.addr));
 }
 
                                 /* Operators */
diff --git a/gdb/ada-lang.h b/gdb/ada-lang.h
index f5b3bca..f92b88d 100644
--- a/gdb/ada-lang.h
+++ b/gdb/ada-lang.h
@@ -379,18 +379,19 @@ extern void create_ada_exception_catchpoint
 
 /* Some information about a given Ada exception.  */
 
-typedef struct ada_exc_info
+struct ada_exc_info
 {
   /* The name of the exception.  */
   const char *name;
 
   /* The address of the symbol corresponding to that exception.  */
   CORE_ADDR addr;
-} ada_exc_info;
 
-DEF_VEC_O(ada_exc_info);
+  bool operator< (const ada_exc_info &);
+  bool operator== (const ada_exc_info &);
+};
 
-extern VEC(ada_exc_info) *ada_exceptions_list (const char *regexp);
+extern std::vector<ada_exc_info> ada_exceptions_list (const char *regexp);
 
 /* Tasking-related: ada-tasks.c */
 
diff --git a/gdb/mi/mi-cmd-info.c b/gdb/mi/mi-cmd-info.c
index fa0d16e..33c64f0 100644
--- a/gdb/mi/mi-cmd-info.c
+++ b/gdb/mi/mi-cmd-info.c
@@ -30,10 +30,6 @@ mi_cmd_info_ada_exceptions (const char *command, char **argv, int argc)
   struct ui_out *uiout = current_uiout;
   struct gdbarch *gdbarch = get_current_arch ();
   char *regexp;
-  struct cleanup *old_chain;
-  VEC(ada_exc_info) *exceptions;
-  int ix;
-  struct ada_exc_info *info;
 
   switch (argc)
     {
@@ -48,24 +44,21 @@ mi_cmd_info_ada_exceptions (const char *command, char **argv, int argc)
       break;
     }
 
-  exceptions = ada_exceptions_list (regexp);
-  old_chain = make_cleanup (VEC_cleanup (ada_exc_info), &exceptions);
+  std::vector<ada_exc_info> exceptions = ada_exceptions_list (regexp);
 
   ui_out_emit_table table_emitter (uiout, 2,
-				   VEC_length (ada_exc_info, exceptions),
+				   exceptions.size (),
 				   "ada-exceptions");
   uiout->table_header (1, ui_left, "name", "Name");
   uiout->table_header (1, ui_left, "address", "Address");
   uiout->table_body ();
 
-  for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
+  for (ada_exc_info &info : exceptions)
     {
       ui_out_emit_tuple tuple_emitter (uiout, NULL);
-      uiout->field_string ("name", info->name);
-      uiout->field_core_addr ("address", gdbarch, info->addr);
+      uiout->field_string ("name", info.name);
+      uiout->field_core_addr ("address", gdbarch, info.addr);
     }
-
-  do_cleanups (old_chain);
 }
 
 /* Implement the "-info-gdb-mi-command" GDB/MI command.  */
-- 
2.9.4

Re: [RFA 10/11] Use a std::vector for ada_exceptions_list

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> Change ada_exceptions_list to return a std::vector and fix up the
> users.  This allows removing a cleanup in MI.

Looks good to me with the nits below addressed.

> diff --git a/gdb/ada-lang.c b/gdb/ada-lang.c
> index 64f1a33..cafba2d 100644
> --- a/gdb/ada-lang.c
> +++ b/gdb/ada-lang.c
> @@ -62,6 +62,7 @@
>  #include "cli/cli-utils.h"
>  #include "common/function-view.h"
>  #include "common/byte-vector.h"
> +#include <algorithm>
>  
>  /* Define whether or not the C operator '/' truncates towards zero for
>     differently signed operands (truncation direction is undefined in C).
> @@ -13121,23 +13122,23 @@ ada_is_non_standard_exception_sym (struct symbol *sym)
>     The comparison is determined first by exception name, and then
>     by exception address.  */

This comment talks about qsort.  It should be updated to mention
std::sort instead, since the logic is different.

>  
> -static int
> -compare_ada_exception_info (const void *a, const void *b)
> +bool
> +ada_exc_info::operator< (const ada_exc_info &other)
>  {
> -  const struct ada_exc_info *exc_a = (struct ada_exc_info *) a;
> -  const struct ada_exc_info *exc_b = (struct ada_exc_info *) b;
>    int result;
>  
> -  result = strcmp (exc_a->name, exc_b->name);
> -  if (result != 0)
> -    return result;
> -
> -  if (exc_a->addr < exc_b->addr)
> -    return -1;
> -  if (exc_a->addr > exc_b->addr)
> -    return 1;
> +  result = strcmp (name, other.name);
> +  if (result < 0)
> +    return true;
> +  if (result == 0 && addr < other.addr)
> +    return true;
> +  return false;
> +}
>  
> -  return 0;
> +bool
> +ada_exc_info::operator== (const ada_exc_info &other)
> +{
> +  return strcmp (name, other.name) == 0 && addr == other.addr;

I'd swap the comparisons to put the cheap addr comparison first.

>    if (regexp != NULL)
>      printf_filtered
> @@ -13404,10 +13386,8 @@ info_exceptions_command (char *regexp, int from_tty)
>    else
>      printf_filtered (_("All defined Ada exceptions:\n"));
>  
> -  for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> -    printf_filtered ("%s: %s\n", info->name, paddress (gdbarch, info->addr));
> -
> -  do_cleanups (cleanup);
> +  for (ada_exc_info &info : exceptions)
> +    printf_filtered ("%s: %s\n", info.name, paddress (gdbarch, info.addr));

I'd write 'const ada_exc_info &', just for general use-const-if-possible
reasons.

> -  for (ix = 0; VEC_iterate(ada_exc_info, exceptions, ix, info); ix++)
> +  for (ada_exc_info &info : exceptions)

Ditto.

Thanks,
Pedro Alves

[RFA 01/11] Remove make_cleanup_defer_target_commit_resume

[Tom Tromey]

This removes make_cleanup_defer_target_commit_resume in favor of using
scoped_restore.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* target.h (make_scoped_defer_target_commit_resume): Update.
	* target.c (make_scoped_defer_target_commit_resume): Rename from
	make_cleanup_defer_target_commit_resume.  Return a
	scoped_restore.
	* infrun.c (proceed): Use make_scoped_defer_target_commit_resume.
---
 gdb/ChangeLog |  8 ++++++++
 gdb/infrun.c  | 60 +++++++++++++++++++++++++++++------------------------------
 gdb/target.c  | 10 +++-------
 gdb/target.h  |  6 +++---
 4 files changed, 44 insertions(+), 40 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 3905a51..b71f23b 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,11 @@
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
+	* target.h (make_scoped_defer_target_commit_resume): Update.
+	* target.c (make_scoped_defer_target_commit_resume): Rename from
+	make_cleanup_defer_target_commit_resume.  Return a
+	scoped_restore.
+	* infrun.c (proceed): Use make_scoped_defer_target_commit_resume.
+
 2017-09-12  Simon Marchi  <simon.marchi@ericsson.com>
 
 	* probe.h (probe_ops_cp): Remove typedef.
diff --git a/gdb/infrun.c b/gdb/infrun.c
index 3f2ac85..e46ceab 100644
--- a/gdb/infrun.c
+++ b/gdb/infrun.c
@@ -2990,7 +2990,6 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
   struct execution_control_state ecss;
   struct execution_control_state *ecs = &ecss;
   struct cleanup *old_chain;
-  struct cleanup *defer_resume_cleanup;
   int started;
 
   /* If we're stopped at a fork/vfork, follow the branch set by the
@@ -3132,26 +3131,27 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
      until the target stops again.  */
   tp->prev_pc = regcache_read_pc (regcache);
 
-  defer_resume_cleanup = make_cleanup_defer_target_commit_resume ();
+  {
+    scoped_restore save_defer_tc = make_scoped_defer_target_commit_resume ();
 
-  started = start_step_over ();
+    started = start_step_over ();
 
-  if (step_over_info_valid_p ())
-    {
-      /* Either this thread started a new in-line step over, or some
-	 other thread was already doing one.  In either case, don't
-	 resume anything else until the step-over is finished.  */
-    }
-  else if (started && !target_is_non_stop_p ())
-    {
-      /* A new displaced stepping sequence was started.  In all-stop,
-	 we can't talk to the target anymore until it next stops.  */
-    }
-  else if (!non_stop && target_is_non_stop_p ())
-    {
-      /* In all-stop, but the target is always in non-stop mode.
-	 Start all other threads that are implicitly resumed too.  */
-      ALL_NON_EXITED_THREADS (tp)
+    if (step_over_info_valid_p ())
+      {
+	/* Either this thread started a new in-line step over, or some
+	   other thread was already doing one.  In either case, don't
+	   resume anything else until the step-over is finished.  */
+      }
+    else if (started && !target_is_non_stop_p ())
+      {
+	/* A new displaced stepping sequence was started.  In all-stop,
+	   we can't talk to the target anymore until it next stops.  */
+      }
+    else if (!non_stop && target_is_non_stop_p ())
+      {
+	/* In all-stop, but the target is always in non-stop mode.
+	   Start all other threads that are implicitly resumed too.  */
+	ALL_NON_EXITED_THREADS (tp)
         {
 	  /* Ignore threads of processes we're not resuming.  */
 	  if (!ptid_match (tp->ptid, resume_ptid))
@@ -3187,18 +3187,18 @@ proceed (CORE_ADDR addr, enum gdb_signal siggnal)
 	  if (!ecs->wait_some_more)
 	    error (_("Command aborted."));
 	}
-    }
-  else if (!tp->resumed && !thread_is_in_step_over_chain (tp))
-    {
-      /* The thread wasn't started, and isn't queued, run it now.  */
-      reset_ecs (ecs, tp);
-      switch_to_thread (tp->ptid);
-      keep_going_pass_signal (ecs);
-      if (!ecs->wait_some_more)
-	error (_("Command aborted."));
-    }
+      }
+    else if (!tp->resumed && !thread_is_in_step_over_chain (tp))
+      {
+	/* The thread wasn't started, and isn't queued, run it now.  */
+	reset_ecs (ecs, tp);
+	switch_to_thread (tp->ptid);
+	keep_going_pass_signal (ecs);
+	if (!ecs->wait_some_more)
+	  error (_("Command aborted."));
+      }
+  }
 
-  do_cleanups (defer_resume_cleanup);
   target_commit_resume ();
 
   discard_cleanups (old_chain);
diff --git a/gdb/target.c b/gdb/target.c
index 3e2b4d0..bcd0008 100644
--- a/gdb/target.c
+++ b/gdb/target.c
@@ -2330,14 +2330,10 @@ target_commit_resume (void)
 
 /* See target.h.  */
 
-struct cleanup *
-make_cleanup_defer_target_commit_resume (void)
+scoped_restore_tmpl<int>
+make_scoped_defer_target_commit_resume ()
 {
-  struct cleanup *old_chain;
-
-  old_chain = make_cleanup_restore_integer (&defer_target_commit_resume);
-  defer_target_commit_resume = 1;
-  return old_chain;
+  return make_scoped_restore (&defer_target_commit_resume, 1);
 }
 
 void
diff --git a/gdb/target.h b/gdb/target.h
index a3f00ab..5160924 100644
--- a/gdb/target.h
+++ b/gdb/target.h
@@ -1371,10 +1371,10 @@ extern void target_resume (ptid_t ptid, int step, enum gdb_signal signal);
    coalesce multiple resumption requests in a single vCont packet.  */
 extern void target_commit_resume ();
 
-/* Setup to defer target_commit_resume calls, and return a cleanup
-   that reactivates target_commit_resume, if it was previously
+/* Setup to defer target_commit_resume calls, and reactivate
+   target_commit_resume on destruction, if it was previously
    active.  */
-struct cleanup *make_cleanup_defer_target_commit_resume ();
+extern scoped_restore_tmpl<int> make_scoped_defer_target_commit_resume ();
 
 /* For target_read_memory see target/target.h.  */
 
-- 
2.9.4

Re: [RFA 01/11] Remove make_cleanup_defer_target_commit_resume

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:
> This removes make_cleanup_defer_target_commit_resume in favor of using
> scoped_restore.
> 
> ChangeLog
> 2017-09-12  Tom Tromey  <tom@tromey.com>
> 
> 	* target.h (make_scoped_defer_target_commit_resume): Update.
> 	* target.c (make_scoped_defer_target_commit_resume): Rename from
> 	make_cleanup_defer_target_commit_resume.  Return a
> 	scoped_restore.
> 	* infrun.c (proceed): Use make_scoped_defer_target_commit_resume.

OK.

Thanks,
Pedro Alves

[RFA 03/11] Remove cleanups from mi-cmd-var.c

[Tom Tromey]

This removes some cleanups from mi-cmd-var.c.  varobj_gen_name now
returns a string, simplifying mi_cmd_var_create.  In
mi_cmd_var_delete, a string copy is apparently unnecessary, so it's
simply removed.

ChangeLog
2017-09-12  Tom Tromey  <tom@tromey.com>

	* varobj.h (varobj_gen_name): Return std::string.
	* varobj.c (varobj_gen_name): Return std::string.
	* mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
	(mi_cmd_var_delete): Don't copy "name".
---
 gdb/ChangeLog       |  7 +++++++
 gdb/mi/mi-cmd-var.c | 43 +++++++++++--------------------------------
 gdb/varobj.c        |  7 ++-----
 gdb/varobj.h        |  2 +-
 4 files changed, 21 insertions(+), 38 deletions(-)

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 15a19bb8..b1fdcda 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,5 +1,12 @@
 2017-09-12  Tom Tromey  <tom@tromey.com>
 
+	* varobj.h (varobj_gen_name): Return std::string.
+	* varobj.c (varobj_gen_name): Return std::string.
+	* mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
+	(mi_cmd_var_delete): Don't copy "name".
+
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
 	* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
 	(mi_cmd_break_insert_1): Update.
 
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 8b22b2f..b628814 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -95,32 +95,21 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
   struct ui_out *uiout = current_uiout;
   CORE_ADDR frameaddr = 0;
   struct varobj *var;
-  char *name;
   char *frame;
   char *expr;
-  struct cleanup *old_cleanups;
   enum varobj_type var_type;
 
   if (argc != 3)
     error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
 
-  name = xstrdup (argv[0]);
-  /* Add cleanup for name. Must be free_current_contents as name can
-     be reallocated.  */
-  old_cleanups = make_cleanup (free_current_contents, &name);
-
-  frame = xstrdup (argv[1]);
-  make_cleanup (xfree, frame);
+  std::string name = argv[0];
 
-  expr = xstrdup (argv[2]);
-  make_cleanup (xfree, expr);
+  frame = argv[1];
+  expr = argv[2];
 
-  if (strcmp (name, "-") == 0)
-    {
-      xfree (name);
-      name = varobj_gen_name ();
-    }
-  else if (!isalpha (*name))
+  if (name == "-")
+    name = varobj_gen_name ();
+  else if (!isalpha (name[0]))
     error (_("-var-create: name of object must begin with a letter"));
 
   if (strcmp (frame, "*") == 0)
@@ -135,10 +124,10 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
 
   if (varobjdebug)
     fprintf_unfiltered (gdb_stdlog,
-		    "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
-			name, frame, hex_string (frameaddr), expr);
+			"Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
+			name.c_str (), frame, hex_string (frameaddr), expr);
 
-  var = varobj_create (name, expr, frameaddr, var_type);
+  var = varobj_create (name.c_str (), expr, frameaddr, var_type);
 
   if (var == NULL)
     error (_("-var-create: unable to create variable object"));
@@ -146,8 +135,6 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
   print_varobj (var, PRINT_ALL_VALUES, 0 /* don't print expression */);
 
   uiout->field_int ("has_more", varobj_has_more (var, 0));
-
-  do_cleanups (old_cleanups);
 }
 
 void
@@ -157,16 +144,12 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
   struct varobj *var;
   int numdel;
   int children_only_p = 0;
-  struct cleanup *old_cleanups;
   struct ui_out *uiout = current_uiout;
 
   if (argc < 1 || argc > 2)
     error (_("-var-delete: Usage: [-c] EXPRESSION."));
 
-  name = xstrdup (argv[0]);
-  /* Add cleanup for name. Must be free_current_contents as name can
-     be reallocated.  */
-  old_cleanups = make_cleanup (free_current_contents, &name);
+  name = argv[0];
 
   /* If we have one single argument it cannot be '-c' or any string
      starting with '-'.  */
@@ -186,9 +169,7 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
       if (strcmp (name, "-c") != 0)
 	error (_("-var-delete: Invalid option."));
       children_only_p = 1;
-      do_cleanups (old_cleanups);
-      name = xstrdup (argv[1]);
-      old_cleanups = make_cleanup (free_current_contents, &name);
+      name = argv[1];
     }
 
   /* If we didn't error out, now NAME contains the name of the
@@ -199,8 +180,6 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
   numdel = varobj_delete (var, children_only_p);
 
   uiout->field_int ("ndeleted", numdel);
-
-  do_cleanups (old_cleanups);
 }
 
 /* Parse a string argument into a format value.  */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f669180..2d850fb 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -435,17 +435,14 @@ varobj_create (const char *objname,
 
 /* Generates an unique name that can be used for a varobj.  */
 
-char *
+std::string
 varobj_gen_name (void)
 {
   static int id = 0;
-  char *obj_name;
 
   /* Generate a name for this object.  */
   id++;
-  obj_name = xstrprintf ("var%d", id);
-
-  return obj_name;
+  return string_printf ("var%d", id);
 }
 
 /* Given an OBJNAME, returns the pointer to the corresponding varobj.  Call
diff --git a/gdb/varobj.h b/gdb/varobj.h
index e35c1b8..0d4a537 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -233,7 +233,7 @@ extern struct varobj *varobj_create (const char *objname,
 				     const char *expression, CORE_ADDR frame,
 				     enum varobj_type type);
 
-extern char *varobj_gen_name (void);
+extern std::string varobj_gen_name (void);
 
 extern struct varobj *varobj_get_handle (const char *name);
 
-- 
2.9.4

Re: [RFA 03/11] Remove cleanups from mi-cmd-var.c

[Pedro Alves]

On 09/12/2017 07:57 PM, Tom Tromey wrote:

> diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
> index 8b22b2f..b628814 100644
> --- a/gdb/mi/mi-cmd-var.c
> +++ b/gdb/mi/mi-cmd-var.c
> @@ -95,32 +95,21 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
>    struct ui_out *uiout = current_uiout;
>    CORE_ADDR frameaddr = 0;
>    struct varobj *var;
> -  char *name;
>    char *frame;
>    char *expr;
> -  struct cleanup *old_cleanups;
>    enum varobj_type var_type;
>  
>    if (argc != 3)
>      error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
>  
> -  name = xstrdup (argv[0]);
> -  /* Add cleanup for name. Must be free_current_contents as name can
> -     be reallocated.  */
> -  old_cleanups = make_cleanup (free_current_contents, &name);
> -
> -  frame = xstrdup (argv[1]);
> -  make_cleanup (xfree, frame);
> +  std::string name = argv[0];
>  
> -  expr = xstrdup (argv[2]);
> -  make_cleanup (xfree, expr);
> +  frame = argv[1];
> +  expr = argv[2];
>  
> -  if (strcmp (name, "-") == 0)
> -    {
> -      xfree (name);
> -      name = varobj_gen_name ();
> -    }
> -  else if (!isalpha (*name))
> +  if (name == "-")
> +    name = varobj_gen_name ();
> +  else if (!isalpha (name[0]))
>      error (_("-var-create: name of object must begin with a letter"));

name was a deep copy of argv[0] up to here.  That copy
appears unnecessary, if you write it like this:

  std::string name;

  if (strcmp (argv[0], "-") == 0)
    name = varobj_gen_name ();
  else if (!isalpha (*argv[0]))
    error (_("-var-create: name of object must begin with a letter"));
  else
    name = argv[0];

There's still a deep copy in that last else above.
We should be able to get rid of that like this:

  const char *name;
  std::string gen_name;

  if (strcmp (argv[0], "-") == 0)
    {
      gen_name = varobj_gen_name ();
      name = gen_name.c_str ();
    }
  else if (!isalpha (*argv[0]))
    error (_("-var-create: name of object must begin with a letter"));
  else
    name = argv[0];

Otherwise OK.

Thanks,
Pedro Alves

Re: [RFA 03/11] Remove cleanups from mi-cmd-var.c

[Tom Tromey]

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

Pedro> There's still a deep copy in that last else above.
Pedro> We should be able to get rid of that like this:
[...]

Here's my latest.

Tom

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 1a58d82..5a6791e 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,10 @@
+2017-09-12  Tom Tromey  <tom@tromey.com>
+
+	* varobj.h (varobj_gen_name): Return std::string.
+	* varobj.c (varobj_gen_name): Return std::string.
+	* mi/mi-cmd-var.c (mi_cmd_var_create): Use std::string.
+	(mi_cmd_var_delete): Don't copy "name".
+
 2017-09-28  Tom Tromey  <tom@tromey.com>
 
 	* mi/mi-cmd-break.c (mi_argv_to_format): Return std::string.
diff --git a/gdb/mi/mi-cmd-var.c b/gdb/mi/mi-cmd-var.c
index 8b22b2f..0215b1a 100644
--- a/gdb/mi/mi-cmd-var.c
+++ b/gdb/mi/mi-cmd-var.c
@@ -95,32 +95,24 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
   struct ui_out *uiout = current_uiout;
   CORE_ADDR frameaddr = 0;
   struct varobj *var;
-  char *name;
   char *frame;
   char *expr;
-  struct cleanup *old_cleanups;
   enum varobj_type var_type;
 
   if (argc != 3)
     error (_("-var-create: Usage: NAME FRAME EXPRESSION."));
 
-  name = xstrdup (argv[0]);
-  /* Add cleanup for name. Must be free_current_contents as name can
-     be reallocated.  */
-  old_cleanups = make_cleanup (free_current_contents, &name);
-
-  frame = xstrdup (argv[1]);
-  make_cleanup (xfree, frame);
-
-  expr = xstrdup (argv[2]);
-  make_cleanup (xfree, expr);
+  frame = argv[1];
+  expr = argv[2];
 
+  const char *name = argv[0];
+  std::string gen_name;
   if (strcmp (name, "-") == 0)
     {
-      xfree (name);
-      name = varobj_gen_name ();
+      gen_name = varobj_gen_name ();
+      name = gen_name.c_str ();
     }
-  else if (!isalpha (*name))
+  else if (!isalpha (name[0]))
     error (_("-var-create: name of object must begin with a letter"));
 
   if (strcmp (frame, "*") == 0)
@@ -135,7 +127,7 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
 
   if (varobjdebug)
     fprintf_unfiltered (gdb_stdlog,
-		    "Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
+			"Name=\"%s\", Frame=\"%s\" (%s), Expression=\"%s\"\n",
 			name, frame, hex_string (frameaddr), expr);
 
   var = varobj_create (name, expr, frameaddr, var_type);
@@ -146,8 +138,6 @@ mi_cmd_var_create (const char *command, char **argv, int argc)
   print_varobj (var, PRINT_ALL_VALUES, 0 /* don't print expression */);
 
   uiout->field_int ("has_more", varobj_has_more (var, 0));
-
-  do_cleanups (old_cleanups);
 }
 
 void
@@ -157,16 +147,12 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
   struct varobj *var;
   int numdel;
   int children_only_p = 0;
-  struct cleanup *old_cleanups;
   struct ui_out *uiout = current_uiout;
 
   if (argc < 1 || argc > 2)
     error (_("-var-delete: Usage: [-c] EXPRESSION."));
 
-  name = xstrdup (argv[0]);
-  /* Add cleanup for name. Must be free_current_contents as name can
-     be reallocated.  */
-  old_cleanups = make_cleanup (free_current_contents, &name);
+  name = argv[0];
 
   /* If we have one single argument it cannot be '-c' or any string
      starting with '-'.  */
@@ -186,9 +172,7 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
       if (strcmp (name, "-c") != 0)
 	error (_("-var-delete: Invalid option."));
       children_only_p = 1;
-      do_cleanups (old_cleanups);
-      name = xstrdup (argv[1]);
-      old_cleanups = make_cleanup (free_current_contents, &name);
+      name = argv[1];
     }
 
   /* If we didn't error out, now NAME contains the name of the
@@ -199,8 +183,6 @@ mi_cmd_var_delete (const char *command, char **argv, int argc)
   numdel = varobj_delete (var, children_only_p);
 
   uiout->field_int ("ndeleted", numdel);
-
-  do_cleanups (old_cleanups);
 }
 
 /* Parse a string argument into a format value.  */
diff --git a/gdb/varobj.c b/gdb/varobj.c
index f669180..2d850fb 100644
--- a/gdb/varobj.c
+++ b/gdb/varobj.c
@@ -435,17 +435,14 @@ varobj_create (const char *objname,
 
 /* Generates an unique name that can be used for a varobj.  */
 
-char *
+std::string
 varobj_gen_name (void)
 {
   static int id = 0;
-  char *obj_name;
 
   /* Generate a name for this object.  */
   id++;
-  obj_name = xstrprintf ("var%d", id);
-
-  return obj_name;
+  return string_printf ("var%d", id);
 }
 
 /* Given an OBJNAME, returns the pointer to the corresponding varobj.  Call
diff --git a/gdb/varobj.h b/gdb/varobj.h
index e35c1b8..0d4a537 100644
--- a/gdb/varobj.h
+++ b/gdb/varobj.h
@@ -233,7 +233,7 @@ extern struct varobj *varobj_create (const char *objname,
 				     const char *expression, CORE_ADDR frame,
 				     enum varobj_type type);
 
-extern char *varobj_gen_name (void);
+extern std::string varobj_gen_name (void);
 
 extern struct varobj *varobj_get_handle (const char *name);
 

Re: [RFA 03/11] Remove cleanups from mi-cmd-var.c

[Pedro Alves]

>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> There's still a deep copy in that last else above.
> Pedro> We should be able to get rid of that like this:
> [...]
> 
> Here's my latest.

OK.

Thanks,
Pedro Alves

Re: more cleanup removal, particularly in MI

[Tom Tromey]

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

Tom> This series removes more cleanups.
Tom> The first patch is generic, but the remaining patches focus on
Tom> removing cleanups from MI.  It doesn't completely remove cleanups from
Tom> MI, but does make significant progress toward that goal.

Tom> Regression tested on the buildbot.

Ping.

Tom