qemu-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-devel] [RFC PATCH v3 0/6] translate: [tcg] Generic translation


From: no-reply
Subject: Re: [Qemu-devel] [RFC PATCH v3 0/6] translate: [tcg] Generic translation framework
Date: Tue, 27 Dec 2016 08:07:57 -0800 (PST)

Hi,

Your series seems to have some coding style problems. See output below for
more information:

Message-id: address@hidden
Type: series
Subject: [Qemu-devel] [RFC PATCH v3 0/6] translate: [tcg] Generic translation 
framework

=== TEST SCRIPT BEGIN ===
#!/bin/bash

BASE=base
n=1
total=$(git log --oneline $BASE.. | wc -l)
failed=0

# Useful git options
git config --local diff.renamelimit 0
git config --local diff.renames True

commits="$(git log --format=%H --reverse $BASE..)"
for c in $commits; do
    echo "Checking PATCH $n/$total: $(git log -n 1 --format=%s $c)..."
    if ! git show $c --format=email | ./scripts/checkpatch.pl --mailback -; then
        failed=1
        echo
    fi
    n=$((n+1))
done

exit $failed
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/address@hidden -> patchew/address@hidden
Switched to a new branch 'test'
f811b31 target: [tcg, arm] Port to generic translation framework
c032b9c target: [tcg, i386] Port to generic translation framework
2d11071 target: [tcg] Redefine DISAS_* onto the generic translation framework 
(DJ_*)
510f515 target: [tcg] Add generic translation framework
d6c2729 queue: Add macro for incremental traversal
e91a708 Pass generic CPUState to gen_intermediate_code()

=== OUTPUT BEGIN ===
Checking PATCH 1/6: Pass generic CPUState to gen_intermediate_code()...
ERROR: "foo * bar" should be "foo *bar"
#829: FILE: target/sh4/translate.c:1832:
+    CPUSH4State * env = cpu->env_ptr;

ERROR: "foo * bar" should be "foo *bar"
#891: FILE: target/sparc/translate.c:5693:
+    CPUSPARCState * env = cpu->env_ptr;

total: 2 errors, 0 warnings, 926 lines checked

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

