diff mbox

[2/2] gdb: Change a VEC to std::vector in btrace.{c,h}

Message ID 2754d81c341f0cbb6240e309f93d3b9efb38c3e3.1569276387.git.andrew.burgess@embecosm.com
State New
Headers show

Commit Message

Andrew Burgess Sept. 23, 2019, 10:09 p.m. UTC 
Replace a VEC with a std::vector in btrace.h, and update btrace.c to
match.  This code is currently untested, so I've been quite
conservative with the changes - where previously we used VEC_safe_push
which would allocate the vector if it isn't already allocated - I now
check and allocate the std::vector using 'new' before pushing to the
vector.

As the new vector is inside a union I've currently used a pointer to
vector, which makes the code slightly uglier than it might otherwise
be, but again, due to lack of testing I'm reluctant to start
refactoring the code in a big way.

gdb/ChangeLog:

	* btrace.c (btrace_maint_clear): Update to handle change from VEC
	to std::vector.
	(btrace_maint_decode_pt): Likewise.
	(btrace_maint_update_packets): Likewise.
	(btrace_maint_print_packets): Likewise.
	(maint_info_btrace_cmd): Likewise.
	* btrace.h: Remove 'vec.h' header include and use of DEF_VEC_O.
	(struct btrace_maint_info) <packets>: Change fromm VEC to
	std::vector.
---
 gdb/ChangeLog | 12 ++++++++++++
 gdb/btrace.c  | 37 +++++++++++++++++++------------------
 gdb/btrace.h  |  4 +---
 3 files changed, 32 insertions(+), 21 deletions(-)

Comments

Simon Marchi Sept. 24, 2019, 2:19 a.m. UTC | #1 
On 2019-09-23 6:09 p.m., Andrew Burgess wrote:
> Replace a VEC with a std::vector in btrace.h, and update btrace.c to
> match.  This code is currently untested, so I've been quite
> conservative with the changes - where previously we used VEC_safe_push
> which would allocate the vector if it isn't already allocated - I now
> check and allocate the std::vector using 'new' before pushing to the
> vector.
> 
> As the new vector is inside a union I've currently used a pointer to
> vector, which makes the code slightly uglier than it might otherwise
> be, but again, due to lack of testing I'm reluctant to start
> refactoring the code in a big way.

Hi Andrew,

Looks fine to me, just a few nits:

> diff --git a/gdb/btrace.c b/gdb/btrace.c
> index 5013b568a45..59e2a49bcd8 100644
> --- a/gdb/btrace.c
> +++ b/gdb/btrace.c
> @@ -1828,7 +1828,7 @@ btrace_maint_clear (struct btrace_thread_info *btinfo)
>  
>  #if defined (HAVE_LIBIPT)
>      case BTRACE_FORMAT_PT:
> -      xfree (btinfo->maint.variant.pt.packets);
> +      delete (btinfo->maint.variant.pt.packets);

You can remove the parenthesis.

