[Qemu-devel] [PATCH v2 00/29] Refactoring with clang-tidy

Marc-André Lureau posted 29 patches 6 years, 9 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20170713163219.9024-1-marcandre.lureau@redhat.com
Test FreeBSD passed
Test checkpatch failed
Test docker passed
Test s390x passed
include/ui/console.h        |  2 +-
linux-headers/asm-x86/kvm.h |  2 +-
block/dmg.c                 |  2 +-
block/qcow2-cluster.c       |  2 +-
block/vhdx-log.c            |  2 +-
block/vpc.c                 |  4 ++--
block/vvfat.c               |  4 ++--
hw/audio/pcspk.c            |  2 +-
hw/char/virtio-serial-bus.c |  8 ++++----
hw/display/vga.c            |  2 +-
hw/display/virtio-gpu.c     |  4 ++--
hw/i386/multiboot.c         |  3 +--
hw/net/eepro100.c           |  3 +--
hw/pci-host/piix.c          |  2 +-
hw/pci-host/q35.c           |  2 +-
hw/pci/msix.c               |  4 ++--
hw/timer/i8254_common.c     |  4 ++--
hw/usb/dev-hub.c            |  8 ++++----
hw/virtio/vhost.c           |  2 +-
libdecnumber/decNumber.c    |  2 +-
monitor.c                   |  4 ++--
target/i386/arch_dump.c     | 40 ++++++++++++++++++++--------------------
target/ppc/mem_helper.c     |  2 +-
target/ppc/translate.c      |  2 +-
tests/test-iov.c            |  3 +--
ui/cursor.c                 |  2 +-
ui/vnc-enc-tight.c          |  2 +-
ui/vnc.c                    | 10 +++++-----
28 files changed, 63 insertions(+), 66 deletions(-)
[Qemu-devel] [PATCH v2 00/29] Refactoring with clang-tidy
Posted by Marc-André Lureau 6 years, 9 months ago
Hi,

Various refactring questions on previously sent series prompted me to
look at coccinelle to automate some changes again. Alas, semantic
patches are not so easy to express for me, cocci doesn't catch all
cases, is quite slow, and it doesn't seem possible to evaluate
expressions to check if E == E-1 or if E is pow2 for example.

I started looking at clang-tidy
(http://clang.llvm.org/extra/clang-tidy/) as an alternative to do some
refactoring.

Our build-system doesn't generate compile_commands.json, which is
pretty much required to use clang refactoring tools. But it is as easy
as running "bear make" to make one, using https://github.com/rizsotto/Bear.

Then, you can run checks and automate fixes (here only "qemu-round")
over the code base with:
run-clang-tidy.py -header-filter='.*' -checks='-*,qemu-round' -fix

There are some path bugs that can easily be solved, see
https://bugs.llvm.org/show_bug.cgi?id=33440 for details.

I ran 'qemu-round' check in this series, and added a few other manual
refactoring I had pending. (I'll submit other series for the rest of
the checks later)

My WIP qemu checks are here:
https://github.com/elmarco/clang-tools-extra/tree/master/clang-tidy/qemu

The "round-check" is:
https://github.com/elmarco/clang-tools-extra/blob/master/clang-tidy/qemu/RoundCheck.cpp

I don't know if you can express a semantic patch that would be as
powerful. The code remains easy to write & read imho, and quite fast
to apply. I like the tool, it's probably a good complement to
coccinelle overall.

v2:
- rebased, and resent to CC qemu-trivial
- drop slirp patch, as Samuel said he applied it on his tree
- added r-b/a-b tags

Thanks

Marc-André Lureau (29):
  i386: use ROUND_UP macro
  vnc: use QEMU_ALIGN_DOWN
  vhdx: use QEMU_ALIGN_DOWN
  vhost: use QEMU_ALIGN_DOWN
  i8254: use QEMU_ALIGN_DOWN
  pcspk: use QEMU_ALIGN_DOWN
  dmg: use DIV_ROUND_UP
  qcow2: use DIV_ROUND_UP
  vpc: use DIV_ROUND_UP
  vvfat: use DIV_ROUND_UP
  vnc: use DIV_ROUND_UP
  ui: use DIV_ROUND_UP
  vga: use DIV_ROUND_UP
  virtio-gpu: use DIV_ROUND_UP
  monitor: use DIV_ROUND_UP
  console: use DIV_ROUND_UP
  virtio-serial: use DIV_ROUND_UP
  piix: use DIV_ROUND_UP
  q35: use DIV_ROUND_UP
  usb-hub: use DIV_ROUND_UP
  msix: use DIV_ROUND_UP
  ppc: use DIV_ROUND_UP
  i386/dump: use DIV_ROUND_UP
  kvm: use DIV_ROUND_UP
  decnumber: use DIV_ROUND_UP
  i386: introduce ELF_NOTE_SIZE macro
  i386: replace g_malloc()+memcpy() with g_memdup()
  test-iov: replace g_malloc()+memcpy() with g_memdup()
  eepro100: replace g_malloc()+memcpy() with g_memdup()

 include/ui/console.h        |  2 +-
 linux-headers/asm-x86/kvm.h |  2 +-
 block/dmg.c                 |  2 +-
 block/qcow2-cluster.c       |  2 +-
 block/vhdx-log.c            |  2 +-
 block/vpc.c                 |  4 ++--
 block/vvfat.c               |  4 ++--
 hw/audio/pcspk.c            |  2 +-
 hw/char/virtio-serial-bus.c |  8 ++++----
 hw/display/vga.c            |  2 +-
 hw/display/virtio-gpu.c     |  4 ++--
 hw/i386/multiboot.c         |  3 +--
 hw/net/eepro100.c           |  3 +--
 hw/pci-host/piix.c          |  2 +-
 hw/pci-host/q35.c           |  2 +-
 hw/pci/msix.c               |  4 ++--
 hw/timer/i8254_common.c     |  4 ++--
 hw/usb/dev-hub.c            |  8 ++++----
 hw/virtio/vhost.c           |  2 +-
 libdecnumber/decNumber.c    |  2 +-
 monitor.c                   |  4 ++--
 target/i386/arch_dump.c     | 40 ++++++++++++++++++++--------------------
 target/ppc/mem_helper.c     |  2 +-
 target/ppc/translate.c      |  2 +-
 tests/test-iov.c            |  3 +--
 ui/cursor.c                 |  2 +-
 ui/vnc-enc-tight.c          |  2 +-
 ui/vnc.c                    | 10 +++++-----
 28 files changed, 63 insertions(+), 66 deletions(-)

-- 
2.13.1.395.gf7b71de06


Re: [Qemu-devel] [PATCH v2 00/29] Refactoring with clang-tidy
Posted by no-reply@patchew.org 6 years, 9 months ago
Hi,

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

Type: series
Message-id: 20170713163219.9024-1-marcandre.lureau@redhat.com
Subject: [Qemu-devel] [PATCH v2 00/29] Refactoring with clang-tidy

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

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

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
 - [tag update]      patchew/20170713010555.6769-1-eblake@redhat.com -> patchew/20170713010555.6769-1-eblake@redhat.com
Switched to a new branch 'test'
2f7ac63 eepro100: replace g_malloc()+memcpy() with g_memdup()
3a8281d test-iov: replace g_malloc()+memcpy() with g_memdup()
3a0c841 i386: replace g_malloc()+memcpy() with g_memdup()
0b15a3c i386: introduce ELF_NOTE_SIZE macro
898ad06 decnumber: use DIV_ROUND_UP
53b41ec kvm: use DIV_ROUND_UP
b0f3ea1 i386/dump: use DIV_ROUND_UP
114596b ppc: use DIV_ROUND_UP
0bbd959 msix: use DIV_ROUND_UP
c79c77f usb-hub: use DIV_ROUND_UP
ae841af q35: use DIV_ROUND_UP
1cc1094 piix: use DIV_ROUND_UP
8994734 virtio-serial: use DIV_ROUND_UP
027155b console: use DIV_ROUND_UP
1870cdd monitor: use DIV_ROUND_UP
11b4eb8 virtio-gpu: use DIV_ROUND_UP
0e596e2 vga: use DIV_ROUND_UP
4e0dbf4 ui: use DIV_ROUND_UP
e9b6331 vnc: use DIV_ROUND_UP
821720e vvfat: use DIV_ROUND_UP
dc25074 vpc: use DIV_ROUND_UP
f157250 qcow2: use DIV_ROUND_UP
ad10be5 dmg: use DIV_ROUND_UP
4035ed7 pcspk: use QEMU_ALIGN_DOWN
e89654b i8254: use QEMU_ALIGN_DOWN
efc2b8b vhost: use QEMU_ALIGN_DOWN
ce3c657 vhdx: use QEMU_ALIGN_DOWN
7de432c vnc: use QEMU_ALIGN_DOWN
b116446 i386: use ROUND_UP macro

=== OUTPUT BEGIN ===
Checking PATCH 1/29: i386: use ROUND_UP macro...
Checking PATCH 2/29: vnc: use QEMU_ALIGN_DOWN...
Checking PATCH 3/29: vhdx: use QEMU_ALIGN_DOWN...
Checking PATCH 4/29: vhost: use QEMU_ALIGN_DOWN...
Checking PATCH 5/29: i8254: use QEMU_ALIGN_DOWN...
Checking PATCH 6/29: pcspk: use QEMU_ALIGN_DOWN...
ERROR: line over 90 characters
#24: FILE: hw/audio/pcspk.c:72:
+        s->samples = (QEMU_ALIGN_DOWN(PCSPK_BUF_LEN * PIT_FREQ, m) / (PIT_FREQ >> 1) + 1) >> 1;

total: 1 errors, 0 warnings, 8 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 7/29: dmg: use DIV_ROUND_UP...
Checking PATCH 8/29: qcow2: use DIV_ROUND_UP...
Checking PATCH 9/29: vpc: use DIV_ROUND_UP...
Checking PATCH 10/29: vvfat: use DIV_ROUND_UP...
ERROR: "(foo*)" should be "(foo *)"
#33: FILE: block/vvfat.c:2523:
+            (uint8_t*)cluster, DIV_ROUND_UP(rest_size, 0x200));