Checking PATCH 2/6: queue: Add macro for incremental traversal...
Checking PATCH 3/6: target: [tcg] Add generic translation framework...
ERROR: open brace '{' following enum go on the same line
#64: FILE: include/exec/translate-all_template.h:34:
+typedef enum BreakpointHitType
+{

ERROR: open brace '{' following enum go on the same line
#79: FILE: include/exec/translate-all_template.h:49:
+typedef enum DisasJumpType
+{

ERROR: open brace '{' following struct go on the same line
#97: FILE: include/exec/translate-all_template.h:67:
+typedef struct DisasContextBase
+{

ERROR: line over 90 characters
#116: FILE: include/qom/cpu.h:952:
+static inline CPUBreakpoint *cpu_breakpoint_get(CPUState *cpu, vaddr pc, 
CPUBreakpoint *bp)

ERROR: "foo * bar" should be "foo *bar"
#170: FILE: translate-all_template.h:26:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: "foo * bar" should be "foo *bar"
#173: FILE: translate-all_template.h:29:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: "foo * bar" should be "foo *bar"
#176: FILE: translate-all_template.h:32:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: "foo * bar" should be "foo *bar"
#179: FILE: translate-all_template.h:35:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: "foo * bar" should be "foo *bar"
#182: FILE: translate-all_template.h:38:
+    DisasContext * restrict dc, CPUArchState * restrict env,

ERROR: "foo * bar" should be "foo *bar"
#183: FILE: translate-all_template.h:39:
+    const CPUBreakpoint * restrict bp);

ERROR: "foo * bar" should be "foo *bar"
#186: FILE: translate-all_template.h:42:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: "foo * bar" should be "foo *bar"
#189: FILE: translate-all_template.h:45:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: "foo * bar" should be "foo *bar"
#192: FILE: translate-all_template.h:48:
+    DisasContext * restrict dc, CPUArchState * restrict env);

ERROR: line over 90 characters
#257: FILE: translate-all_template.h:113:
+                BreakpointHitType bh = 
gen_intermediate_code_target_breakpoint_hit(dc, env, bp);

WARNING: line over 80 characters
#335: FILE: translate-all_template.h:191:
+        log_target_disas(cpu, dc->base.pc_first, dc->base.pc_next - 
dc->base.pc_first,

total: 14 errors, 1 warnings, 313 lines checked

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

Checking PATCH 4/6: target: [tcg] Redefine DISAS_* onto the generic translation 
framework (DJ_*)...
ERROR: spaces required around that '+' (ctx:VxV)
#31: FILE: include/exec/exec-all.h:42:
+#define DISAS_JUMP    (DJ_TARGET+0)     /* only pc was modified dynamically */
                                 ^

ERROR: spaces required around that '+' (ctx:VxV)
#32: FILE: include/exec/exec-all.h:43:
+#define DISAS_UPDATE  (DJ_TARGET+1)     /* cpu state was modified dynamically 
*/
                                 ^

ERROR: spaces required around that '+' (ctx:VxV)
#33: FILE: include/exec/exec-all.h:44:
+#define DISAS_TB_JUMP (DJ_TARGET+2)     /* only pc was modified statically */
                                 ^

ERROR: spaces required around that '+' (ctx:VxV)
#34: FILE: include/exec/exec-all.h:45:
+#define DISAS_TARGET  (DJ_TARGET+3)     /* base for target-specific values */
                                 ^

ERROR: Macros with complex values should be enclosed in parenthesis
#53: FILE: target/arm/translate.h:113:
+#define DISAS_WFI DISAS_TARGET + 0

ERROR: Macros with complex values should be enclosed in parenthesis
#54: FILE: target/arm/translate.h:114:
+#define DISAS_SWI DISAS_TARGET + 1

ERROR: Macros with complex values should be enclosed in parenthesis
#59: FILE: target/arm/translate.h:118:
+#define DISAS_EXC DISAS_TARGET + 2

ERROR: Macros with complex values should be enclosed in parenthesis
#65: FILE: target/arm/translate.h:120:
+#define DISAS_WFE DISAS_TARGET + 3

ERROR: Macros with complex values should be enclosed in parenthesis
#66: FILE: target/arm/translate.h:121:
+#define DISAS_HVC DISAS_TARGET + 4

ERROR: Macros with complex values should be enclosed in parenthesis
#67: FILE: target/arm/translate.h:122:
+#define DISAS_SMC DISAS_TARGET + 5

ERROR: Macros with complex values should be enclosed in parenthesis
#68: FILE: target/arm/translate.h:123:
+#define DISAS_YIELD DISAS_TARGET + 6

ERROR: Macros with complex values should be enclosed in parenthesis
#82: FILE: target/cris/translate.c:54:
+#define DISAS_SWI DISAS_TARGET + 0

ERROR: Macros with complex values should be enclosed in parenthesis
#96: FILE: target/m68k/translate.c:147:
+#define DISAS_JUMP_NEXT DISAS_TARGET + 0

ERROR: Macros with complex values should be enclosed in parenthesis
#110: FILE: target/s390x/translate.c:78:
+#define DISAS_EXCP DISAS_TARGET + 0

ERROR: Macros with complex values should be enclosed in parenthesis
#126: FILE: target/unicore32/translate.c:51:
+#define DISAS_SYSCALL DISAS_TARGET + 0

total: 15 errors, 0 warnings, 84 lines checked

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

Checking PATCH 5/6: target: [tcg, i386] Port to generic translation framework...
ERROR: spaces required around that '+' (ctx:VxV)
#21: FILE: target/i386/translate.c:73:
+#define DJ_JUMP (DJ_TARGET+0)           /* end of block due to call/jump */
                           ^

ERROR: spaces required around that '+' (ctx:VxV)
#22: FILE: target/i386/translate.c:74:
+#define DJ_MISC (DJ_TARGET+1)           /* some other reason */
                           ^

ERROR: "foo * bar" should be "foo *bar"
#213: FILE: target/i386/translate.c:4399:
+    DisasContext * restrict s, CPUX86State * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#431: FILE: target/i386/translate.c:8346:
+    DisasContext * restrict dc, CPUX86State * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#481: FILE: target/i386/translate.c:8402:
+    DisasContext * restrict dc, CPUX86State * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#503: FILE: target/i386/translate.c:8419:
+    DisasContext * restrict dc, CPUX86State * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#528: FILE: target/i386/translate.c:8424:
+    DisasContext * restrict dc, CPUX86State * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#576: FILE: target/i386/translate.c:8430:
+    DisasContext * restrict dc, CPUX86State * restrict env,

ERROR: "foo * bar" should be "foo *bar"
#577: FILE: target/i386/translate.c:8431:
+    const CPUBreakpoint * restrict bp)

ERROR: "foo * bar" should be "foo *bar"
#615: FILE: target/i386/translate.c:8449:
+    DisasContext * restrict dc, CPUX86State * restrict env)

