[PATCHv3,2/3] 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.
(typedef btrace_pt_packet_s): Delete.
(struct btrace_maint_info) <packets>: Change fromm VEC to
std::vector.
---
gdb/ChangeLog | 13 +++++++++++++
gdb/btrace.c | 39 +++++++++++++++++++++------------------
gdb/btrace.h | 6 +-----
3 files changed, 35 insertions(+), 23 deletions(-)
Comments
Hello Andrew,
> @@ -2986,8 +2986,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>;
> + maint->variant.pt.packets->push_back (packet);
We shouldn't do that check inside the loop. I used VEC_safe_push for the realloc aspect.
> @@ -3098,11 +3101,13 @@ 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 == nullptr
> + || btinfo->maint.variant.pt.packets->empty ())
> btrace_maint_update_pt_packets (btinfo);
Here, I'd check for nullptr in order to allocate the vector and call
btrace_maint_update_pt_packets. Afterwards, I'd check for emptiness and delete
the vector again if there are no packets.
We can also leave an empty vector if you prefer that.
Regards,
Markus.
Intel Deutschland GmbH
Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
Tel: +49 89 99 8853-0, www.intel.de
Managing Directors: Christin Eisenschmid, Gary Kershaw
Chairperson of the Supervisory Board: Nicole Lau
Registered Office: Munich
Commercial Register: Amtsgericht Muenchen HRB 186928
* Metzger, Markus T <markus.t.metzger@intel.com> [2019-09-26 08:47:28 +0000]:
> Hello Andrew,
>
> > @@ -2986,8 +2986,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>;
> > + maint->variant.pt.packets->push_back (packet);
>
> We shouldn't do that check inside the loop. I used VEC_safe_push
> for the realloc aspect.
Thanks, I pushed the vector allocation out of the loop.
>
>
> > @@ -3098,11 +3101,13 @@ 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 == nullptr
> > + || btinfo->maint.variant.pt.packets->empty ())
> > btrace_maint_update_pt_packets (btinfo);
>
> Here, I'd check for nullptr in order to allocate the vector and call
> btrace_maint_update_pt_packets. Afterwards, I'd check for emptiness and delete
> the vector again if there are no packets.
>
> We can also leave an empty vector if you prefer that.
I hope I've interpreted your suggestion correctly. Please do check my
next version and correct me if I've not understood you.
Thanks,
Andrew
>
> Regards,
> Markus.
>
> Intel Deutschland GmbH
> Registered Address: Am Campeon 10-12, 85579 Neubiberg, Germany
> Tel: +49 89 99 8853-0, www.intel.de
> Managing Directors: Christin Eisenschmid, Gary Kershaw
> Chairperson of the Supervisory Board: Nicole Lau
> Registered Office: Munich
> Commercial Register: Amtsgericht Muenchen HRB 186928
>
@@ -1827,7 +1827,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;
@@ -2986,8 +2986,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>;
+ maint->variant.pt.packets->push_back (packet);
}
}
@@ -2995,8 +2997,9 @@ 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>;
+ maint->variant.pt.packets->push_back (packet);
warning (_("Error at trace offset 0x%" PRIx64 ": %s."),
packet.offset, pt_errstr (packet.errcode));
@@ -3098,11 +3101,13 @@ 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 == nullptr
+ || 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 == nullptr)
+ ? 0 : 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 +3150,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> *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 +3455,9 @@ 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 == nullptr)
+ ? 0 : 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>
@@ -265,9 +264,6 @@ struct btrace_pt_packet
struct pt_packet 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 +297,7 @@ struct btrace_maint_info
struct
{
/* A vector of decoded packets. */
- VEC (btrace_pt_packet_s) *packets;
+ std::vector <btrace_pt_packet> *packets;
/* The packet history iterator.
We are iterating over the above PACKETS vector. */