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

Andrew Burgess
2019-09-26T11:33:43Z

* 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
>