WARNING: line over 80 characters
#630: FILE: target/i386/translate.c:8464:
+               != ((dc->base.pc_next + TARGET_MAX_INSN_SIZE - 1) & 
TARGET_PAGE_MASK)) {

ERROR: "foo * bar" should be "foo *bar"
#643: FILE: target/i386/translate.c:8474:
+    DisasContext * restrict dc, CPUX86State * restrict env)

total: 11 errors, 1 warnings, 615 lines checked

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

Checking PATCH 6/6: target: [tcg, arm] Port to generic translation framework...
WARNING: line over 80 characters
#52: FILE: target/arm/translate-a64.c:345:
+    if (s->base.singlestep_enabled || s->ss_active || (s->base.tb->cflags & 
CF_LAST_IO)) {

WARNING: line over 80 characters
#94: FILE: target/arm/translate-a64.c:408:
+                      __FILE__, __LINE__, insn, s->base.pc_next - 4);          
    \

ERROR: externs should be avoided in .c files
#316: FILE: target/arm/translate-a64.c:11162:
+void disas_a64_insn(CPUARMState *env, DisasContext *s);

ERROR: "foo * bar" should be "foo *bar"
#355: FILE: target/arm/translate-a64.c:11214:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#446: FILE: target/arm/translate-a64.c:11269:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#454: FILE: target/arm/translate-a64.c:11274:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#476: FILE: target/arm/translate-a64.c:11279:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#483: FILE: target/arm/translate-a64.c:11286:
+    DisasContext * restrict dc, CPUArchState * restrict env,

ERROR: "foo * bar" should be "foo *bar"
#484: FILE: target/arm/translate-a64.c:11287:
+    const CPUBreakpoint * restrict bp)

ERROR: "foo * bar" should be "foo *bar"
#505: FILE: target/arm/translate-a64.c:11308:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#548: FILE: target/arm/translate-a64.c:11332:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#565: FILE: target/arm/translate-a64.c:11347:
+    DisasContext * restrict dc, CPUArchState * restrict env)

WARNING: line over 80 characters
#813: FILE: target/arm/translate.c:4039:
+           ((s->base.pc_next - 1) & TARGET_PAGE_MASK) == (dest & 
TARGET_PAGE_MASK);

