Re: [Qemu-devel] [PATCH v2 0/6] hypertrace: Lightweight guest-to-QEMU trace channel

[no-reply]

Hi,

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

Subject: [Qemu-devel] [PATCH v2 0/6] hypertrace: Lightweight guest-to-QEMU 
trace channel
Type: series
Message-id: address@hidden

=== 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 show --no-patch --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
 * [new tag]         patchew/address@hidden -> patchew/address@hidden
 * [new tag]         patchew/address@hidden -> patchew/address@hidden
Switched to a new branch 'test'
0b7ef18 hypertrace: Add guest-side Linux module
1c66cbd hypertrace: Add guest-side user-level library
b67ef31 hypertrace: [softmmu] Add QEMU-side proxy to "guest_hypertrace" event
536c66c hypertrace: [*-user] Add QEMU-side proxy to "guest_hypertrace" event
8861ce2 hypertrace: Add tracing event "guest_hypertrace"
bc68e0b hypertrace: Add documentation

=== OUTPUT BEGIN ===
Checking PATCH 1/6: hypertrace: Add documentation...
ERROR: trailing whitespace
#109: FILE: docs/hypertrace.txt:92:
+    $

ERROR: trailing whitespace
#110: FILE: docs/hypertrace.txt:93:
+    $

ERROR: trailing whitespace
#117: FILE: docs/hypertrace.txt:100:
+    $

ERROR: trailing whitespace
#123: FILE: docs/hypertrace.txt:106:
+    $

ERROR: trailing whitespace
#130: FILE: docs/hypertrace.txt:113:
+    $

ERROR: trailing whitespace
#154: FILE: docs/hypertrace.txt:137:
+    $

ERROR: trailing whitespace
#156: FILE: docs/hypertrace.txt:139:
+    $

ERROR: trailing whitespace
#160: FILE: docs/hypertrace.txt:143:
+    $

ERROR: trailing whitespace
#171: FILE: docs/hypertrace.txt:154:
+    $

ERROR: trailing whitespace
#181: FILE: docs/hypertrace.txt:164:
+    $

total: 10 errors, 0 warnings, 236 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: hypertrace: Add tracing event "guest_hypertrace"...
Checking PATCH 3/6: hypertrace: [*-user] Add QEMU-side proxy to 
"guest_hypertrace" event...
WARNING: line over 80 characters
#81: FILE: bsd-user/main.c:866:
+            hypertrace_opt_parse(optarg, &hypertrace_base, 
&hypertrace_max_clients);

WARNING: line over 80 characters
#176: FILE: bsd-user/syscall.c:375:
+                                        target_to_host_bitmask(arg4, 
mmap_flags_tbl),

WARNING: line over 80 characters
#200: FILE: bsd-user/syscall.c:463:
+                                        target_to_host_bitmask(arg4, 
mmap_flags_tbl),

WARNING: line over 80 characters
#224: FILE: bsd-user/syscall.c:540:
+                                        target_to_host_bitmask(arg4, 
mmap_flags_tbl),

ERROR: architecture specific defines should be avoided
#302: FILE: hypertrace/common.h:10:
+#if !defined(__KERNEL__)

ERROR: open brace '{' following struct go on the same line
#307: FILE: hypertrace/common.h:15:
+struct hypertrace_config
+{

ERROR: do not initialise statics to 0 or NULL
#370: FILE: hypertrace/user.c:48:
+static char *config_path = NULL;

ERROR: do not initialise statics to 0 or NULL
#372: FILE: hypertrace/user.c:50:
+static uint64_t *qemu_config = NULL;

ERROR: do not initialise statics to 0 or NULL
#374: FILE: hypertrace/user.c:52:
+static char *data_path = NULL;

ERROR: do not initialise statics to 0 or NULL
#376: FILE: hypertrace/user.c:54:
+static uint64_t *qemu_data = NULL;

ERROR: do not initialise statics to 0 or NULL
#378: FILE: hypertrace/user.c:56:
+static char *control_path = NULL;

ERROR: do not initialise statics to 0 or NULL
#380: FILE: hypertrace/user.c:58:
+static uint64_t *qemu_control = NULL;

WARNING: line over 80 characters
#405: FILE: hypertrace/user.c:83:
+void hypertrace_opt_parse(const char *optarg, char **base, unsigned int 
*max_clients_)

ERROR: "foo ** bar" should be "foo **bar"
#427: FILE: hypertrace/user.c:105:
+                         char ** path, int *fd, uint64_t **addr)

ERROR: line over 90 characters
#490: FILE: hypertrace/user.c:168:
+    init_channel(base, "-config", TARGET_PAGE_SIZE, &config_path, &config_fd, 
&qemu_config);

ERROR: "(foo*)" should be "(foo *)"
#491: FILE: hypertrace/user.c:169:
+    pconfig = (struct hypertrace_config*)qemu_config;

WARNING: line over 80 characters
#498: FILE: hypertrace/user.c:176:
+    init_channel(base, "-data", config.data_size, &data_path, &data_fd, 
&qemu_data);

