qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
From: no-reply@patchew.org
To: slp@redhat.com
Cc: ehabkost@redhat.com, slp@redhat.com, mst@redhat.com,
	philmd@redhat.com, qemu-devel@nongnu.org, kraxel@redhat.com,
	imammedo@redhat.com, pbonzini@redhat.com, rth@twiddle.net,
	lersek@redhat.com, sgarzare@redhat.com
Subject: Re: [PATCH v6 00/10] Introduce the microvm machine type
Date: Fri, 4 Oct 2019 10:22:17 -0700 (PDT)	[thread overview]
Message-ID: <157020973522.31166.10377462722795249056@8230166b0665> (raw)
In-Reply-To: <20191004093752.16564-1-slp@redhat.com>

Patchew URL: https://patchew.org/QEMU/20191004093752.16564-1-slp@redhat.com/



Hi,

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

Type: series
Message-id: 20191004093752.16564-1-slp@redhat.com
Subject: [PATCH v6 00/10] Introduce the microvm machine type

=== TEST SCRIPT BEGIN ===
#!/bin/bash
git rev-parse base > /dev/null || exit 0
git config --local diff.renamelimit 0
git config --local diff.renames True
git config --local diff.algorithm histogram
./scripts/checkpatch.pl --mailback base..
=== TEST SCRIPT END ===

Updating 3c8cf5a9c21ff8782164d1def7f44bd888713384
From https://github.com/patchew-project/qemu
 * [new tag]         patchew/20191004171204.21040-1-eric.auger@redhat.com -> patchew/20191004171204.21040-1-eric.auger@redhat.com
Switched to a new branch 'test'
82de93f hw/i386: Introduce the microvm machine type
fda0032 docs/microvm.rst: document the new microvm machine type
8dc483d roms: add microvm-bios (qboot) as binary and git submodule
16f12bc hw/intc/apic: reject pic ints if isa_pic == NULL
22f8cab fw_cfg: add "modify" functions for all types
7cdaa3f hw/i386: make x86.c independent from PCMachineState
052084d hw/i386: split PCMachineState deriving X86MachineState from it
afc0d5a hw/i386/pc: move shared x86 functions to x86.c and export them
9c1dc683 hw/i386/pc: rename functions shared with non-PC machines
bd6947a hw/virtio: Factorize virtio-mmio headers

=== OUTPUT BEGIN ===
1/10 Checking commit bd6947a2e366 (hw/virtio: Factorize virtio-mmio headers)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#77: 
new file mode 100644

total: 0 errors, 1 warnings, 131 lines checked

Patch 1/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
2/10 Checking commit 9c1dc683f829 (hw/i386/pc: rename functions shared with non-PC machines)
3/10 Checking commit afc0d5a54977 (hw/i386/pc: move shared x86 functions to x86.c and export them)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#749: 
new file mode 100644

WARNING: Block comments use a leading /* on a separate line
#809: FILE: hw/i386/x86.c:56:
+/* Calculates initial APIC ID for a specific CPU index

WARNING: Block comments use a leading /* on a separate line
#866: FILE: hw/i386/x86.c:113:
+    /* Calculates the limit to CPU APIC ID values

WARNING: Block comments should align the * on each line
#913: FILE: hw/i386/x86.c:160:
+         * -smp hasn't been parsed after it
+        */

WARNING: line over 80 characters
#926: FILE: hw/i386/x86.c:173:
+        ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(pcms, i);

ERROR: spaces required around that '+' (ctx:VxV)
#1087: FILE: hw/i386/x86.c:334:
+    cmdline_size = (strlen(kernel_cmdline)+16) & ~15;
                                           ^

