RE: [patch v7 22/24] btrace, gdbserver: read branch trace incrementally

["Metzger, Markus T" <markus.t.metzger@intel.com>]

> -----Original Message-----
> From: Jan Kratochvil [mailto:jan.kratochvil@redhat.com]
> Sent: Wednesday, December 11, 2013 8:58 PM

Thanks for your review.

> > +static int
> > +btrace_stitch_trace (VEC (btrace_block_s) **btrace,
> > +		     const struct btrace_thread_info *btinfo)
> > +{
> > +  struct btrace_function *end;
> > +  struct btrace_insn *insn;
> > +  btrace_block_s *block;
> > +
> > +  /* If we don't have trace, there's nothing to do.  */
> > +  if (VEC_empty (btrace_block_s, *btrace))
> > +    return 0;
> > +
> > +  end = btinfo->end;
> > +  gdb_assert (end != NULL);
> > +
> > +  block = VEC_last (btrace_block_s, *btrace);
> > +  insn = VEC_last (btrace_insn_s, end->insn);
> 
> At least call block and insn somehow specific from where they come from.
> Maybe btrace_block and btinfo_end.  Also end should be called btinfo_end
> (if
> the extra variable still makes sense in such case).
> 
> I would even call it new_btrace and old_btinfo with variables old_end etc.

Hmm, btrace_block and btinfo_end doesn't help very much, I think.

What this function does is stitch the old and new trace together by inserting
the last instruction of the old trace as beginning of the chronologically first
block of the new trace.  Then it removes the last instruction from the old
trace - it will be re-added when we process the new trace.

I renamed the variables as follows:
    end -> last_bfun
    insn -> last_insn
    block -> first_new_block

I also added a comment explaining why first_new_block is assigned from
VEC_last ().

I hope those names are clearer.

Regards,
Markus.

Intel GmbH
Dornacher Strasse 1
85622 Feldkirchen/Muenchen, Deutschland
Sitz der Gesellschaft: Feldkirchen bei Muenchen
Geschaeftsfuehrer: Christian Lamprechter, Hannes Schwaderer, Douglas Lusk
Registergericht: Muenchen HRB 47456
Ust.-IdNr./VAT Registration No.: DE129385895
Citibank Frankfurt a.M. (BLZ 502 109 00) 600119052