Re: [Qemu-devel] [PATCH v5 3/4] sPAPR: Support RTAS call ibm, {open, close}-errinjct

[Thomas Huth]

On 18/08/15 18:15, David Gibson wrote:
> On Wed, Aug 19, 2015 at 09:52:00AM +1000, Gavin Shan wrote:
>> On Tue, Aug 18, 2015 at 10:32:13AM -0700, Thomas Huth wrote:
>>> On 17/08/15 18:47, Gavin Shan wrote:
>>>> The patch supports RTAS calls "ibm,{open,close}-errinjct" to
>>>> manupliate the token, which is passed to RTAS call "ibm,errinjct"
>>>> to indicate the valid context for error injection. Each VM is
>>>> permitted to have only one token at once and we simply have one
>>>> random number for that.
>>>
>>> Looking at the code, you're using a sequence number now instead of a
>>> random number?
>>>
>>
>> Yes, it's what Alexey suggested.

Then please update the commit message accordingly.

>>>> diff --git a/hw/ppc/spapr_rtas.c b/hw/ppc/spapr_rtas.c
>>>> index e99e25f..8405056 100644
>>>> --- a/hw/ppc/spapr_rtas.c
>>>> +++ b/hw/ppc/spapr_rtas.c
>>>> @@ -604,6 +604,68 @@ out:
>>>>      rtas_st(rets, 0, rc);
>>>>  }
>>>>  
>>>> +static void rtas_ibm_open_errinjct(PowerPCCPU *cpu,
>>>> +                                   sPAPRMachineState *spapr,
>>>> +                                   uint32_t token, uint32_t nargs,
>>>> +                                   target_ulong args, uint32_t nret,
>>>> +                                   target_ulong rets)
>>>> +{
>>>> +    int32_t ret;
>>>> +
>>>> +    /* Sanity check on number of arguments */
>>>> +    if ((nargs != 0) || (nret != 2)) {
>>>
>>> Uh, did Alexey infect you with paranthesitis?
>>>
>>
>> hehe~, nope. I'll drop those unnecessary paranthesitis :-)
> 
> I'd prefer you didn't.  Unlike Thomas, I also don't remember C order
> of ops that well and would prefer the clarity.

You can always look it up if you're unsure, e.g.:

http://en.cppreference.com/w/c/language/operator_precedence

And once you've learnt it, the additional paranthesis just look
cumbersome. So please remove them!

 Thomas

signature.asc
Description: OpenPGP digital signature