ERROR: do not use assignment in if condition
#1091: FILE: hw/i386/x86.c:338:
+    if (!f || !(kernel_size = get_file_size(f)) ||

ERROR: if this code is redundant consider removing it
#1100: FILE: hw/i386/x86.c:347:
+#if 0

ERROR: spaces required around that '+' (ctx:VxV)
#1101: FILE: hw/i386/x86.c:348:
+    fprintf(stderr, "header magic: %#x\n", ldl_p(header+0x202));
                                                        ^

ERROR: spaces required around that '+' (ctx:VxV)
#1103: FILE: hw/i386/x86.c:350:
+    if (ldl_p(header+0x202) == 0x53726448) {
                     ^

ERROR: spaces required around that '+' (ctx:VxV)
#1104: FILE: hw/i386/x86.c:351:
+        protocol = lduw_p(header+0x206);
                                 ^

ERROR: if this code is redundant consider removing it
#1194: FILE: hw/i386/x86.c:441:
+#if 0

ERROR: spaces required around that '+' (ctx:VxV)
#1206: FILE: hw/i386/x86.c:453:
+        lduw_p(header+0x236) & XLF_CAN_BE_LOADED_ABOVE_4G) {
                      ^

ERROR: spaces required around that '+' (ctx:VxV)
#1225: FILE: hw/i386/x86.c:472:
+        initrd_max = ldl_p(header+0x22c);
                                  ^

ERROR: spaces required around that '+' (ctx:VxV)
#1235: FILE: hw/i386/x86.c:482:
+    fw_cfg_add_i32(fw_cfg, FW_CFG_CMDLINE_SIZE, strlen(kernel_cmdline)+1);
                                                                       ^

ERROR: spaces required around that '+' (ctx:VxV)
#1239: FILE: hw/i386/x86.c:486:
+        stl_p(header+0x228, cmdline_addr);
                     ^

ERROR: spaces required around that '+' (ctx:VxV)
#1241: FILE: hw/i386/x86.c:488:
+        stw_p(header+0x20, 0xA33F);
                     ^

ERROR: spaces required around that '+' (ctx:VxV)
#1242: FILE: hw/i386/x86.c:489:
+        stw_p(header+0x22, cmdline_addr-real_addr);
                     ^

ERROR: spaces required around that '-' (ctx:VxV)
#1242: FILE: hw/i386/x86.c:489:
+        stw_p(header+0x22, cmdline_addr-real_addr);
                                        ^

ERROR: consider using qemu_strtol in preference to strtol
#1258: FILE: hw/i386/x86.c:505:
+            video_mode = strtol(vmode, NULL, 0);

ERROR: spaces required around that '+' (ctx:VxV)
#1260: FILE: hw/i386/x86.c:507:
+        stw_p(header+0x1fa, video_mode);
                     ^

WARNING: Block comments use a leading /* on a separate line
#1264: FILE: hw/i386/x86.c:511:
+    /* High nybble = B reserved for QEMU; low nybble is revision number.

WARNING: Block comments use * on subsequent lines
#1265: FILE: hw/i386/x86.c:512:
+    /* High nybble = B reserved for QEMU; low nybble is revision number.
+       If this code is substantially changed, you may want to consider

WARNING: Block comments use a trailing */ on a separate line
#1266: FILE: hw/i386/x86.c:513:
+       incrementing the revision. */

ERROR: code indent should never use tabs
#1272: FILE: hw/i386/x86.c:519:
+        header[0x211] |= 0x80;^I/* CAN_USE_HEAP */$

ERROR: spaces required around that '+' (ctx:VxV)
#1273: FILE: hw/i386/x86.c:520:
+        stw_p(header+0x224, cmdline_addr-real_addr-0x200);
                     ^

ERROR: spaces required around that '-' (ctx:VxV)
#1273: FILE: hw/i386/x86.c:520:
+        stw_p(header+0x224, cmdline_addr-real_addr-0x200);
                                         ^

ERROR: spaces required around that '-' (ctx:VxV)
#1273: FILE: hw/i386/x86.c:520:
+        stw_p(header+0x224, cmdline_addr-real_addr-0x200);
                                                   ^

ERROR: spaces required around that '-' (ctx:VxV)
#1305: FILE: hw/i386/x86.c:552:
+        initrd_addr = (initrd_max-initrd_size) & ~4095;
                                  ^

ERROR: spaces required around that '+' (ctx:VxV)
#1311: FILE: hw/i386/x86.c:558:
+        stl_p(header+0x218, initrd_addr);
                     ^

ERROR: spaces required around that '+' (ctx:VxV)
#1312: FILE: hw/i386/x86.c:559:
+        stl_p(header+0x21c, initrd_size);
                     ^

ERROR: spaces required around that '+' (ctx:VxV)
#1320: FILE: hw/i386/x86.c:567:
+    setup_size = (setup_size+1)*512;
                             ^

ERROR: spaces required around that '*' (ctx:VxV)
#1320: FILE: hw/i386/x86.c:567:
+    setup_size = (setup_size+1)*512;
                                ^

ERROR: spaces required around that '+' (ctx:VxV)
#1358: FILE: hw/i386/x86.c:605:
+        stq_p(header+0x250, prot_addr + setup_data_offset);
                     ^

total: 26 errors, 8 warnings, 1430 lines checked

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

4/10 Checking commit 052084dca6ed (hw/i386: split PCMachineState deriving X86MachineState from it)
WARNING: Block comments use a leading /* on a separate line
#880: FILE: hw/i386/pc_q35.c:158:
+        x86ms->max_ram_below_4g = 1ULL << 32; /* default: 4G */;

WARNING: line over 80 characters
#1103: FILE: hw/i386/x86.c:420:
+                initrd_max = x86ms->below_4g_mem_size - pcmc->acpi_data_size - 1;

WARNING: line over 80 characters
#1232: FILE: hw/i386/xen/xen-hvm.c:204:
+                                                    X86_MACHINE_MAX_RAM_BELOW_4G,

WARNING: Block comments use a leading /* on a separate line
#1435: FILE: include/hw/i386/x86.h:61:
+    /* Address space used by IOAPIC device. All IOAPIC interrupts

WARNING: Block comments use a trailing */ on a separate line
#1436: FILE: include/hw/i386/x86.h:62:
+     * will be translated to MSI messages in the address space. */

total: 0 errors, 5 warnings, 1296 lines checked

Patch 4/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
5/10 Checking commit 7cdaa3f2e445 (hw/i386: make x86.c independent from PCMachineState)
WARNING: line over 80 characters
#178: FILE: hw/i386/x86.c:173:
+        ms->possible_cpus->cpus[i].arch_id = x86_cpu_apic_id_from_index(x86ms, i);

total: 0 errors, 1 warnings, 220 lines checked

Patch 5/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
6/10 Checking commit 22f8cab11248 (fw_cfg: add "modify" functions for all types)
7/10 Checking commit 16f12bca2dce (hw/intc/apic: reject pic ints if isa_pic == NULL)
8/10 Checking commit 8dc483dafc50 (roms: add microvm-bios (qboot) as binary and git submodule)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#29: 
new file mode 100755

total: 0 errors, 1 warnings, 28 lines checked

Patch 8/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
9/10 Checking commit fda00320f641 (docs/microvm.rst: document the new microvm machine type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#13: 
new file mode 100644

total: 0 errors, 1 warnings, 98 lines checked

Patch 9/10 has style problems, please review.  If any of these errors
are false positives report them to the maintainer, see
CHECKPATCH in MAINTAINERS.
10/10 Checking commit 82de93f5898c (hw/i386: Introduce the microvm machine type)
WARNING: added, moved or deleted file(s), does MAINTAINERS need updating?
#55: 
new file mode 100644

total: 0 errors, 1 warnings, 678 lines checked

Patch 10/10 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


The full log is available at
http://patchew.org/logs/20191004093752.16564-1-slp@redhat.com/testing.checkpatch/?type=message.
---
Email generated automatically by Patchew [https://patchew.org/].
Please send your feedback to patchew-devel@redhat.com

  parent reply	other threads:[~2019-10-04 17:23 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-10-04  9:37 [PATCH v6 00/10] Introduce the microvm machine type Sergio Lopez
2019-10-04  9:37 ` [PATCH v6 01/10] hw/virtio: Factorize virtio-mmio headers Sergio Lopez
2019-10-04  9:43   ` Philippe Mathieu-Daudé
2019-10-04  9:37 ` [PATCH v6 02/10] hw/i386/pc: rename functions shared with non-PC machines Sergio Lopez
2019-10-04  9:46   ` Philippe Mathieu-Daudé
2019-10-04 11:24   ` Stefano Garzarella
2019-10-04  9:37 ` [PATCH v6 03/10] hw/i386/pc: move shared x86 functions to x86.c and export them Sergio Lopez
2019-10-04  9:46   ` Philippe Mathieu-Daudé
2019-10-04 11:23   ` Stefano Garzarella
2019-10-04 11:36   ` Stefano Garzarella
2019-10-07 13:46     ` Sergio Lopez
2019-10-04  9:37 ` [PATCH v6 04/10] hw/i386: split PCMachineState deriving X86MachineState from it Sergio Lopez
2019-10-04  9:49   ` Philippe Mathieu-Daudé
2019-10-04  9:37 ` [PATCH v6 05/10] hw/i386: make x86.c independent from PCMachineState Sergio Lopez
2019-10-04  9:51   ` Philippe Mathieu-Daudé
2019-10-04  9:37 ` [PATCH v6 06/10] fw_cfg: add "modify" functions for all types Sergio Lopez
2019-10-04  9:37 ` [PATCH v6 07/10] hw/intc/apic: reject pic ints if isa_pic == NULL Sergio Lopez
2019-10-04  9:37 ` [PATCH v6 08/10] roms: add microvm-bios (qboot) as binary and git submodule Sergio Lopez
2019-10-04 11:50   ` Stefano Garzarella
2019-10-04  9:37 ` [PATCH v6 09/10] docs/microvm.rst: document the new microvm machine type Sergio Lopez
2019-10-04  9:37 ` [PATCH v6 10/10] hw/i386: Introduce the " Sergio Lopez
2019-10-04 13:57 ` [PATCH v6 00/10] " Michael S. Tsirkin
2019-10-04 17:21 ` no-reply
2019-10-08 12:37   ` Paolo Bonzini
2019-10-08 13:14     ` Sergio Lopez
2019-10-04 17:22 ` no-reply [this message]
2019-10-05 22:08 ` Michael S. Tsirkin
2019-10-07 13:44   ` Sergio Lopez
2019-10-07 13:59     ` Philippe Mathieu-Daudé
2019-10-09 19:21     ` Michael S. Tsirkin
2019-10-09 20:52       ` Eduardo Habkost

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=157020973522.31166.10377462722795249056@8230166b0665 \
    --to=no-reply@patchew.org \
    --cc=ehabkost@redhat.com \
    --cc=imammedo@redhat.com \
    --cc=kraxel@redhat.com \
    --cc=lersek@redhat.com \
    --cc=mst@redhat.com \
    --cc=pbonzini@redhat.com \
    --cc=philmd@redhat.com \
    --cc=qemu-devel@nongnu.org \
    --cc=rth@twiddle.net \
    --cc=sgarzare@redhat.com \
    --cc=slp@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).