Patchwork [RFA,1/2] Remove cleanups from btrace code

login
register
mail settings
Submitter Tom Tromey
Date June 7, 2018, 10:44 p.m.
Message ID <20180607224426.6403-2-tom@tromey.com>
Download mbox | patch
Permalink /patch/27700/
State New
Headers show

Comments

Tom Tromey - June 7, 2018, 10:44 p.m.
This removes some cleanups from the btrace code by minorly C++-ifying
struct btrace_data.

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

	* common/btrace-common.h (struct btrace_data): Add constructor,
	destructor, move assignment operator.
	<empty, clear, fini>: New methods.
	<format>: Initialize.
	(btrace_data_init, btrace_data_fini, btrace_data_clear)
	(btrace_data_empty): Don't declare.
	* common/btrace-common.c (btrace_data_init): Remove.
	(btrace_data::fini): Rename from btrace_data_fini.
	(btrace_data::empty): Rename from btrace_data_empty.
	(btrace_data::clear): Rename from btrace_data_clear.  Return
	bool.
	* btrace.h (make_cleanup_btrace_data): Don't declare.
	* btrace.c (btrace_add_pc, btrace_stitch_trace, btrace_clear)
	(parse_xml_btrace): Update.
	(do_btrace_data_cleanup, make_cleanup_btrace_data): Remove.
	(maint_btrace_clear_packet_history_cmd): Update.

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

	* linux-low.c (linux_low_read_btrace): Update.
---
 gdb/ChangeLog              | 19 +++++++++++++++++++
 gdb/btrace.c               | 47 +++++++++-------------------------------------
 gdb/btrace.h               |  3 ---
 gdb/common/btrace-common.c | 34 +++++++++++++--------------------
 gdb/common/btrace-common.h | 45 +++++++++++++++++++++++++++++++-------------
 gdb/gdbserver/ChangeLog    |  4 ++++
 gdb/gdbserver/linux-low.c  | 13 +++----------
 7 files changed, 80 insertions(+), 85 deletions(-)

Patch

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 269ee51254e..690770572e8 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1565,25 +1565,19 @@  btrace_add_pc (struct thread_info *tp)
   struct btrace_data btrace;
   struct btrace_block *block;
   struct regcache *regcache;
-  struct cleanup *cleanup;
   CORE_ADDR pc;
 
   regcache = get_thread_regcache (tp->ptid);
   pc = regcache_read_pc (regcache);
 
-  btrace_data_init (&btrace);
   btrace.format = BTRACE_FORMAT_BTS;
   btrace.variant.bts.blocks = NULL;
 
-  cleanup = make_cleanup_btrace_data (&btrace);
-
   block = VEC_safe_push (btrace_block_s, btrace.variant.bts.blocks, NULL);
   block->begin = pc;
   block->end = pc;
 
   btrace_compute_ftrace (tp, &btrace, NULL);
-
-  do_cleanups (cleanup);
 }
 
 /* See btrace.h.  */
@@ -1777,7 +1771,7 @@  static int
 btrace_stitch_trace (struct btrace_data *btrace, struct thread_info *tp)
 {
   /* If we don't have trace, there's nothing to do.  */
-  if (btrace_data_empty (btrace))
+  if (btrace->empty ())
     return 0;
 
   switch (btrace->format)
@@ -1894,7 +1888,6 @@  btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
   struct btrace_thread_info *btinfo;
   struct btrace_target_info *tinfo;
   struct btrace_data btrace;
-  struct cleanup *cleanup;
   int errcode;
 
   DEBUG ("fetch thread %s (%s)", print_thread_id (tp),
@@ -1920,9 +1913,6 @@  btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
   /* We should not be called on running or exited threads.  */
   gdb_assert (can_access_registers_ptid (tp->ptid));
 
-  btrace_data_init (&btrace);
-  cleanup = make_cleanup_btrace_data (&btrace);
-
   /* Let's first try to extend the trace we already have.  */
   if (!btinfo->functions.empty ())
     {
@@ -1938,7 +1928,7 @@  btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
 	  errcode = target_read_btrace (&btrace, tinfo, BTRACE_READ_NEW);
 
 	  /* If we got any new trace, discard what we have.  */
-	  if (errcode == 0 && !btrace_data_empty (&btrace))
+	  if (errcode == 0 && !btrace.empty ())
 	    btrace_clear (tp);
 	}
 
@@ -1957,7 +1947,7 @@  btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
     error (_("Failed to read branch trace."));
 
   /* Compute the trace, provided we have any.  */
-  if (!btrace_data_empty (&btrace))
+  if (!btrace.empty ())
     {
       /* Store the raw trace data.  The stored data will be cleared in
 	 btrace_clear, so we always append the new trace.  */
@@ -1967,8 +1957,6 @@  btrace_fetch (struct thread_info *tp, const struct btrace_cpu *cpu)
       btrace_clear_history (btinfo);
       btrace_compute_ftrace (tp, &btrace, cpu);
     }
-
-  do_cleanups (cleanup);
 }
 
 /* See btrace.h.  */
@@ -1992,7 +1980,7 @@  btrace_clear (struct thread_info *tp)
 
   /* Must clear the maint data before - it depends on BTINFO->DATA.  */
   btrace_maint_clear (btinfo);
-  btrace_data_clear (&btinfo->data);
+  btinfo->data.clear ();
   btrace_clear_history (btinfo);
 }
 
