Lluís Vilanova writes:
> Emilio G Cota writes:
>> 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.)
> I just figured that this series could also take the chance of trying to
> rename a
> few common variables I'm changing to something more readable.
> But if you feel very strongly about keeping the original names (and minimizing
> the diffs as you say later), I'll revert the name changes.
Also, going through the changes to break them down into smaller pieces, I saw
that TranslationBlock (at least in i386) already has a "pc" member, so using
"pc_next" in DisasContextBase makes it even clearer it's a different variable.
You comments still apply to "is_jmp" vs "jmp_type" though. Unless you or anybody
else feels strongly against it, I'll keep "jmp_type", since I'm already changing
all lines that reference "is_jmp" to use DisasContextBase (instead of
DisasContext).
Thanks,
Lluis
