Re: [Xen-devel] [PATCH] x86/PV: avoid indirect call/thunk in I/O emulation

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/02/18 08:00, Jan Beulich wrote:
>>>> On 15.02.18 at 17:53, <andrew.cooper3@xxxxxxxxxx> wrote:
>> On 15/02/18 16:03, Jan Beulich wrote:
>>> --- a/xen/arch/x86/pv/emul-priv-op.c
>>> +++ b/xen/arch/x86/pv/emul-priv-op.c
>>> @@ -73,55 +73,42 @@ void (*pv_post_outb_hook)(unsigned int p
>>>  
>>>  typedef void io_emul_stub_t(struct cpu_user_regs *);
>>>  
>>> -void __x86_indirect_thunk_rcx(void);
>>> -
>>>  static io_emul_stub_t *io_emul_stub_setup(struct priv_op_ctxt *ctxt, u8 
>>> opcode,
>>>                                            unsigned int port, unsigned int 
>>> bytes)
>>>  {
>>>      struct stubs *this_stubs = &this_cpu(stubs);
>>>      unsigned long stub_va = this_stubs->addr + STUB_BUF_SIZE / 2;
>>> +    signed long disp;
>> I'm not in favour of sprinkling 'signed' all over the code base.
>>
>> long is already unambiguous, and a far more common construct to encounter. 
> It's this "common" which made me use "signed" here (and in similar
> places elsewhere) - we still have far too many cases left where plain
> short/int/long are used when the unsigned variant would actually be
> better. Hence, _at least_ until those issues are all gone, I very much
> prefer making explicit when signed quantities are meant.

But why does it actually matter?  Noone can blindly switch between
prefixed and unprefixed versions without understanding what is going on,
and disp here is a signed usage.

Also, are you proposing that the end goal for a "clean" coding style is
to have all native C types explicitly prefixed with signed or unsigned? 
If not (and I really hope not), then putting signed in here is causing
extra work for us to undo later.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel