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

Message ID 20170912185736.20436-9-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey Sept. 12, 2017, 6:57 p.m. UTC
  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(-)
  

Comments

Pedro Alves Sept. 28, 2017, 9:57 a.m. UTC | #1
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
  

Patch

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.  */