[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

On Fri, Feb 16, 2018 at 5:02 PM, Andrew Cooper
<andrew.cooper3@xxxxxxxxxx> wrote:
> On 16/02/18 16:21, Jan Beulich wrote:
>>>>> On 16.02.18 at 16:50, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> 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.
>> Personally I would indeed prefer for "signed" to always be added
>> when a signed quantity is meant. Of course you have to look at
>> the context when considering to switch a plain use to unsigned,
>> but quickly recognizing non-candidates of such a transformation
>> is possible only when they are amended by "signed".
> The first thing I ever thing when seeing "signed" is "the author doesn't
> understand C", which is a direct consequence of being an redundant
> keyword which is barely used.  The only case I'm aware of it actually
> having a semantic use is with char, whose signed-ness is implementation
> defined, rather than specified.
> Personally, I find that redundant signed hinders readability, rather
> than helping it, and I'd certainly prefer not to see signed inserted all
> over the codebase.  Either way, the topic should be discussed as part of
> CODING_STYLE, rather than being argued over in a single patch.


FWIW I'd also like to avoid using 'signed''.


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.