@@ -2217,21 +2205,20 @@  static const struct gdb_xml_element btrace_elements[] = {
 void
 parse_xml_btrace (struct btrace_data *btrace, const char *buffer)
 {
-  struct cleanup *cleanup;
   int errcode;
 
 #if defined (HAVE_LIBEXPAT)
 
-  btrace->format = BTRACE_FORMAT_NONE;
+  btrace_data result;
+  result.format = BTRACE_FORMAT_NONE;
 
-  cleanup = make_cleanup_btrace_data (btrace);
   errcode = gdb_xml_parse_quick (_("btrace"), "btrace.dtd", btrace_elements,
-				 buffer, btrace);
+				 buffer, &result);
   if (errcode != 0)
     error (_("Error parsing branch trace."));
 
   /* Keep parse results.  */
-  discard_cleanups (cleanup);
+  *btrace = std::move (result);
 
 #else  /* !defined (HAVE_LIBEXPAT) */
 
@@ -2844,22 +2831,6 @@  btrace_is_empty (struct thread_info *tp)
   return btrace_insn_cmp (&begin, &end) == 0;
 }
 
-/* Forward the cleanup request.  */
-
-static void
-do_btrace_data_cleanup (void *arg)
-{
-  btrace_data_fini ((struct btrace_data *) arg);
-}
-
-/* See btrace.h.  */
-
-struct cleanup *
-make_cleanup_btrace_data (struct btrace_data *data)
-{
-  return make_cleanup (do_btrace_data_cleanup, data);
-}
-
 #if defined (HAVE_LIBIPT)
 
 /* Print a single packet.  */
@@ -3381,7 +3352,7 @@  maint_btrace_clear_packet_history_cmd (const char *args, int from_tty)
 
   /* Must clear the maint data before - it depends on BTINFO->DATA.  */
   btrace_maint_clear (btinfo);
-  btrace_data_clear (&btinfo->data);
+  btinfo->data.clear ();
 }
 
 /* The "maintenance btrace clear" command.  */
diff --git a/gdb/btrace.h b/gdb/btrace.h
index 4d57edb41f7..bfb0b9f2787 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -507,7 +507,4 @@  extern int btrace_is_replaying (struct thread_info *tp);
 /* Return non-zero if the branch trace for TP is empty; zero otherwise.  */
 extern int btrace_is_empty (struct thread_info *tp);
 
-/* Create a cleanup for DATA.  */
-extern struct cleanup *make_cleanup_btrace_data (struct btrace_data *data);
-
 #endif /* BTRACE_H */
diff --git a/gdb/common/btrace-common.c b/gdb/common/btrace-common.c
index 6cbbdc192ea..ec0ba6af136 100644
--- a/gdb/common/btrace-common.c
+++ b/gdb/common/btrace-common.c
@@ -64,28 +64,20 @@  btrace_format_short_string (enum btrace_format format)
 /* See btrace-common.h.  */
 
 void
