On Wed, Aug 19, 2015 at 09:15:26AM -0700, 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.
>
Yes, I'll update changelog 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!
>
Ok. I'll check the code and remove unnecessary paranthesis in next revision.
Thanks,
Gavin
