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

[Alexey Kardashevskiy <aik@ozlabs.ru>]

On 08/20/2015 02:15 AM, Thomas Huth wrote:
> 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!

I still prefer the clarity and therefore more braces which make no harm 
whatsoever.



-- 
Alexey