[2/2] gdb: Change a VEC to std::vector in btrace.{c,h}
Commit Message
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
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
@@ -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) */
@@ -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. */