ERROR: spaces required around that '=' (ctx:VxV)
#1290: FILE: target/arm/translate.c:11610:
+    for(i=0;i<16;i++) {
          ^

ERROR: space required after that ';' (ctx:VxV)
#1290: FILE: target/arm/translate.c:11610:
+    for(i=0;i<16;i++) {
            ^

ERROR: spaces required around that '<' (ctx:VxV)
#1290: FILE: target/arm/translate.c:11610:
+    for(i=0;i<16;i++) {
              ^

ERROR: space required after that ';' (ctx:VxV)
#1290: FILE: target/arm/translate.c:11610:
+    for(i=0;i<16;i++) {
                 ^

ERROR: space required before the open parenthesis '('
#1290: FILE: target/arm/translate.c:11610:
+    for(i=0;i<16;i++) {

ERROR: braces {} are necessary for all arms of this statement
#1292: FILE: target/arm/translate.c:11612:
+        if ((i % 4) == 3)
[...]
+        else
[...]

ERROR: "foo * bar" should be "foo *bar"
#1374: FILE: target/arm/translate.c:11690:
+    DisasContext * restrict dc, CPUARMState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#1434: FILE: target/arm/translate.c:11744:
+    DisasContext * restrict dc, CPUARMState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#1459: FILE: target/arm/translate.c:11757:
+    DisasContext * restrict dc, CPUARMState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#1490: FILE: target/arm/translate.c:11801:
+    DisasContext * restrict dc, CPUARMState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#1578: FILE: target/arm/translate.c:11827:
+    DisasContext * restrict dc, CPUARMState * restrict env,

ERROR: "foo * bar" should be "foo *bar"
#1579: FILE: target/arm/translate.c:11828:
+    const CPUBreakpoint * restrict bp)

ERROR: "foo * bar" should be "foo *bar"
#1603: FILE: target/arm/translate.c:11852:
+    DisasContext * restrict dc, CPUArchState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#1673: FILE: target/arm/translate.c:11897:
+    DisasContext * restrict dc, CPUARMState * restrict env)

ERROR: "foo * bar" should be "foo *bar"
#1710: FILE: target/arm/translate.c:11928:
+    DisasContext * restrict dc, CPUARMState * restrict env)

ERROR: space required before the open parenthesis '('
#1742: FILE: target/arm/translate.c:11950:
+        switch(jt) {

ERROR: spaces required around that '+' (ctx:VxV)
#1988: FILE: target/arm/translate.h:111:
+#define DJ_JUMP    (DJ_TARGET+0)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#1989: FILE: target/arm/translate.h:112:
+#define DJ_UPDATE  (DJ_TARGET+1)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#1990: FILE: target/arm/translate.h:113:
+#define DJ_TB_JUMP (DJ_TARGET+2)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#1997: FILE: target/arm/translate.h:118:
+#define DJ_WFI     (DJ_TARGET+3)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#1998: FILE: target/arm/translate.h:119:
+#define DJ_SWI     (DJ_TARGET+4)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2003: FILE: target/arm/translate.h:123:
+#define DJ_EXC     (DJ_TARGET+5)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2009: FILE: target/arm/translate.h:125:
+#define DJ_WFE     (DJ_TARGET+6)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2010: FILE: target/arm/translate.h:126:
+#define DJ_HVC     (DJ_TARGET+7)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2011: FILE: target/arm/translate.h:127:
+#define DJ_SMC     (DJ_TARGET+8)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2012: FILE: target/arm/translate.h:128:
+#define DJ_YIELD   (DJ_TARGET+9)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2013: FILE: target/arm/translate.h:129:
+#define DJ_SS      (DJ_TARGET+10)
                              ^

ERROR: spaces required around that '+' (ctx:VxV)
#2014: FILE: target/arm/translate.h:130:
+#define DJ_PAGE_CROSS (DJ_TARGET+11)
                                 ^

ERROR: spaces required around that '+' (ctx:VxV)
#2015: FILE: target/arm/translate.h:131:
+#define DJ_SKIP    (DJ_TARGET+12)
                              ^

total: 39 errors, 3 warnings, 1920 lines checked

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

=== OUTPUT END ===

Test command exited with code: 1


---
Email generated automatically by Patchew [http://patchew.org/].
Please send your feedback to address@hidden

reply via email to

[Prev in Thread] Current Thread [Next in Thread]