ERROR: line over 90 characters
#506: FILE: hypertrace/user.c:184:
+    init_channel(base, "-control", config.control_size, &control_path, 
&control_fd, &qemu_control);

ERROR: do not initialise statics to 0 or NULL
#549: FILE: hypertrace/user.c:227:
+    static bool atexit_in = false;

WARNING: line over 80 characters
#565: FILE: hypertrace/user.c:243:
+bool hypertrace_guest_mmap_check(int fd, unsigned long len, unsigned long 
offset)

WARNING: line over 80 characters
#600: FILE: hypertrace/user.c:278:
+    printf("cpu->hypertrace_control=%p %lu %lu\n", qemu_addr, s.st_dev, 
s.st_ino);

WARNING: line over 80 characters
#602: FILE: hypertrace/user.c:280:
+    if (mprotect(vcpu->hypertrace_control, config.control_size / 2, PROT_READ) 
== -1) {

WARNING: line over 80 characters
#603: FILE: hypertrace/user.c:281:
+        error_report("error: mprotect(hypertrace_control): %s", 
strerror(errno));

ERROR: "(foo*)" should be "(foo *)"
#641: FILE: hypertrace/user.c:319:
+        uint64_t vcontrol = ((uint64_t*)control_0)[client];

WARNING: line over 80 characters
#694: FILE: hypertrace/user.h:27:
+void hypertrace_opt_parse(const char *optarg, char **base, unsigned int 
*max_clients);

WARNING: line over 80 characters
#712: FILE: hypertrace/user.h:45:
+bool hypertrace_guest_mmap_check(int fd, unsigned long len, unsigned long 
offset);

total: 14 errors, 12 warnings, 833 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: hypertrace: [softmmu] Add QEMU-side proxy to 
"guest_hypertrace" event...
ERROR: open brace '{' following struct go on the same line
#81: FILE: hypertrace/softmmu.c:43:
+typedef struct HypertraceState
+{

ERROR: "(foo*)" should be "(foo *)"
#103: FILE: hypertrace/softmmu.c:65:
+    char *mem = &((char*)s->control_ptr)[addr];

ERROR: "(foo*)" should be "(foo *)"
#108: FILE: hypertrace/softmmu.c:70:
+        res = ((uint8_t*)mem)[0];

ERROR: "(foo*)" should be "(foo *)"
#113: FILE: hypertrace/softmmu.c:75:
+        res = ((uint16_t*)mem)[0];

ERROR: "(foo*)" should be "(foo *)"
#118: FILE: hypertrace/softmmu.c:80:
+        res = ((uint32_t*)mem)[0];

ERROR: "(foo*)" should be "(foo *)"
#123: FILE: hypertrace/softmmu.c:85:
+        res = ((uint64_t*)mem)[0];

ERROR: Error messages should not contain newlines
#127: FILE: hypertrace/softmmu.c:89:
+        error_report("error: hypertrace: Unexpected read of size %d\n", size);

ERROR: "(foo*)" should be "(foo *)"
#139: FILE: hypertrace/softmmu.c:101:
+    char *mem = &((char*)s->control_ptr)[addr];

ERROR: "(foo*)" should be "(foo *)"
#144: FILE: hypertrace/softmmu.c:106:
+        uint8_t *res = (uint8_t*)mem;

ERROR: "(foo*)" should be "(foo *)"
#150: FILE: hypertrace/softmmu.c:112:
+        uint16_t *res = (uint16_t*)mem;

ERROR: "(foo*)" should be "(foo *)"
#156: FILE: hypertrace/softmmu.c:118:
+        uint32_t *res = (uint32_t*)mem;

ERROR: "(foo*)" should be "(foo *)"
#162: FILE: hypertrace/softmmu.c:124:
+        uint64_t *res = (uint64_t*)mem;

ERROR: Error messages should not contain newlines
#167: FILE: hypertrace/softmmu.c:129:
+        error_report("error: hypertrace: Unexpected write of size %d\n", size);

ERROR: "(foo*)" should be "(foo *)"
#172: FILE: hypertrace/softmmu.c:134:
+        uint64_t vcontrol = ((uint64_t*)s->control_ptr)[client];

ERROR: "(foo*)" should be "(foo *)"
#173: FILE: hypertrace/softmmu.c:135:
+        uint64_t *data_ptr = (uint64_t*)s->data_ptr;

ERROR: Error messages should not contain newlines
#197: FILE: hypertrace/softmmu.c:159:
+        error_setg(errp, "hypertrace: must have at least one client\n");

WARNING: line over 80 characters
#203: FILE: hypertrace/softmmu.c:165:
+    error_setg(&s->migration_blocker, "The 'hypertrace' device cannot be 
migrated");

total: 16 errors, 1 warnings, 256 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: hypertrace: Add guest-side user-level library...
ERROR: do not initialise statics to 0 or NULL
#123: FILE: hypertrace/guest/user/common.c:35:
+static char *config_path = NULL;

ERROR: do not initialise statics to 0 or NULL
#125: FILE: hypertrace/guest/user/common.c:37:
+static uint64_t *config_addr = NULL;

ERROR: do not initialise statics to 0 or NULL
#126: FILE: hypertrace/guest/user/common.c:38:
+static struct hypertrace_config *config = NULL;

ERROR: do not initialise statics to 0 or NULL
#129: FILE: hypertrace/guest/user/common.c:41:
+static char *data_path = NULL;

ERROR: do not initialise statics to 0 or NULL
#131: FILE: hypertrace/guest/user/common.c:43:
+static uint64_t *data_addr = NULL;

ERROR: do not initialise statics to 0 or NULL
#133: FILE: hypertrace/guest/user/common.c:45:
+static char *control_path = NULL;

ERROR: do not initialise statics to 0 or NULL
#136: FILE: hypertrace/guest/user/common.c:48:
+static __thread uint64_t *control_addr = NULL;

ERROR: do not initialise statics to 0 or NULL
#137: FILE: hypertrace/guest/user/common.c:49:
+static __thread uint64_t *control_addr_1 = NULL;

ERROR: do not initialise statics to 0 or NULL
#139: FILE: hypertrace/guest/user/common.c:51:
+static uint64_t *control_addr = NULL;

ERROR: "foo ** bar" should be "foo **bar"
#144: FILE: hypertrace/guest/user/common.c:56:
+                             char ** path, int *fd, uint64_t **addr, bool 
write)

ERROR: architecture specific defines should be avoided
#172: FILE: hypertrace/guest/user/common.c:84:
+#if !defined(CONFIG_USER_ONLY) && defined(__linux__)

ERROR: space prohibited between function name and open parenthesis '('
#173: FILE: hypertrace/guest/user/common.c:85:
+static int check_device_id (const char *base, const char *name, uint64_t value)

ERROR: consider using qemu_strtoull in preference to strtoull
#192: FILE: hypertrace/guest/user/common.c:104:
+    uint64_t vv = strtoull(v, &end, 16);

ERROR: else should follow close brace '}'
#196: FILE: hypertrace/guest/user/common.c:108:
+    }
+    else {

ERROR: "foo* bar" should be "foo *bar"
#201: FILE: hypertrace/guest/user/common.c:113:
+static char* find_device(void)

WARNING: line over 80 characters
#216: FILE: hypertrace/guest/user/common.c:128:
+        if (check_device_id(path, "vendor", PCI_VENDOR_ID_REDHAT_QUMRANET) < 
0) {

ERROR: "(foo*)" should be "(foo *)"
#279: FILE: hypertrace/guest/user/common.c:191:
+        config = (struct hypertrace_config*)config_addr;

WARNING: line over 80 characters
#323: FILE: hypertrace/guest/user/common.c:235:
+        control_addr_1 = (uint64_t*)((char*)control_addr + 
config->control_size / 2);

ERROR: "(foo*)" should be "(foo *)"
#323: FILE: hypertrace/guest/user/common.c:235:
+        control_addr_1 = (uint64_t*)((char*)control_addr + 
config->control_size / 2);

ERROR: space prohibited between function name and open parenthesis '('
#380: FILE: hypertrace/guest/user/common.c:292:
+void qemu_hypertrace (uint64_t client, uint64_t arg1)

total: 18 errors, 2 warnings, 433 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: hypertrace: Add guest-side Linux module...
ERROR: space prohibited between function name and open parenthesis '('
#135: FILE: 
hypertrace/guest/linux-module/include/linux/qemu-hypertrace-internal.h:42:
+static inline void qemu_hypertrace (uint64_t client, uint64_t arg1)

ERROR: do not use C99 // comments
#266: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:42:
+//////////////////////////////////////////////////////////////////////

ERROR: do not use C99 // comments
#267: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:43:
+// Kernel interface

ERROR: do not use C99 // comments
#281: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:57:
+//////////////////////////////////////////////////////////////////////

ERROR: do not use C99 // comments
#282: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:58:
+// Channel initialization

ERROR: space prohibited between function name and open parenthesis '('
#286: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:62:
+init_channel (uint64_t **vaddr, struct pci_dev *dev, int bar)

ERROR: "foo * bar" should be "foo *bar"
#288: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:64:
+    void * res;

ERROR: do not use C99 // comments
#307: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:83:
+//////////////////////////////////////////////////////////////////////

ERROR: do not use C99 // comments
#308: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:84:
+// Module (de)initialization

WARNING: line over 80 characters
#319: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:95:
+    dev = pci_get_device(PCI_VENDOR_ID_REDHAT_QUMRANET, 
PCI_DEVICE_ID_HYPERTRACE, NULL);

ERROR: "(foo*)" should be "(foo *)"
#332: FILE: hypertrace/guest/linux-module/qemu-hypertrace.c:108:
+    config = (struct hypertrace_config*)_qemu_hypertrace_channel_config;

total: 10 errors, 1 warnings, 316 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