total: 1 errors, 0 warnings, 16 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 11/29: vnc: use DIV_ROUND_UP...
WARNING: line over 80 characters
#37: FILE: ui/vnc.c:2784:
+        guest_ll = pixman_image_get_width(vd->guest.fb) * (DIV_ROUND_UP(guest_bpp, 8));

total: 0 errors, 1 warnings, 16 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 12/29: ui: use DIV_ROUND_UP...
Checking PATCH 13/29: vga: use DIV_ROUND_UP...
Checking PATCH 14/29: virtio-gpu: use DIV_ROUND_UP...
Checking PATCH 15/29: monitor: use DIV_ROUND_UP...
Checking PATCH 16/29: console: use DIV_ROUND_UP...
Checking PATCH 17/29: virtio-serial: use DIV_ROUND_UP...
WARNING: line over 80 characters
#51: FILE: hw/char/virtio-serial-bus.c:1078:
+    vser->ports_map = g_malloc0((DIV_ROUND_UP(vser->serial.max_virtserial_ports, 32))

total: 0 errors, 1 warnings, 32 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 18/29: piix: use DIV_ROUND_UP...
Checking PATCH 19/29: q35: use DIV_ROUND_UP...
Checking PATCH 20/29: usb-hub: use DIV_ROUND_UP...
Checking PATCH 21/29: msix: use DIV_ROUND_UP...
Checking PATCH 22/29: ppc: use DIV_ROUND_UP...
Checking PATCH 23/29: i386/dump: use DIV_ROUND_UP...
WARNING: line over 80 characters
#26: FILE: target/i386/arch_dump.c:80:
+    note_size = (DIV_ROUND_UP(sizeof(Elf64_Nhdr), 4) + DIV_ROUND_UP(name_size, 4) +

WARNING: line over 80 characters
#37: FILE: target/i386/arch_dump.c:159:
+    note_size = (DIV_ROUND_UP(sizeof(Elf64_Nhdr), 4) + DIV_ROUND_UP(name_size, 4) +

WARNING: line over 80 characters
#48: FILE: target/i386/arch_dump.c:214:
+    note_size = (DIV_ROUND_UP(sizeof(Elf32_Nhdr), 4) + DIV_ROUND_UP(name_size, 4) +

WARNING: line over 80 characters
#72: FILE: target/i386/arch_dump.c:446:
+    elf_note_size = (DIV_ROUND_UP(note_head_size, 4) + DIV_ROUND_UP(name_size, 4) +

WARNING: line over 80 characters
#74: FILE: target/i386/arch_dump.c:448:
+    qemu_note_size = (DIV_ROUND_UP(note_head_size, 4) + DIV_ROUND_UP(name_size, 4) +

total: 0 errors, 5 warnings, 54 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 24/29: kvm: use DIV_ROUND_UP...
Checking PATCH 25/29: decnumber: use DIV_ROUND_UP...
ERROR: code indent should never use tabs
#24: FILE: libdecnumber/decNumber.c:4778:
+^I    *(up-1)+=DIV_ROUND_UP(DECDPUNMAX, 2);$

ERROR: spaces required around that '-' (ctx:VxV)
#24: FILE: libdecnumber/decNumber.c:4778:
+	    *(up-1)+=DIV_ROUND_UP(DECDPUNMAX, 2);
 	        ^

ERROR: spaces required around that '+=' (ctx:VxV)
#24: FILE: libdecnumber/decNumber.c:4778:
+	    *(up-1)+=DIV_ROUND_UP(DECDPUNMAX, 2);
 	           ^

total: 3 errors, 0 warnings, 8 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 26/29: i386: introduce ELF_NOTE_SIZE macro...
Checking PATCH 27/29: i386: replace g_malloc()+memcpy() with g_memdup()...
Checking PATCH 28/29: test-iov: replace g_malloc()+memcpy() with g_memdup()...
Checking PATCH 29/29: eepro100: replace g_malloc()+memcpy() with g_memdup()...
=== OUTPUT END ===

Test command exited with code: 1


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