Re: [Xen-devel] [PATCH 2/2] x86emul: consolidate string insn address increments

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 12/12/16 07:27, Jan Beulich wrote:
>>>> On 09.12.16 at 18:11, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 09/12/16 15:22, Jan Beulich wrote:
>>> Move the looking at EFLAGS.DF into the macro, rendering all call sites
>>> more readable.
>>>
>>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>> The net change is ok; it is certainly cleaner to read in the body of
>> x86_emulate().
>>
>> However, the naming of register_address_increment() was previously ok,
>> as it was obvious at the calling point that a negative increment was
>> possible.  This subtly is now hidden.
>>
>> How about reg_addr_adjust() or reg_addr_adjust_dir() as an alternative
>> name?  This retains the property that it is obvious that the direction
>> flag is followed in the calculation.
> That's fine with me; I admit that I've overlooked that the use of
> "increment" in the name has now become further disconnected
> from the actual operation. In line with
> _register_address_increment() I'd prefer to call it
> _register_address_adjust() though; in fact it might be reasonable
> to also replace "increment" with "adjust" in the other one (as it's
> similarly used for adjustments in both directions). What do you
> think?

register_address_adjust() was my first alternative name, so ok.

With that changed, Revewed-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
to save another trip on email.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel