diff mbox

[RFA] Remove cleanups from tracefile.c

Message ID 20180607232706.13875-1-tom@tromey.com
State New, archived
Headers

Commit Message

Tom Tromey June 7, 2018, 11:27 p.m. UTC 
  This removes cleanups from tracefile.c, by introducing a unique_ptr
specialization.

This code could be made even simpler via a deeper C++-ification, but I
have not attempted that.

Tested by the buildbot.

gdb/ChangeLog
2018-06-07  Tom Tromey  <tom@tromey.com>

	* tracefile.c (struct trace_file_writer_deleter): New.
	<operator()>: Rename from trace_file_writer_xfree.
	(trace_file_writer_up): New typedef.
	(tsave_command, trace_save_tfile, trace_save_ctf): Update.
---
 gdb/ChangeLog   |  7 +++++++
 gdb/tracefile.c | 51 ++++++++++++++++++++-------------------------------
 2 files changed, 27 insertions(+), 31 deletions(-)

Comments

Simon Marchi June 9, 2018, 12:27 p.m. UTC | #1 
On 2018-06-07 19:27, Tom Tromey wrote:
> @@ -336,20 +339,17 @@ tsave_command (const char *args, int from_tty)
>    if (!filename)
>      error_no_arg (_("file in which to save trace data"));
> 
> +  trace_file_writer_up writer;
>    if (generate_ctf)
> -    writer = ctf_trace_file_writer_new ();
> +    writer.reset (ctf_trace_file_writer_new ());
>    else
> -    writer = tfile_trace_file_writer_new ();
> -
> -  back_to = make_cleanup (trace_file_writer_xfree, writer);
> +    writer.reset (tfile_trace_file_writer_new ());
> 
> -  trace_save (filename, writer, target_does_save);
> +  trace_save (filename, writer.get (), target_does_save);

Could this call trace_save_tfile and trace_save_ctf instead to 
duplication?

   if (generate_ctf)
     trace_save_ctf (filename, target_does_save);
   else
     trace_save_tfile (filename, target_does_save);

Either way, LGTM.

Simon
Tom Tromey June 9, 2018, 10:17 p.m. UTC | #2 
>>>>> "Simon" == Simon Marchi <simon.marchi@polymtl.ca> writes:

Simon> Could this call trace_save_tfile and trace_save_ctf instead to
Simon> duplication?
Simon>   if (generate_ctf)
Simon>     trace_save_ctf (filename, target_does_save);
Simon>   else
Simon>     trace_save_tfile (filename, target_does_save);

I made this change and I'll push it after re-testing.

Tom
diff mbox

Patch

diff --git a/gdb/tracefile.c b/gdb/tracefile.c
index ab34ecfdf99..18ce53bf12d 100644
--- a/gdb/tracefile.c
+++ b/gdb/tracefile.c
@@ -36,16 +36,21 @@ 
 #define TRACE_WRITE_V_BLOCK(writer, num, val)	\
   writer->ops->frame_ops->write_v_block ((writer), (num), (val))
 
-/* Free trace file writer.  */
+/* A unique pointer policy class for trace_file_writer.  */
 
-static void
-trace_file_writer_xfree (void *arg)
+struct trace_file_writer_deleter
 {
-  struct trace_file_writer *writer = (struct trace_file_writer *) arg;
+  void operator() (struct trace_file_writer *writer)
+  {
+    writer->ops->dtor (writer);
+    xfree (writer);
+  }
+};
 
-  writer->ops->dtor (writer);
-  xfree (writer);
-}
+/* A unique_ptr specialization for trace_file_writer.  */
+
+typedef std::unique_ptr<trace_file_writer, trace_file_writer_deleter>
+    trace_file_writer_up;
 
 /* Save tracepoint data to file named FILENAME through WRITER.  WRITER
    determines the trace file format.  If TARGET_DOES_SAVE is non-zero,
@@ -311,9 +316,7 @@  tsave_command (const char *args, int from_tty)
   int target_does_save = 0;
   char **argv;
   char *filename = NULL;
-  struct cleanup *back_to;
   int generate_ctf = 0;
-  struct trace_file_writer *writer = NULL;
 
   if (args == NULL)
     error_no_arg (_("file in which to save trace data"));
@@ -336,20 +339,17 @@  tsave_command (const char *args, int from_tty)
   if (!filename)
     error_no_arg (_("file in which to save trace data"));
 
+  trace_file_writer_up writer;
   if (generate_ctf)
-    writer = ctf_trace_file_writer_new ();
+    writer.reset (ctf_trace_file_writer_new ());
   else
-    writer = tfile_trace_file_writer_new ();
-
-  back_to = make_cleanup (trace_file_writer_xfree, writer);
+    writer.reset (tfile_trace_file_writer_new ());
 
-  trace_save (filename, writer, target_does_save);
+  trace_save (filename, writer.get (), target_does_save);
 
   if (from_tty)
     printf_filtered (_("Trace data saved to %s '%s'.\n"),
 		     generate_ctf ? "directory" : "file", filename);
-
-  do_cleanups (back_to);
 }
 
 /* Save the trace data to file FILENAME of tfile format.  */
@@ -357,13 +357,8 @@  tsave_command (const char *args, int from_tty)
 void
 trace_save_tfile (const char *filename, int target_does_save)
 {
-  struct trace_file_writer *writer;
-  struct cleanup *back_to;
-
-  writer = tfile_trace_file_writer_new ();
-  back_to = make_cleanup (trace_file_writer_xfree, writer);
-  trace_save (filename, writer, target_does_save);
-  do_cleanups (back_to);
+  trace_file_writer_up writer (tfile_trace_file_writer_new ());
+  trace_save (filename, writer.get (), target_does_save);
 }
 
 /* Save the trace data to dir DIRNAME of ctf format.  */
@@ -371,14 +366,8 @@  trace_save_tfile (const char *filename, int target_does_save)
 void
 trace_save_ctf (const char *dirname, int target_does_save)
 {
-  struct trace_file_writer *writer;
-  struct cleanup *back_to;
-
-  writer = ctf_trace_file_writer_new ();
-  back_to = make_cleanup (trace_file_writer_xfree, writer);
-
-  trace_save (dirname, writer, target_does_save);
-  do_cleanups (back_to);
+  trace_file_writer_up writer (ctf_trace_file_writer_new ());
+  trace_save (dirname, writer.get (), target_does_save);
 }
 
 /* Fetch register data from tracefile, shared for both tfile and