On Thu, Jun 15, 2017 at 18:19:11 -0400, Emilio G. Cota wrote:
> (snip)
> > +/**
> > + * DisasContextBase:
> > + * @tb: Translation block for this disassembly.
> > + * @pc_first: Address of first guest instruction in this TB.
> > + * @pc_next: Address of next guest instruction in this TB (current during
> > + *           disassembly).
> > + * @num_insns: Number of translated instructions (including current).
> > + * @singlestep_enabled: "Hardware" single stepping enabled.
> > + *
> > + * Architecture-agnostic disassembly context.
> > + */
> > +typedef struct DisasContextBase {
> > +    TranslationBlock *tb;
> > +    target_ulong pc_first;
> > +    target_ulong pc_next;
> > +    DisasJumpType jmp_type;
> > +    unsigned int num_insns;
> > +    bool singlestep_enabled;
> > +} DisasContextBase;
> 
> - @pc_next: I'd stick with @pc, it's shorter, it's everywhere already, and
>   with the documentation it's very clear what it is for.
> - @jmp_type: missing doc :-)

Also, consider keeping the @is_jmp name instead of renaming it to
@jmp_type. (@jmp would be shorter but it would be confusing though,
e.g. cris has both dc->jmp and dc->is_jmp.)

                E.

Reply via email to