RE: [patch v9 01/23] thread, btrace: add generic branch trace support

Metzger, Markus T
2013-03-07T07:50:22Z

> -----Original Message-----
> From: Doug Evans [mailto:dje@google.com]
> Sent: Wednesday, March 06, 2013 10:12 PM

Thanks for your review.

> > + /* The function index range [begin; end[ that has been covered last time.
>
> Typo: end]

That's intended to indicate that begin is included in the range whereas end isn't.
I see that this has not been clear enough. I'll reword it.

> > +/* A branch trace block.
> > +
> > + This represents a block of sequential control-flow. Adjacent blocks will be
> > + connected via calls, returns, or jumps. The latter can be direct or
> > + indirect, conditional or unconditional. Branches can further be
> > + asynchronous, e.g. interrupts. */
> > +struct btrace_block
> > +{
> > + /* The address of the first instruction in the block. */
> > + CORE_ADDR begin;
> > +
> > + /* The address of the last instruction in the block. */
> > + CORE_ADDR end;
> > +};
>
> Can you elaborate in the docs for "end" what it is?
> E.g., on an ISA with only 4 byte instructions, and the block contains
> two instructions, is end == begin+4 or begin+7 or begin+8?
> I'd guess that it's begin+4, but IWBN if the comment
> removed all doubt.

I changed both comments to say "the first byte of ...".

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