>  
>        btinfo->maint.variant.pt.packets = NULL;
>        btinfo->maint.variant.pt.packet_history.begin = 0;
> @@ -2987,8 +2987,10 @@ btrace_maint_decode_pt (struct btrace_maint_info *maint,
>  	  if (maint_btrace_pt_skip_pad == 0 || packet.packet.type != ppt_pad)
>  	    {
>  	      packet.errcode = pt_errcode (errcode);
> -	      VEC_safe_push (btrace_pt_packet_s, maint->variant.pt.packets,
> -			     &packet);
> +	      if (maint->variant.pt.packets == NULL)
> +		maint->variant.pt.packets
> +		  = new std::vector <btrace_pt_packet_s>;

This will fit on a single line once you remove the `_s` :).

> --- a/gdb/btrace.h
> +++ b/gdb/btrace.h
> @@ -29,7 +29,6 @@
>  #include "gdbsupport/btrace-common.h"
>  #include "target/waitstatus.h" /* For enum target_stop_reason.  */
>  #include "gdbsupport/enum-flags.h"
> -#include "gdbsupport/vec.h"
>  
>  #if defined (HAVE_LIBIPT)
>  #  include <intel-pt.h>
> @@ -267,7 +266,6 @@ struct btrace_pt_packet
>  
>  /* Define functions operating on a vector of packets.  */
>  typedef struct btrace_pt_packet btrace_pt_packet_s;
> -DEF_VEC_O (btrace_pt_packet_s);

Like in the previous patch, you can drop this typedef.  It was only useful to define the VEC.

Simon
diff mbox

Patch

diff --git a/gdb/btrace.c b/gdb/btrace.c
index 5013b568a45..59e2a49bcd8 100644
--- a/gdb/btrace.c
+++ b/gdb/btrace.c
@@ -1828,7 +1828,7 @@  btrace_maint_clear (struct btrace_thread_info *btinfo)
 
 #if defined (HAVE_LIBIPT)
     case BTRACE_FORMAT_PT:
-      xfree (btinfo->maint.variant.pt.packets);
+      delete (btinfo->maint.variant.pt.packets);
 
       btinfo->maint.variant.pt.packets = NULL;
       btinfo->maint.variant.pt.packet_history.begin = 0;
@@ -2987,8 +2987,10 @@  btrace_maint_decode_pt (struct btrace_maint_info *maint,
 	  if (maint_btrace_pt_skip_pad == 0 || packet.packet.type != ppt_pad)
 	    {
 	      packet.errcode = pt_errcode (errcode);
-	      VEC_safe_push (btrace_pt_packet_s, maint->variant.pt.packets,
-			     &packet);
+	      if (maint->variant.pt.packets == NULL)
+		maint->variant.pt.packets
+		  = new std::vector <btrace_pt_packet_s>;
+	      maint->variant.pt.packets->push_back (packet);
 	    }
 	}
 
@@ -2996,8 +2998,10 @@  btrace_maint_decode_pt (struct btrace_maint_info *maint,
 	break;
 
       packet.errcode = pt_errcode (errcode);
-      VEC_safe_push (btrace_pt_packet_s, maint->variant.pt.packets,
-		     &packet);
+      if (maint->variant.pt.packets == NULL)
+	maint->variant.pt.packets
+	  = new std::vector <btrace_pt_packet_s>;
+      maint->variant.pt.packets->push_back (packet);
 
       warning (_("Error at trace offset 0x%" PRIx64 ": %s."),
 	       packet.offset, pt_errstr (packet.errcode));
@@ -3098,11 +3102,11 @@  btrace_maint_update_packets (struct btrace_thread_info *btinfo,
 
 #if defined (HAVE_LIBIPT)
     case BTRACE_FORMAT_PT:
-      if (VEC_empty (btrace_pt_packet_s, btinfo->maint.variant.pt.packets))
+      if (btinfo->maint.variant.pt.packets->empty ())
 	btrace_maint_update_pt_packets (btinfo);
 
       *begin = 0;
-      *end = VEC_length (btrace_pt_packet_s, btinfo->maint.variant.pt.packets);
+      *end = btinfo->maint.variant.pt.packets->size ();
       *from = btinfo->maint.variant.pt.packet_history.begin;
       *to = btinfo->maint.variant.pt.packet_history.end;
       break;
@@ -3145,23 +3149,21 @@  btrace_maint_print_packets (struct btrace_thread_info *btinfo,
 #if defined (HAVE_LIBIPT)
     case BTRACE_FORMAT_PT:
       {
-	VEC (btrace_pt_packet_s) *packets;
+	std::vector <btrace_pt_packet_s> *packets;
 	unsigned int pkt;
 
 	packets = btinfo->maint.variant.pt.packets;
 	for (pkt = begin; pkt < end; ++pkt)
 	  {
-	    const struct btrace_pt_packet *packet;
-
-	    packet = VEC_index (btrace_pt_packet_s, packets, pkt);
+	    const struct btrace_pt_packet &packet = packets->at (pkt);
 
 	    printf_unfiltered ("%u\t", pkt);
-	    printf_unfiltered ("0x%" PRIx64 "\t", packet->offset);
+	    printf_unfiltered ("0x%" PRIx64 "\t", packet.offset);
 
-	    if (packet->errcode == pte_ok)
-	      pt_print_packet (&packet->packet);
+	    if (packet.errcode == pte_ok)
+	      pt_print_packet (&packet.packet);
 	    else
-	      printf_unfiltered ("[error: %s]", pt_errstr (packet->errcode));
+	      printf_unfiltered ("[error: %s]", pt_errstr (packet.errcode));
 
 	    printf_unfiltered ("\n");
 	  }
@@ -3452,9 +3454,8 @@  maint_info_btrace_cmd (const char *args, int from_tty)
 			   version.ext != NULL ? version.ext : "");
 
 	btrace_maint_update_pt_packets (btinfo);
-	printf_unfiltered (_("Number of packets: %u.\n"),
-			   VEC_length (btrace_pt_packet_s,
-				       btinfo->maint.variant.pt.packets));
+	printf_unfiltered (_("Number of packets: %zu.\n"),
+			   btinfo->maint.variant.pt.packets->size ());
       }
       break;
 #endif /* defined (HAVE_LIBIPT)  */
diff --git a/gdb/btrace.h b/gdb/btrace.h
index d09c424804b..cea8cd9ad6a 100644
--- a/gdb/btrace.h
+++ b/gdb/btrace.h
@@ -29,7 +29,6 @@ 
 #include "gdbsupport/btrace-common.h"
 #include "target/waitstatus.h" /* For enum target_stop_reason.  */
 #include "gdbsupport/enum-flags.h"
-#include "gdbsupport/vec.h"
 
 #if defined (HAVE_LIBIPT)
 #  include <intel-pt.h>
@@ -267,7 +266,6 @@  struct btrace_pt_packet
 
 /* Define functions operating on a vector of packets.  */
 typedef struct btrace_pt_packet btrace_pt_packet_s;
-DEF_VEC_O (btrace_pt_packet_s);
 #endif /* defined (HAVE_LIBIPT)  */
 
 /* Branch trace iteration state for "maintenance btrace packet-history".  */
@@ -301,7 +299,7 @@  struct btrace_maint_info
     struct
     {
       /* A vector of decoded packets.  */
-      VEC (btrace_pt_packet_s) *packets;
+      std::vector <btrace_pt_packet_s> *packets;
 
       /* The packet history iterator.
 	 We are iterating over the above PACKETS vector.  */