Re: [Qemu-devel] [PATCH 0/14] target/mips: Add Enhanced Virtual Addressing (EVA) support

[James Hogan]

On Tue, Jul 18, 2017 at 02:21:20PM -0700, address@hidden wrote:
> Checking PATCH 2/14: target/mips: Fix TLBWI shadow flush for EHINV, XI, RI...
> ERROR: space prohibited after that '&' (ctx:WxW)
> #44: FILE: target/mips/op_helper.c:2045:
> +    XI0 = (env->CP0_EntryLo0 >> CP0EnLo_XI) & 1;
>                                              ^
> 
> ERROR: space prohibited after that '&' (ctx:WxW)
> #45: FILE: target/mips/op_helper.c:2046:
> +    RI0 = (env->CP0_EntryLo0 >> CP0EnLo_RI) & 1;
>                                              ^
> 
> ERROR: space prohibited after that '&' (ctx:WxW)
> #48: FILE: target/mips/op_helper.c:2049:
> +    XI1 = (env->CP0_EntryLo1 >> CP0EnLo_XI) & 1;
>                                              ^
> 
> ERROR: space prohibited after that '&' (ctx:WxW)
> #49: FILE: target/mips/op_helper.c:2050:
> +    RI1 = (env->CP0_EntryLo1 >> CP0EnLo_RI) & 1;
>                                              ^
> 
> total: 4 errors, 0 warnings, 34 lines checked

These are false positives. The code looks fine to me. They are binary
bitwise-and operators, not unary address-of operators.

> Your patch has style problems, please review.  If any of these errors
> are false positives report them to the maintainer, see
> CHECKPATCH in MAINTAINERS.

It looks orphaned.

> Checking PATCH 10/14: target/mips: Add an MMU mode for ERL...
> ERROR: trailing statements should be on next line
> #94: FILE: target/mips/op_helper.c:98:
> +    case 3: cpu_##insn##_error_ra(env, addr, val, retaddr); break;      \
> 
> ERROR: trailing statements should be on next line
> #102: FILE: target/mips/op_helper.c:1456:
> +        case 3: qemu_log(", ERL\n"); break;
> 
> ERROR: trailing statements should be on next line
> #110: FILE: target/mips/op_helper.c:2251:
> +        case 3: qemu_log(", ERL\n"); break;
> 
> total: 3 errors, 0 warnings, 79 lines checked

These are all consistent with the surrounding code.

> Checking PATCH 12/14: target/mips: Implement segmentation control...
> ERROR: braces {} are necessary for all arms of this statement
> #246: FILE: target/mips/helper.c:836:
> +            if ((R != 0 || UX) && (R != 3 || KX) &&
> [...]
> 
> ERROR: braces {} are necessary for all arms of this statement
> #258: FILE: target/mips/helper.c:854:
> +            if ((R != 0 || UX) && (R != 3 || KX) &&
> [...]
> 
> total: 2 errors, 0 warnings, 234 lines checked

And these are both pre-existing style issues that the patch hasn't
changed.

Cheers
James

signature.asc
Description: Digital signature