-btrace_data_init (struct btrace_data *data)
+btrace_data::fini ()
 {
-  data->format = BTRACE_FORMAT_NONE;
-}
-
-/* See btrace-common.h.  */
-
-void
-btrace_data_fini (struct btrace_data *data)
-{
-  switch (data->format)
+  switch (format)
     {
     case BTRACE_FORMAT_NONE:
       /* Nothing to do.  */
       return;
 
     case BTRACE_FORMAT_BTS:
-      VEC_free (btrace_block_s, data->variant.bts.blocks);
+      VEC_free (btrace_block_s, variant.bts.blocks);
       return;
 
     case BTRACE_FORMAT_PT:
-      xfree (data->variant.pt.data);
+      xfree (variant.pt.data);
       return;
     }
 
@@ -94,19 +86,19 @@  btrace_data_fini (struct btrace_data *data)
 
 /* See btrace-common.h.  */
 
-int
-btrace_data_empty (struct btrace_data *data)
+bool
+btrace_data::empty () const
 {
-  switch (data->format)
+  switch (format)
     {
     case BTRACE_FORMAT_NONE:
-      return 1;
+      return true;
 
     case BTRACE_FORMAT_BTS:
-      return VEC_empty (btrace_block_s, data->variant.bts.blocks);
+      return VEC_empty (btrace_block_s, variant.bts.blocks);
 
     case BTRACE_FORMAT_PT:
-      return (data->variant.pt.size == 0);
+      return (variant.pt.size == 0);
     }
 
   internal_error (__FILE__, __LINE__, _("Unkown branch trace format."));
@@ -115,10 +107,10 @@  btrace_data_empty (struct btrace_data *data)
 /* See btrace-common.h.  */
 
 void
-btrace_data_clear (struct btrace_data *data)
+btrace_data::clear ()
 {
-  btrace_data_fini (data);
-  btrace_data_init (data);
+  fini ();
+  format = BTRACE_FORMAT_NONE;
 }
 
 /* See btrace-common.h.  */
diff --git a/gdb/common/btrace-common.h b/gdb/common/btrace-common.h
index 80e9fc65ba9..debf833f05d 100644
--- a/gdb/common/btrace-common.h
+++ b/gdb/common/btrace-common.h
@@ -164,7 +164,32 @@  struct btrace_data_pt
 /* The branch trace data.  */
 struct btrace_data
 {
-  enum btrace_format format;
+  btrace_data () = default;
+
+  ~btrace_data ()
+  {
+    fini ();
+  }
+
+  btrace_data &operator= (btrace_data &&other)
+  {
+    if (this != &other)
+      {
+	fini ();
+	format = other.format;
+	variant = other.variant;
+	other.format = BTRACE_FORMAT_NONE;
+      }
+    return *this;
+  }
+
+  /* Return true if this is empty; false otherwise.  */
+  bool empty () const;
+
+  /* Clear this object.  */
+  void clear ();
+
+  enum btrace_format format = BTRACE_FORMAT_NONE;
 
   union
   {
@@ -174,6 +199,12 @@  struct btrace_data
     /* Format == BTRACE_FORMAT_PT.  */
     struct btrace_data_pt pt;
   } variant;
+
+private:
+
+  DISABLE_COPY_AND_ASSIGN (btrace_data);
+
+  void fini ();
 };
 
 /* Target specific branch trace information.  */
@@ -217,18 +248,6 @@  extern const char *btrace_format_string (enum btrace_format format);
 /* Return an abbreviation string representation of FORMAT.  */
 extern const char *btrace_format_short_string (enum btrace_format format);
 
-/* Initialize DATA.  */
-extern void btrace_data_init (struct btrace_data *data);
-
-/* Cleanup DATA.  */
-extern void btrace_data_fini (struct btrace_data *data);
-
-/* Clear DATA.  */
-extern void btrace_data_clear (struct btrace_data *data);
-
-/* Return non-zero if DATA is empty; zero otherwise.  */
-extern int btrace_data_empty (struct btrace_data *data);
-
 /* Append the branch trace data from SRC to the end of DST.
    Both SRC and DST must use the same format.
    Returns zero on success; a negative number otherwise.  */
diff --git a/gdb/gdbserver/linux-low.c b/gdb/gdbserver/linux-low.c
index f8507b79684..1211944d79a 100644
--- a/gdb/gdbserver/linux-low.c
+++ b/gdb/gdbserver/linux-low.c
@@ -7250,8 +7250,6 @@  linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
   enum btrace_error err;
   int i;
 
-  btrace_data_init (&btrace);
-
   err = linux_read_btrace (&btrace, tinfo, type);
   if (err != BTRACE_ERR_NONE)
     {
@@ -7260,14 +7258,14 @@  linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
       else
 	buffer_grow_str0 (buffer, "E.Generic Error.");
 
-      goto err;
+      return -1;
     }
 
   switch (btrace.format)
     {
     case BTRACE_FORMAT_NONE:
       buffer_grow_str0 (buffer, "E.No Trace.");
-      goto err;
+      return -1;
 
     case BTRACE_FORMAT_BTS:
       buffer_grow_str (buffer, "<!DOCTYPE btrace SYSTEM \"btrace.dtd\">\n");
@@ -7298,15 +7296,10 @@  linux_low_read_btrace (struct btrace_target_info *tinfo, struct buffer *buffer,
 
     default:
       buffer_grow_str0 (buffer, "E.Unsupported Trace Format.");
-      goto err;
+      return -1;
     }
 
-  btrace_data_fini (&btrace);
   return 0;
-
-err:
-  btrace_data_fini (&btrace);
-  return -1;
 }
 
 /* See to_btrace_conf target method.  */