On 2 December 2015 at 22:51, Michael Davidsaver <mdavidsa...@gmail.com> wrote:
>
>
> On 11/17/2015 12:09 PM, Peter Maydell wrote:
>> On 9 November 2015 at 01:11, Michael Davidsaver <mdavidsa...@gmail.com>
>> wrote:
>>> The MRS and MSR instruction handling isn't checking
>>> the current permission level.
>>>
>>> Signed-off-by: Michael Davidsaver <mdavidsa...@gmail.com>
>>> ---
>>> target-arm/helper.c | 79
>>> +++++++++++++++++++++++++----------------------------
>>> 1 file changed, 37 insertions(+), 42 deletions(-)
>>
>> This patch looks good overall, but there's one style nit:
>>
>>> + case 0 ... 7: /* xPSR sub-fields */
>>> + mask = 0;
>>> + if ((reg&1) && el) {
>>
>> you want spaces around operators, so "reg & 1" here and elsewhere.
>
> Would be nice if checkpatch.pl caught these, but I understand that
> this would be quite difficult to do well. I've tried to catch this
> with grep and sort through the false positives. I think I got them
> all.
It looks like this is a case where checkpatch.pl is applying
Linux kernel style (since that's where we borrowed it from)
which is looser than ours, or at least looser than what I think
our style is. There's a specific check:
# << and >> may either have or not have spaces both sides
} elsif ($op eq '<<' or $op eq '>>' or
$op eq '&' or $op eq '^' or $op eq '|' or
$op eq '+' or $op eq '-' or
$op eq '*' or $op eq '/' or
$op eq '%')
and those operators are only checked for "must have consistent
spacing both sides" rather than "must have spaces on both sides".
thanks
-- PMM
