Historically QEMU has relied on the checkpatch.pl script,
borrowed from Linux, to check coding style compliance on
patches which are submitted. For what it is designed for,
it does a reasonable job, but I feel that QEMU would benefit
from some more checking in this area, in particular checks
that run across the entire repository, not just new patches.
Rather than attempt to replace checkpatch.pl, this series
illustrates how we can augment our existing style compliance
checking. This imports the infrastructure from GNULIB which
provides a 'syntax-check' target in the makefiles.
By default the checks are run against all files which are
committed to GIT, as identified by the 'vc-list-files'
script. On a per-check basis you can define a list of file
exclusions, so you can skip checks in places where there
are valid reasons for the exceptional code style. It is
also possible to set up rules so that they can be skipped
on a per-line basis with magic comments in the source
though this is not illustrated in this series.
Each style rule is provided by a separate make target named
with an 'sc_' prefix. There are some boilerplate make rules
provided to simplify the process of defining new checks.
The maint.mk file provides the set of rules defined by the
GNULIB project and the cfg.mk file is intended to contain
any further rules desired by QEMU. That is not to say that
QEMU must use all the rules defined by GNULIB - it is
possible to turn each rule on/off globally as desired.
In this series the first patch sets up the infrastructure
such that any developer can do
make syntax-check
to see violations in their local tree. The following patches
then incrementally enable some of the rules and fix up the
violations they detect. None of the things I've detected
are particularly interesting/troublesome until I get to
the second to last patch where I look for non-reentrant
safe POSIX function usage. There are some areas identified
that I think have potential lead to data corruption / crashes
if we're unlucky with usage across threads.
The very last patch illustrates how we would mandate that
inclusion of 'osdep.h' as the first header in all .c files
Although I've enabled it, I didn't try to fix the .c files
so 'make syntax-check' will fail with patch 10 applied. It
should however pass for all patches upto #9
There are plenty more rules that could be enabled and there
is obviously scope to right more custom rules.
The intent would be that developers run 'make syntax-check'
before sending patches to check rules. It is also wired
into 'make check' to make it harder to forget. It would
also be expected that maintainers would run syntax-check
on any patches they receive from contributors and reject
them if failing, in the same way they'd be rejected if
checkpatch.pl fails. Finally the automated build systems
that various people are running should also run the
'make syntax-check' if they are not already running the
'make check' rule.
Daniel P. Berrange (10):
tests: import GNULIB's syntax-check infrastructure
maint: remove double semicolons in many files
maint: remove / fix many doubled words
maint: remove unused include for assert.h
maint: remove unused include for dirent.h
maint: remove unused include for signal.h
maint: remove unused include for strings.h
maint: avoid useless "if (foo) free(foo)" pattern
maint: add check for use of POSIX functions which are not reentrant
safe
maint: enable checking for qemu/osdep.h header usage
Makefile | 5 +
Makefile.nonreentrant | 120 ++++
backends/hostmem-file.c | 4 +-
block/qcow2-cluster.c | 4 +-
block/qcow2-refcount.c | 2 +-
block/vhdx.c | 2 +-
bsd-user/elfload.c | 4 +-
bsd-user/signal.c | 1 -
cfg.mk | 156 +++++
disas/ia64.c | 1 -
disas/microblaze.c | 1 -
disas/sparc.c | 3 +-
docs/libcacard.txt | 4 +-
docs/multiseat.txt | 2 +-
docs/specs/qcow2.txt | 2 +-
docs/specs/rocker.txt | 2 +-
fsdev/virtio-9p-marshal.c | 1 -
hw/arm/vexpress.c | 4 +-
hw/block/xen_disk.c | 1 -
hw/bt/hci.c | 9 +-
hw/core/loader.c | 3 +-
hw/core/qdev-properties.c | 4 +-
hw/display/exynos4210_fimd.c | 4 +-
hw/i386/xen/xen_platform.c | 2 -
hw/intc/arm_gic.c | 2 +-
hw/mips/mips_r4k.c | 4 +-
hw/net/fsl_etsec/rings.c | 4 +-
hw/net/rocker/rocker.c | 4 +-
hw/net/rocker/rocker_desc.c | 8 +-
hw/net/rtl8139.c | 2 +-
hw/net/xen_nic.c | 1 -
hw/nvram/fw_cfg.c | 4 +-
hw/pci-host/prep.c | 4 +-
hw/pci/shpc.c | 1 -
hw/sd/sd.c | 3 +-
hw/tpm/tpm_passthrough.c | 2 -
hw/usb/hcd-xhci.c | 4 +-
hw/usb/host-libusb.c | 2 +-
hw/usb/redirect.c | 2 -
hw/vfio/common.c | 2 +-
hw/vfio/pci.c | 1 -
hw/xen/xen_pt_config_init.c | 4 +-
include/block/block.h | 2 +-
include/exec/memory.h | 2 +-
linux-user/elfload.c | 2 +-
linux-user/signal.c | 1 -
maint.mk | 1228 ++++++++++++++++++++++++++++++++++++++++
migration/rdma.c | 2 +-
migration/savevm.c | 8 +-
numa.c | 2 +-
os-win32.c | 1 -
page_cache.c | 1 -
qemu-char.c | 5 +-
qemu-doc.texi | 2 +-
qemu-img.texi | 2 +-
qemu-options.hx | 2 +-
qga/commands-win32.c | 2 +-
scripts/useless-if-before-free | 207 +++++++
scripts/vc-list-files | 113 ++++
target-arm/cpu.h | 4 +-
target-arm/helper.c | 2 +-
target-arm/translate.c | 2 +-
target-i386/translate.c | 1 -
target-lm32/helper.c | 2 +-
target-microblaze/op_helper.c | 1 -
target-microblaze/translate.c | 2 +-
target-mips/helper.c | 1 -
target-moxie/helper.c | 3 +-
target-moxie/translate.c | 1 -
target-sh4/helper.c | 1 -
target-sh4/op_helper.c | 1 -
target-tricore/helper.c | 1 -
tests/Makefile | 2 +-
tests/bios-tables-test.c | 36 +-
tests/tcg/testthread.c | 1 -
tests/test-xbzrle.c | 2 -
ui/spice-display.c | 14 +-
util/bitmap.c | 2 +-
78 files changed, 1901 insertions(+), 155 deletions(-)
create mode 100644 Makefile.nonreentrant
create mode 100644 cfg.mk
create mode 100644 maint.mk
create mode 100755 scripts/useless-if-before-free
create mode 100755 scripts/vc-list-files
--
2.4.3
