[Qemu-devel] [PULL 00/14] checkpatch, warnings and other fixes

[Qemu-devel] [PULL 00/14] checkpatch, warnings and other fixes

[Paolo Bonzini]

The following changes since commit 2bb15bddf2607110820d5ce5aa43baac27292fb3:

  Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into staging (2016-08-09 16:53:32 +0100)

are available in the git repository at:


  git://github.com/bonzini/qemu.git tags/for-upstream

for you to fetch changes up to 141de8865488189ad9d75408b3e0ad24c6fff2bb:

  checkpatch: default to success if only warnings (2016-08-10 12:44:51 +0200)

----------------------------------------------------------------
* pc-bios/optionrom/Makefile fixes
* warning fixes for __atomic_load and -1 << x in clang
* missed interrupt fix from Gonglei
* checkpatch fix from Radim and myself

----------------------------------------------------------------
Gonglei (1):
      timer: set vm_clock disabled default

Marc-André Lureau (1):
      build-sys: fix building with make CFLAGS=.. argument

Paolo Bonzini (9):
      linuxboot_dma: avoid guest ABI breakage on gcc vs. clang compilation
      optionrom: add -fno-stack-protector
      optionrom: fix compilation with mingw docker target
      atomic: strip "const" from variables declared with typeof
      checkpatch: tweak the files in which TABs are checked
      checkpatch: check for CVS keywords on all sources
      CODING_STYLE, checkpatch: update line length rules
      checkpatch: bump most warnings to errors
      checkpatch: default to success if only warnings

Pranith Kumar (2):
      Disable warn about left shifts of negative values
      clang: Fix warning reg. expansion to 'defined'

Radim Krčmář (1):
      checkpatch: ignore automatically imported Linux headers

 CODING_STYLE                      |   8 ++-
 HACKING                           |   4 ++
 Makefile                          |   3 +-
 configure                         |   2 +-
 include/qemu/atomic.h             |  54 ++++++++++++++++---
 include/ui/qemu-spice.h           |   7 ++-
 pc-bios/linuxboot_dma.bin         | Bin 1024 -> 1536 bytes
 pc-bios/optionrom/Makefile        |  13 +++--
 pc-bios/optionrom/flat.lds        |   6 +++
 pc-bios/optionrom/linuxboot_dma.c |   8 ++-
 qemu-timer.c                      |   2 +-
 scripts/checkpatch.pl             | 110 +++++++++++++++++++-------------------
 scripts/signrom.py                |  27 ++++------
 tests/test-aio.c                  |   4 +-
 14 files changed, 158 insertions(+), 90 deletions(-)
 create mode 100644 pc-bios/optionrom/flat.lds
-- 
1.8.3.1

[Qemu-devel] [PULL 01/14] linuxboot_dma: avoid guest ABI breakage on gcc vs. clang compilation

[Paolo Bonzini]

Recent GCC compiles linuxboot_dma.c to 921 bytes, while CentOS 6 needs
1029 and clang needs 1527.  Because the size of the ROM, rounded to the
next 512 bytes, must match, this causes the API to break between a <1K
ROM and one that is bigger.

We want to make the ROM 1.5 KB in size, but it's better to make clang
produce leaner ROMs, because currently it is worryingly close to the limit.
To fix this prevent clang's happy inlining (which -Os cannot prevent).
This only requires adding a noinline attribute.

Second, the patch makes sure that the ROM has enough padding to prevent
ABI breakage on different compilers.  The size is now hardcoded in the file
that is passed to signrom.py, as was the case before commit 6f71b77
("scripts/signrom.py: Allow option ROM checksum script to write the size
header.", 2016-05-23); signrom.py however will still pad the input to
the requested size.  This ensures that the padding goes beyond the
next multiple of 512 if necessary, and also avoids the need for
-fno-toplevel-reorder which clang doesn't support.  signrom.py can then
error out if the requested size is too small for the actual size of the
compiled ROM.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 pc-bios/linuxboot_dma.bin         | Bin 1024 -> 1536 bytes
 pc-bios/optionrom/linuxboot_dma.c |   8 ++++++--
 scripts/signrom.py                |  27 +++++++++++----------------
 3 files changed, 17 insertions(+), 18 deletions(-)

diff --git a/pc-bios/linuxboot_dma.bin b/pc-bios/linuxboot_dma.bin
index e1f623a1245efff0d46ea5b685e0f18e4ef98d74..238a195d3869995067f158d243d852778d38a736 100644
GIT binary patch
literal 1536
zcmeHFO=}ZT6n#mjn#9N?ZK{K0k;PaL-DD+#E`)TKK>SF%@QZ?;pp*(SMO{f8vv`CI
z_!9&-x)zKBLZ%f_LNuh%S`|r(v{S59T@^nv-g)U07ybYjeTz4D&OPs(`|i7ihXW1v
z&y{3)emWlr%H&aYT!!qlh)#^<3M_khdgexI>gwdhOV?7FoX`3Gb8V4T4OV^17z*G`
zOn^I3n-~Xwj#DUg8S@xQE3`AD-$A|K(7+ertc1X4;Vf$ie}J<ZLwEt^>j?IftcP?P
z=6ZtIvqam3<7z0a3aQ<ckN9L~BLt^Q$2W`MSv^F%p+{+-(iPej9lQfu8zf)#1vZMP
z5JZvL=^1gV`EOw68_Z`3oWXfqz&XrSAJM6CJW<GuZej27zvjA-bca39E25gwV_O7!
zub*~*^nBcRu{8oH%o4@@_y=}e#j2yWgBm?rtgvIj;Pp7{L-dOpX&xS>vV%fdk;)OF
zd{0P_V?yG)1kU@?8C~hi6fr;?lBo<)AHR4WO3UI?t{W35w~doH(lT=XJJbR$+43du
zx6Kjoh43q=nR%#VPPwh#*%zRfiBT!H$tO_6LEl;^Qi;M&6PFP6(lryXhx%9clkV4F
zTOA6?dUuL?mn5!9JS1>O;H0C$j!T^o+y{~$mHc7Bzbo;i#1jI)+1JVFQP?AWovfaO
zJK4t*I(6nOC3y=6=4q%Dn4e<S8>?=0EZ1s*zc=ft{1q{%>6iWxrfC#EH)uF?F&uL=
cH&7qfG012%;B+Eu-5wcjuk8-}=N-898@_u-5C8xG

literal 1024
zcmd6mziX307{@Q^tG-DiZ>p(g$dKVIh%T9=qM-1)wGcJY!KxJW`(h~-Bt;!#jBtbm
z9lAI>If`HmD0vg9wINblk!l@67gN$=2eDW*ug_gPT>JyP;dt+J&wZbtZ~C#n!Tz~o
zj3=j(KEJ*^#!l)_mQr7*PmQM8$hE2ITk*;3<5#ZUh})ymX8Y&b7go%$;tR&$u7;7u
zd7Q&ph$V;`5*|d8xQX0)4B`i}tBiRJVivhZ(1A#CL)y8mD6B{C<hEr(%)@Frcy05r
zbS@uxj5+8hYkD)uLkKKDY|{mCMhBuck3cNL?L^_WO=M-NK^%}T5vY^F7&jdhZEm6x
z-9gZr!&e7e1U%2{W_I8Idzk))+^ZNa;1Vw5BI@G1%1`F?+>jnC`{bTm=MXKl`$#}Z
zhxPS-dEM@GB`+A9wh<mCzY4$Cm;yCQRg`y9;fI_8JMQ=296;qT9Z@FDBSRFnUnv_}
zIjAHaC<#g?qItjKw|mkl9&ykoYg?9LBvVjT-1I!E2<_I4i{Y9zQ@!hEfc#=+9ElZ_
zeQokAK0<tI4nn*Qe6aGvK3LB-iBK5#7$vW|oUM^NAGIx*Uy;oe3$W~22Zv-t%7f~N
zK`HnA%QUD46js5euqu00xbwgM6wcIX>{9%R=26W}#UCn8I#H!Vog7g7j<yeJ`@M=!
zXdcr%qWGDfj?F_Vsu(JRiXZLqh-}yxO&O(ia7lIvxlXvWT&k8@rFMx%?{lsiPfjpd
jw8R@E7UiHPHX1ZFTun{A_*7Ps{(-r*mtE5S57vp_DS}uK

diff --git a/pc-bios/optionrom/linuxboot_dma.c b/pc-bios/optionrom/linuxboot_dma.c
index 8509b28..7549797 100644
--- a/pc-bios/optionrom/linuxboot_dma.c
+++ b/pc-bios/optionrom/linuxboot_dma.c
@@ -25,7 +25,7 @@ asm(
 ".global _start\n"
 "_start:\n"
 "   .short 0xaa55\n"
-"   .byte 0\n" /* size in 512 units, filled in by signrom.py */
+"   .byte 3\n" /* desired size in 512 units; signrom.py adds padding */
 "   .byte 0xcb\n" /* far return without prefix */
 "   .org 0x18\n"
 "   .short 0\n"
@@ -157,7 +157,11 @@ static inline uint32_t be32_to_cpu(uint32_t x)
     return bswap32(x);
 }
 
-static void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
+/* clang is happy to inline this function, and bloats the
+ * ROM.
+ */
+static __attribute__((__noinline__))
+void bios_cfg_read_entry(void *buf, uint16_t entry, uint32_t len)
 {
     FWCfgDmaAccess access;
     uint32_t control = (entry << 16) | BIOS_CFG_DMA_CTL_SELECT
diff --git a/scripts/signrom.py b/scripts/signrom.py
index 5629bca..d1dabe0 100644
--- a/scripts/signrom.py
+++ b/scripts/signrom.py
@@ -23,26 +23,21 @@ if magic != '\x55\xaa':
 
 size_byte = ord(fin.read(1))
 fin.seek(0)
+data = fin.read()
 
-if size_byte == 0:
-    # If the caller left the size field blank then we will fill it in,
-    # also rounding the whole input to a multiple of 512 bytes.
-    data = fin.read()
-    # +1 because we need a byte to store the checksum.
-    size = len(data) + 1
-    # Round up to next multiple of 512.
-    size += 511
-    size -= size % 512
-    if size >= 65536:
-        sys.exit("%s: option ROM size too large" % sys.argv[1])
+size = size_byte * 512
+if len(data) > size:
+    sys.stderr.write('error: ROM is too large (%d > %d)\n' % (len(data), size))
+    sys.exit(1)
+elif len(data) < size:
+    # Add padding if necessary, rounding the whole input to a multiple of
+    # 512 bytes according to the third byte of the input.
     # size-1 because a final byte is added below to store the checksum.
     data = data.ljust(size-1, '\0')
-    data = data[:2] + chr(size/512) + data[3:]
 else:
-    # Otherwise the input file specifies the size so use it.
-    # -1 because we overwrite the last byte of the file with the checksum.
-    size = size_byte * 512 - 1
-    data = fin.read(size)
+    if ord(data[-1:]) != 0:
+        sys.stderr.write('WARNING: ROM includes nonzero checksum\n')
+    data = data[:size-1]
 
 fout.write(data)
 
-- 
1.8.3.1

[Qemu-devel] [PULL 02/14] build-sys: fix building with make CFLAGS=.. argument

[Paolo Bonzini]

From: Marc-André Lureau <marcandre.lureau@redhat.com>

When calling make with a CFLAGS=.. argument, the -g/-O filter is not
applied, which may result with build failure with ASAN for example. It
could be solved with an 'override' directive on CFLAGS, but that would
actually prevent setting different CFLAGS manually.

Instead, filter the CFLAGS argument from the top-level Makefile (so
you could still call make with a different CFLAGS argument on a
rom/Makefile manually)

Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Message-Id: <20160805082421.21994-2-marcandre.lureau@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 Makefile                   | 3 ++-
 pc-bios/optionrom/Makefile | 2 --
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/Makefile b/Makefile
index 0d7647f..50b4b3a 100644
--- a/Makefile
+++ b/Makefile
@@ -225,8 +225,9 @@ dtc/%:
 $(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
 
 ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
+# Only keep -O and -g cflags
 romsubdir-%:
-	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/",)
+	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)
 
 ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
 
diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 24e175e..6bab490 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -24,8 +24,6 @@ QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -no-integrated-as)
 QEMU_CFLAGS += -m32 -include $(SRC_PATH)/pc-bios/optionrom/code16gcc.h
 endif
 
-# Drop gcov and glib flags
-CFLAGS := $(filter -O% -g%, $(CFLAGS))
 QEMU_INCLUDES += -I$(SRC_PATH)
 
 Wa = -Wa,
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 02/14] build-sys: fix building with make CFLAGS=.. argument

[Christian Borntraeger]

On 08/10/2016 03:57 PM, Paolo Bonzini wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> When calling make with a CFLAGS=.. argument, the -g/-O filter is not
> applied, which may result with build failure with ASAN for example. It
> could be solved with an 'override' directive on CFLAGS, but that would
> actually prevent setting different CFLAGS manually.
> 
> Instead, filter the CFLAGS argument from the top-level Makefile (so
> you could still call make with a different CFLAGS argument on a
> rom/Makefile manually)
> 
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Message-Id: <20160805082421.21994-2-marcandre.lureau@redhat.com>
> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>


This patch breaks the build on the s390-ccw.img bios if I enable
--enable-debug

main.o: In function `virtio_setup':
/home/cborntra/REPOS/qemu/pc-bios/s390-ccw/main.c:117: undefined reference to `__stack_chk_fail'

Looks like it also removes other necessary fixups like -msoft-float.


 ---
>  Makefile                   | 3 ++-
>  pc-bios/optionrom/Makefile | 2 --
>  2 files changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index 0d7647f..50b4b3a 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -225,8 +225,9 @@ dtc/%:
>  $(SUBDIR_RULES): libqemuutil.a libqemustub.a $(common-obj-y) $(qom-obj-y) $(crypto-aes-obj-$(CONFIG_USER_ONLY))
> 
>  ROMSUBDIR_RULES=$(patsubst %,romsubdir-%, $(ROMS))
> +# Only keep -O and -g cflags
>  romsubdir-%:
> -	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/",)
> +	$(call quiet-command,$(MAKE) $(SUBDIR_MAKEFLAGS) -C pc-bios/$* V="$(V)" TARGET_DIR="$*/" CFLAGS="$(filter -O% -g%,$(CFLAGS))",)
> 
>  ALL_SUBDIRS=$(TARGET_DIRS) $(patsubst %,pc-bios/%, $(ROMS))
> 
> diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
> index 24e175e..6bab490 100644
> --- a/pc-bios/optionrom/Makefile
> +++ b/pc-bios/optionrom/Makefile
> @@ -24,8 +24,6 @@ QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -no-integrated-as)
>  QEMU_CFLAGS += -m32 -include $(SRC_PATH)/pc-bios/optionrom/code16gcc.h
>  endif
> 
> -# Drop gcov and glib flags
> -CFLAGS := $(filter -O% -g%, $(CFLAGS))
>  QEMU_INCLUDES += -I$(SRC_PATH)
> 
>  Wa = -Wa,
> 

Re: [Qemu-devel] [PULL 02/14] build-sys: fix building with make CFLAGS=.. argument

[Christian Borntraeger]

On 08/15/2016 10:41 AM, Christian Borntraeger wrote:
> On 08/10/2016 03:57 PM, Paolo Bonzini wrote:
>> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>>
>> When calling make with a CFLAGS=.. argument, the -g/-O filter is not
>> applied, which may result with build failure with ASAN for example. It
>> could be solved with an 'override' directive on CFLAGS, but that would
>> actually prevent setting different CFLAGS manually.
>>
>> Instead, filter the CFLAGS argument from the top-level Makefile (so
>> you could still call make with a different CFLAGS argument on a
>> rom/Makefile manually)
>>
>> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
>> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
>> Message-Id: <20160805082421.21994-2-marcandre.lureau@redhat.com>
>> Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
> 
> 
> This patch breaks the build on the s390-ccw.img bios if I enable
> --enable-debug
> 
[...]
> 
Something like 
--- a/pc-bios/s390-ccw/Makefile
+++ b/pc-bios/s390-ccw/Makefile
@@ -10,8 +10,10 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/s390-ccw)
 .PHONY : all clean build-all
 
 OBJECTS = start.o main.o bootmap.o sclp-ascii.o virtio.o virtio-scsi.o
-CFLAGS += -fPIE -fno-stack-protector -ffreestanding -march=z900
-CFLAGS += -fno-delete-null-pointer-checks -msoft-float
+QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS))
+QEMU_CFLAGS := -ffreestanding -fno-delete-null-pointer-checks -msoft-float
+QEMU_CFLAGS := -march=z900 -fPIE
+QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
 LDFLAGS += -Wl,-pie -nostdlib
 
 build-all: s390-ccw.img


seems to help. I need to double check...

[Qemu-devel] [PULL 03/14] optionrom: add -fno-stack-protector

[Paolo Bonzini]

This is required by OpenBSD.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 pc-bios/optionrom/Makefile | 1 +
 1 file changed, 1 insertion(+)

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 6bab490..9c018ea 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -11,6 +11,7 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
 
 # Drop -fstack-protector and the like
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS)) $(CFLAGS_NOPIE) -ffreestanding
+QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
 QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -m16)
 ifeq ($(filter -m16, $(QEMU_CFLAGS)),)
 # Attempt to work around compilers that lack -m16 (GCC <= 4.8, clang <= ??)
-- 
1.8.3.1

[Qemu-devel] [PULL 04/14] optionrom: fix compilation with mingw docker target

[Paolo Bonzini]

Two fixes are needed.  First, mingw does not have -D_FORTIFY_SOURCE,
hence --enable-debug disables optimization.  This is not acceptable
for ROMs, which should override CFLAGS to force inclusion of -O2.

Second, PE stores global constructors and destructors using the
following linker script snippet:

     ___CTOR_LIST__ = .; __CTOR_LIST__ = . ;
			LONG (-1);*(.ctors); *(.ctor); *(SORT(.ctors.*));  LONG (0);
     ___DTOR_LIST__ = .; __DTOR_LIST__ = . ;
			LONG (-1); *(.dtors); *(.dtor); *(SORT(.dtors.*));  LONG (0);

The LONG directives cause the .img files to be 16 bytes too large;
the recently added check to signrom.py catches this.  To fix this,
replace -T and -e options with a linker script.

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 pc-bios/optionrom/Makefile | 10 +++++++++-
 pc-bios/optionrom/flat.lds |  6 ++++++
 2 files changed, 15 insertions(+), 1 deletion(-)
 create mode 100644 pc-bios/optionrom/flat.lds

diff --git a/pc-bios/optionrom/Makefile b/pc-bios/optionrom/Makefile
index 9c018ea..8aef152 100644
--- a/pc-bios/optionrom/Makefile
+++ b/pc-bios/optionrom/Makefile
@@ -9,6 +9,14 @@ $(call set-vpath, $(SRC_PATH)/pc-bios/optionrom)
 
 .PHONY : all clean build-all
 
+# Compiling with no optimization creates ROMs that are too large
+ifeq ($(filter -O%, $(CFLAGS)),)
+override CFLAGS += -O2
+endif
+ifeq ($(filter -O%, $(CFLAGS)),-O0)
+override CFLAGS += -O2
+endif
+
 # Drop -fstack-protector and the like
 QEMU_CFLAGS := $(filter -W%, $(QEMU_CFLAGS)) $(CFLAGS_NOPIE) -ffreestanding
 QEMU_CFLAGS += $(call cc-option, $(QEMU_CFLAGS), -fno-stack-protector)
@@ -51,7 +59,7 @@ endif
 endif
 
 %.img: %.o
-	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_EMULATION) -Ttext 0 -e _start -s -o $@ $<,"  Building $(TARGET_DIR)$@")
+	$(call quiet-command,$(LD) $(LDFLAGS_NOPIE) -m $(LD_EMULATION) -T $(SRC_PATH)/pc-bios/optionrom/flat.lds -s -o $@ $<,"  Building $(TARGET_DIR)$@")
 
 %.raw: %.img
 	$(call quiet-command,$(OBJCOPY) -O binary -j .text $< $@,"  Building $(TARGET_DIR)$@")
diff --git a/pc-bios/optionrom/flat.lds b/pc-bios/optionrom/flat.lds
new file mode 100644
index 0000000..cee2eda
--- /dev/null
+++ b/pc-bios/optionrom/flat.lds
@@ -0,0 +1,6 @@
+SECTIONS
+{
+  . = 0;
+  .text : { *(.text) *(.text.$) }
+}
+ENTRY(_start)
-- 
1.8.3.1

[Qemu-devel] [PULL 05/14] atomic: strip "const" from variables declared with typeof

[Paolo Bonzini]

With the latest clang, we have the following warning:

    /home/pranith/devops/code/qemu/include/qemu/seqlock.h:62:21: warning: passing 'typeof (*&sl->sequence) *' (aka 'const unsigned int *') to parameter of type 'unsigned int *' discards qualifiers [-Wincompatible-pointer-types-discards-qualifiers]
        return unlikely(atomic_read(&sl->sequence) != start);
                        ^~~~~~~~~~~~~~~~~~~~~~~~~~
    /home/pranith/devops/code/qemu/include/qemu/atomic.h:58:25: note: expanded from macro 'atomic_read'
        __atomic_load(ptr, &_val, __ATOMIC_RELAXED);     \
                           ^~~~~

Stripping const is a bit tricky due to promotions, but it is doable
with either C11 _Generic or GCC extensions.  Use the latter.

Reported-by: Pranith Kumar <bobby.prani@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[pranith: Add conversion for bool type]
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/qemu/atomic.h | 54 +++++++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/include/qemu/atomic.h b/include/qemu/atomic.h
index 7e13fca..43b0645 100644
--- a/include/qemu/atomic.h
+++ b/include/qemu/atomic.h
@@ -18,6 +18,48 @@
 /* Compiler barrier */
 #define barrier()   ({ asm volatile("" ::: "memory"); (void)0; })
 
+/* The variable that receives the old value of an atomically-accessed
+ * variable must be non-qualified, because atomic builtins return values
+ * through a pointer-type argument as in __atomic_load(&var, &old, MODEL).
+ *
+ * This macro has to handle types smaller than int manually, because of
+ * implicit promotion.  int and larger types, as well as pointers, can be
+ * converted to a non-qualified type just by applying a binary operator.
+ */
+#define typeof_strip_qual(expr)                                                    \
+  typeof(                                                                          \
+    __builtin_choose_expr(                                                         \
+      __builtin_types_compatible_p(typeof(expr), bool) ||                          \
+        __builtin_types_compatible_p(typeof(expr), const bool) ||                  \
+        __builtin_types_compatible_p(typeof(expr), volatile bool) ||               \
+        __builtin_types_compatible_p(typeof(expr), const volatile bool),           \
+        (bool)1,                                                                   \
+    __builtin_choose_expr(                                                         \
+      __builtin_types_compatible_p(typeof(expr), signed char) ||                   \
+        __builtin_types_compatible_p(typeof(expr), const signed char) ||           \
+        __builtin_types_compatible_p(typeof(expr), volatile signed char) ||        \
+        __builtin_types_compatible_p(typeof(expr), const volatile signed char),    \
+        (signed char)1,                                                            \
+    __builtin_choose_expr(                                                         \
+      __builtin_types_compatible_p(typeof(expr), unsigned char) ||                 \
+        __builtin_types_compatible_p(typeof(expr), const unsigned char) ||         \
+        __builtin_types_compatible_p(typeof(expr), volatile unsigned char) ||      \
+        __builtin_types_compatible_p(typeof(expr), const volatile unsigned char),  \
+        (unsigned char)1,                                                          \
+    __builtin_choose_expr(                                                         \
+      __builtin_types_compatible_p(typeof(expr), signed short) ||                  \
+        __builtin_types_compatible_p(typeof(expr), const signed short) ||          \
+        __builtin_types_compatible_p(typeof(expr), volatile signed short) ||       \
+        __builtin_types_compatible_p(typeof(expr), const volatile signed short),   \
+        (signed short)1,                                                           \
+    __builtin_choose_expr(                                                         \
+      __builtin_types_compatible_p(typeof(expr), unsigned short) ||                \
+        __builtin_types_compatible_p(typeof(expr), const unsigned short) ||        \
+        __builtin_types_compatible_p(typeof(expr), volatile unsigned short) ||     \
+        __builtin_types_compatible_p(typeof(expr), const volatile unsigned short), \
+        (unsigned short)1,                                                         \
+      (expr)+0))))))
+
 #ifdef __ATOMIC_RELAXED
 /* For C11 atomic ops */
 
@@ -54,7 +96,7 @@
 #define atomic_read(ptr)                              \
     ({                                                \
     QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
-    typeof(*ptr) _val;                                \
+    typeof_strip_qual(*ptr) _val;                     \
      __atomic_load(ptr, &_val, __ATOMIC_RELAXED);     \
     _val;                                             \
     })
@@ -80,7 +122,7 @@
 #define atomic_rcu_read(ptr)                          \
     ({                                                \
     QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *)); \
-    typeof(*ptr) _val;                                \
+    typeof_strip_qual(*ptr) _val;                     \
     atomic_rcu_read__nocheck(ptr, &_val);             \
     _val;                                             \
     })
@@ -103,7 +145,7 @@
 #define atomic_mb_read(ptr)                             \
     ({                                                  \
     QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));   \
-    typeof(*ptr) _val;                                  \
+    typeof_strip_qual(*ptr) _val;                       \
      __atomic_load(ptr, &_val, __ATOMIC_RELAXED);       \
      smp_rmb();                                         \
     _val;                                               \
@@ -120,7 +162,7 @@
 #define atomic_mb_read(ptr)                             \
     ({                                                  \
     QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));   \
-    typeof(*ptr) _val;                                  \
+    typeof_strip_qual(*ptr) _val;                       \
     __atomic_load(ptr, &_val, __ATOMIC_SEQ_CST);        \
     _val;                                               \
     })
@@ -137,7 +179,7 @@
 
 #define atomic_xchg(ptr, i)    ({                           \
     QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));       \
-    typeof(*ptr) _new = (i), _old;                          \
+    typeof_strip_qual(*ptr) _new = (i), _old;               \
     __atomic_exchange(ptr, &_new, &_old, __ATOMIC_SEQ_CST); \
     _old;                                                   \
 })
@@ -146,7 +188,7 @@
 #define atomic_cmpxchg(ptr, old, new)                                   \
     ({                                                                  \
     QEMU_BUILD_BUG_ON(sizeof(*ptr) > sizeof(void *));                   \
-    typeof(*ptr) _old = (old), _new = (new);                            \
+    typeof_strip_qual(*ptr) _old = (old), _new = (new);                 \
     __atomic_compare_exchange(ptr, &_old, &_new, false,                 \
                               __ATOMIC_SEQ_CST, __ATOMIC_SEQ_CST);      \
     _old;                                                               \
-- 
1.8.3.1

[Qemu-devel] [PULL 06/14] Disable warn about left shifts of negative values

[Paolo Bonzini]

From: Pranith Kumar <bobby.prani@gmail.com>

It seems like there's no good reason for the compiler to exploit the
undefinedness of left shifts.  GCC explicitly documents that they do not
use at all this possibility and, while they also say this is subject
to change, they have been saying this for 10 years (since the wording
appeared in the GCC 4.0 manual).

Disable these warnings by passing in -Wno-shift-negative-value.

Cc: Peter Maydell <peter.maydell@linaro.org>
Cc: Markus Armbruster <armbru@redhat.com>
Cc: Laszlo Ersek <lersek@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
[pranith: forward-port part of patch to 2.7]
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
---
 HACKING   | 4 ++++
 configure | 2 +-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/HACKING b/HACKING
index 058aa8f..20a9101 100644
--- a/HACKING
+++ b/HACKING
@@ -158,6 +158,10 @@ painful. These are:
  * you may assume that right shift of a signed integer duplicates
    the sign bit (ie it is an arithmetic shift, not a logical shift)
 
+In addition, QEMU assumes that the compiler does not use the latitude
+given in C99 and C11 to treat aspects of signed '<<' as undefined, as
+documented in the GNU Compiler Collection manual starting at version 4.0.
+
 7. Error handling and reporting
 
 7.1 Reporting errors to the human user
diff --git a/configure b/configure
index f57fcc6..8d84919 100755
--- a/configure
+++ b/configure
@@ -1452,7 +1452,7 @@ fi
 gcc_flags="-Wold-style-declaration -Wold-style-definition -Wtype-limits"
 gcc_flags="-Wformat-security -Wformat-y2k -Winit-self -Wignored-qualifiers $gcc_flags"
 gcc_flags="-Wmissing-include-dirs -Wempty-body -Wnested-externs $gcc_flags"
-gcc_flags="-Wendif-labels $gcc_flags"
+gcc_flags="-Wendif-labels -Wno-shift-negative-value $gcc_flags"
 gcc_flags="-Wno-initializer-overrides $gcc_flags"
 gcc_flags="-Wno-string-plus-int $gcc_flags"
 # Note that we do not add -Werror to gcc_flags here, because that would
-- 
1.8.3.1

[Qemu-devel] [PULL 07/14] clang: Fix warning reg. expansion to 'defined'

[Paolo Bonzini]

From: Pranith Kumar <bobby.prani@gmail.com>

Clang produces the following warning. The warning is detailed here:
https://reviews.llvm.org/D15866. Fix the warning.

/home/pranith/devops/code/qemu/hw/display/qxl.c:507:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
    ^
/home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
    ^
/home/pranith/devops/code/qemu/hw/display/qxl.c:1074:5: warning: macro expansion producing 'defined' has undefined behavior [-Wexpansion-to-defined]
    ^
/home/pranith/devops/code/qemu/include/ui/qemu-spice.h:46:5: note: expanded from macro 'SPICE_NEEDS_SET_MM_TIME'
  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))

Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Pranith Kumar <bobby.prani@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 include/ui/qemu-spice.h | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/include/ui/qemu-spice.h b/include/ui/qemu-spice.h
index edad5e7..75e1239 100644
--- a/include/ui/qemu-spice.h
+++ b/include/ui/qemu-spice.h
@@ -42,8 +42,11 @@ int qemu_spice_set_pw_expire(time_t expires);
 int qemu_spice_migrate_info(const char *hostname, int port, int tls_port,
                             const char *subject);
 
-#define SPICE_NEEDS_SET_MM_TIME                                       \
-  (!defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06))
+#if !defined(SPICE_SERVER_VERSION) || (SPICE_SERVER_VERSION < 0xc06)
+#define SPICE_NEEDS_SET_MM_TIME 1
+#else
+#define SPICE_NEEDS_SET_MM_TIME 0
+#endif
 
 #if SPICE_SERVER_VERSION >= 0x000c02
 void qemu_spice_register_ports(void);
-- 
1.8.3.1

[Qemu-devel] [PULL 08/14] checkpatch: ignore automatically imported Linux headers

[Paolo Bonzini]

From: Radim Krčmář <rkrcmar@redhat.com>

Linux uses tabs for indentation and checkpatch always complained about
automatically imported headers.  update-linux-headers.sh could be modified to
expand tabs, but there is no real reason to complain about any ugly code in
Linux headers, so skip all hunk-related checks.

Signed-off-by: Radim Krčmář <rkrcmar@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 9297087..8d1813e 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1319,6 +1319,9 @@ sub process {
 # ignore non-hunk lines and lines being removed
 		next if (!$hunk_line || $line =~ /^-/);
 
+# ignore files that are being periodically imported from Linux
+		next if ($realfile =~ /^(linux-headers|include\/standard-headers)\//);
+
 #trailing whitespace
 		if ($line =~ /^\+.*\015/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
-- 
1.8.3.1

[Qemu-devel] [PULL 09/14] timer: set vm_clock disabled default

[Paolo Bonzini]

From: Gonglei <arei.gonglei@huawei.com>

(commit 80dcfb8532ae76343109a48f12ba8ca1c505c179)
Upon migration, the code use a timer based on vm_clock for 1ns
in the future from post_load to do the event send in case host_connected
differs between migration source and target.

However, it's not guaranteed that the apic is ready to inject irqs into
the guest, and the irq line remained high, resulting in any future interrupts
going unnoticed by the guest as well.

That's because 1) the migration coroutine is not blocked when it get EAGAIN
while reading QEMUFile. 2) The vm_clock is enabled default currently, it doesn't
rely on the calling of vm_start(), that means vm_clock timers can run before
VCPUs are running.

So, let's set the vm_clock disabled default, keep the initial intention of
design for vm_clock timers.

Meanwhile, change the test-aio usecase, using QEMU_CLOCK_REALTIME instead of
QEMU_CLOCK_VIRTUAL as the block code does.

CC: Paolo Bonzini <pbonzini@redhat.com>
CC: Dr. David Alan Gilbert <dgilbert@redhat.com>
CC: qemu-stable@nongnu.org
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Message-Id: <1470728955-90600-1-git-send-email-arei.gonglei@huawei.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 qemu-timer.c     | 2 +-
 tests/test-aio.c | 4 ++--
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/qemu-timer.c b/qemu-timer.c
index eb22e92..9299cdc 100644
--- a/qemu-timer.c
+++ b/qemu-timer.c
@@ -129,7 +129,7 @@ static void qemu_clock_init(QEMUClockType type)
     assert(main_loop_tlg.tl[type] == NULL);
 
     clock->type = type;
-    clock->enabled = true;
+    clock->enabled = (type == QEMU_CLOCK_VIRTUAL ? false : true);
     clock->last = INT64_MIN;
     QLIST_INIT(&clock->timerlists);
     notifier_list_init(&clock->reset_notifiers);
diff --git a/tests/test-aio.c b/tests/test-aio.c
index 982339c..03aa846 100644
--- a/tests/test-aio.c
+++ b/tests/test-aio.c
@@ -452,7 +452,7 @@ static void test_timer_schedule(void)
 {
     TimerTestData data = { .n = 0, .ctx = ctx, .ns = SCALE_MS * 750LL,
                            .max = 2,
-                           .clock_type = QEMU_CLOCK_VIRTUAL };
+                           .clock_type = QEMU_CLOCK_REALTIME };
     EventNotifier e;
 
     /* aio_poll will not block to wait for timers to complete unless it has
@@ -782,7 +782,7 @@ static void test_source_timer_schedule(void)
 {
     TimerTestData data = { .n = 0, .ctx = ctx, .ns = SCALE_MS * 750LL,
                            .max = 2,
-                           .clock_type = QEMU_CLOCK_VIRTUAL };
+                           .clock_type = QEMU_CLOCK_REALTIME };
     EventNotifier e;
     int64_t expiry;
 
-- 
1.8.3.1

[Qemu-devel] [PULL 10/14] checkpatch: tweak the files in which TABs are checked

[Paolo Bonzini]

Include Python and shell scripts, and make an exception for Perl
scripts we imported from Linux or elsewhere.

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 8d1813e..082c4ce 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1334,7 +1334,7 @@ sub process {
 		}
 
 # check we are in a valid source file if not then ignore this hunk
-		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|sh)$/);
+		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
 
 #80 column limit
 		if ($line =~ /^\+/ &&
@@ -1354,10 +1354,11 @@ sub process {
 			WARN("adding a line without newline at end of file\n" . $herecurr);
 		}
 
-# check we are in a valid source file C or perl if not then ignore this hunk
-		next if ($realfile !~ /\.(h|c|cpp|pl)$/);
+# tabs are only allowed in assembly source code, and in
+# some scripts we imported from other projects.
+		next if ($realfile =~ /\.(s|S)$/);
+		next if ($realfile =~ /(checkpatch|get_maintainer|texi2pod)\.pl$/);
 
-# in QEMU, no tabs are allowed
 		if ($rawline =~ /^\+.*\t/) {
 			my $herevet = "$here\n" . cat_vet($rawline) . "\n";
 			ERROR("code indent should never use tabs\n" . $herevet);
-- 
1.8.3.1

[Qemu-devel] [PULL 11/14] checkpatch: check for CVS keywords on all sources

[Paolo Bonzini]

These should apply to all files, not just C/C++.  Tweak the regular
expression to check for whole words, to avoid false positives on Perl
variables starting with "Id".

Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 082c4ce..fea576d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1354,6 +1354,11 @@ sub process {
 			WARN("adding a line without newline at end of file\n" . $herecurr);
 		}
 
+# check for RCS/CVS revision markers
+		if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|\b)/) {
+			WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr);
+		}
+
 # tabs are only allowed in assembly source code, and in
 # some scripts we imported from other projects.
 		next if ($realfile =~ /\.(s|S)$/);
@@ -1368,11 +1373,6 @@ sub process {
 # check we are in a valid C source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|cpp)$/);
 
-# check for RCS/CVS revision markers
-		if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|)/) {
-			WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr);
-		}
-
 # Check for potential 'bare' types
 		my ($stat, $cond, $line_nr_next, $remain_next, $off_next,
 		    $realline_next);
-- 
1.8.3.1

[Qemu-devel] [PULL 12/14] CODING_STYLE, checkpatch: update line length rules

[Paolo Bonzini]

Line lengths above 80 characters do exist.  They are rare, but
they happen from time to time.  An ignored rule is worse than an
exception to the rule, so do the latter.

Some on the list expressed their preference for a soft limit that
is slightly lower than 80 characters, to account for extra characters
in unified diffs (including three-way diffs) and for email quoting.
However, there was no consensus on this so keep the 80-character
soft limit and add a hard limit at 90.

Acked-by: Cornelia Huck <cornelia.huck@de.ibm.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 CODING_STYLE          | 8 +++++++-
 scripts/checkpatch.pl | 8 ++++++--
 2 files changed, 13 insertions(+), 3 deletions(-)

diff --git a/CODING_STYLE b/CODING_STYLE
index 3c6978f..e7fde15 100644
--- a/CODING_STYLE
+++ b/CODING_STYLE
@@ -31,7 +31,11 @@ Do not leave whitespace dangling off the ends of lines.
 
 2. Line width
 
-Lines are 80 characters; not longer.
+Lines should be 80 characters; try not to make them longer.
+
+Sometimes it is hard to do, especially when dealing with QEMU subsystems
+that use long function or symbol names.  Even in that case, do not make
+lines much longer than 80 characters.
 
 Rationale:
  - Some people like to tile their 24" screens with a 6x4 matrix of 80x24
@@ -39,6 +43,8 @@ Rationale:
    let them keep doing it.
  - Code and especially patches is much more readable if limited to a sane
    line length.  Eighty is traditional.
+ - The four-space indentation makes the most common excuse ("But look
+   at all that white space on the left!") moot.
  - It is the QEMU coding style.
 
 3. Naming
diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index fea576d..ba6760d 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1336,12 +1336,16 @@ sub process {
 # check we are in a valid source file if not then ignore this hunk
 		next if ($realfile !~ /\.(h|c|cpp|s|S|pl|py|sh)$/);
 
-#80 column limit
+#90 column limit
 		if ($line =~ /^\+/ &&
 		    !($line =~ /^\+\s*"[^"]*"\s*(?:\s*|,|\)\s*;)\s*$/) &&
 		    $length > 80)
 		{
-			WARN("line over 80 characters\n" . $herecurr);
+			if ($length > 90) {
+				ERROR("line over 90 characters\n" . $herecurr);
+			} else {
+				WARN("line over 80 characters\n" . $herecurr);
+			}
 		}
 
 # check for spaces before a quoted newline
-- 
1.8.3.1

[Qemu-devel] [PULL 13/14] checkpatch: bump most warnings to errors

[Paolo Bonzini]

This only leaves a warning-level message for the extra-long lines
soft limit.  Everything else is bumped up.

In the future warnings can be added for checks that can have false
positives.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 66 +++++++++++++++++++++++++--------------------------
 1 file changed, 33 insertions(+), 33 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index ba6760d..714dbbe 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -1289,11 +1289,11 @@ sub process {
 			# This is a signoff, if ugly, so do not double report.
 			$signoff++;
 			if (!($line =~ /^\s*Signed-off-by:/)) {
-				WARN("Signed-off-by: is the preferred form\n" .
+				ERROR("The correct form is \"Signed-off-by\"\n" .
 					$herecurr);
 			}
 			if ($line =~ /^\s*signed-off-by:\S/i) {
-				WARN("space required after Signed-off-by:\n" .
+				ERROR("space required after Signed-off-by:\n" .
 					$herecurr);
 			}
 		}
@@ -1350,17 +1350,17 @@ sub process {
 
 # check for spaces before a quoted newline
 		if ($rawline =~ /^.*\".*\s\\n/) {
-			WARN("unnecessary whitespace before a quoted newline\n" . $herecurr);
+			ERROR("unnecessary whitespace before a quoted newline\n" . $herecurr);
 		}
 
 # check for adding lines without a newline.
 		if ($line =~ /^\+/ && defined $lines[$linenr] && $lines[$linenr] =~ /^\\ No newline at end of file/) {
-			WARN("adding a line without newline at end of file\n" . $herecurr);
+			ERROR("adding a line without newline at end of file\n" . $herecurr);
 		}
 
 # check for RCS/CVS revision markers
 		if ($rawline =~ /^\+.*\$(Revision|Log|Id)(?:\$|\b)/) {
-			WARN("CVS style keyword markers, these will _not_ be updated\n". $herecurr);
+			ERROR("CVS style keyword markers, these will _not_ be updated\n". $herecurr);
 		}
 
 # tabs are only allowed in assembly source code, and in
@@ -1506,7 +1506,7 @@ sub process {
 			{
 				my ($nlength, $nindent) = line_stats($lines[$ctx_ln - 1]);
 				if ($nindent > $indent) {
-					WARN("trailing semicolon indicates no statements, indent implies otherwise\n" .
+					ERROR("trailing semicolon indicates no statements, indent implies otherwise\n" .
 						"$here\n$ctx\n$rawlines[$ctx_ln - 1]\n");
 				}
 			}
@@ -1594,7 +1594,7 @@ sub process {
 
 			if ($check && (($sindent % 4) != 0 ||
 			    ($sindent <= $indent && $s ne ''))) {
-				WARN("suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n");
+				ERROR("suspect code indent for conditional statements ($indent, $sindent)\n" . $herecurr . "$stat_real\n");
 			}
 		}
 
@@ -1772,7 +1772,7 @@ sub process {
 			} elsif ($ctx =~ /$Type$/) {
 
 			} else {
-				WARN("space prohibited between function name and open parenthesis '('\n" . $herecurr);
+				ERROR("space prohibited between function name and open parenthesis '('\n" . $herecurr);
 			}
 		}
 # Check operator spacing.
@@ -2011,7 +2011,7 @@ sub process {
 		if ($line =~ /^.\s*return\s*(E[A-Z]*)\s*;/) {
 			my $name = $1;
 			if ($name ne 'EOF' && $name ne 'ERROR') {
-				WARN("return of an errno should typically be -ve (return -$1)\n" . $herecurr);
+				ERROR("return of an errno should typically be -ve (return -$1)\n" . $herecurr);
 			}
 		}
 
@@ -2083,7 +2083,7 @@ sub process {
 				(?:\&\&|\|\||\)|\])
 			)/x)
 		{
-			WARN("boolean test with hexadecimal, perhaps just 1 \& or \|?\n" . $herecurr);
+			ERROR("boolean test with hexadecimal, perhaps just 1 \& or \|?\n" . $herecurr);
 		}
 
 # if and else should not have general statements after it
@@ -2139,7 +2139,7 @@ sub process {
 
 #no spaces allowed after \ in define
 		if ($line=~/\#\s*define.*\\\s$/) {
-			WARN("Whitepspace after \\ makes next lines useless\n" . $herecurr);
+			ERROR("Whitespace after \\ makes next lines useless\n" . $herecurr);
 		}
 
 # multi-statement macros should be enclosed in a do while loop, grab the
@@ -2291,7 +2291,7 @@ sub process {
 					}
 				}
 				if ($seen != ($#chunks + 1)) {
-					WARN("braces {} are necessary for all arms of this statement\n" . $herectx);
+					ERROR("braces {} are necessary for all arms of this statement\n" . $herectx);
 				}
 			}
 		}
@@ -2359,19 +2359,19 @@ sub process {
 					$herectx .= raw_line($linenr, $n) . "\n";;
 				}
 
-				WARN("braces {} are necessary even for single statement blocks\n" . $herectx);
+				ERROR("braces {} are necessary even for single statement blocks\n" . $herectx);
 			}
 		}
 
 # no volatiles please
 		my $asm_volatile = qr{\b(__asm__|asm)\s+(__volatile__|volatile)\b};
 		if ($line =~ /\bvolatile\b/ && $line !~ /$asm_volatile/) {
-			WARN("Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr);
+			ERROR("Use of volatile is usually wrong: see Documentation/volatile-considered-harmful.txt\n" . $herecurr);
 		}
 
 # warn about #if 0
 		if ($line =~ /^.\s*\#\s*if\s+0\b/) {
-			WARN("if this code is redundant consider removing it\n" .
+			ERROR("if this code is redundant consider removing it\n" .
 				$herecurr);
 		}
 
@@ -2379,7 +2379,7 @@ sub process {
 		if ($prevline =~ /\bif\s*\(([^\)]*)\)/) {
 			my $expr = $1;
 			if ($line =~ /\bg_free\(\Q$expr\E\);/) {
-				WARN("g_free(NULL) is safe this check is probably not required\n" . $hereprev);
+				ERROR("g_free(NULL) is safe this check is probably not required\n" . $hereprev);
 			}
 		}
 
@@ -2397,19 +2397,19 @@ sub process {
 # check for memory barriers without a comment.
 		if ($line =~ /\b(smp_mb|smp_rmb|smp_wmb|smp_read_barrier_depends)\(/) {
 			if (!ctx_has_comment($first_line, $linenr)) {
-				WARN("memory barrier without comment\n" . $herecurr);
+				ERROR("memory barrier without comment\n" . $herecurr);
 			}
 		}
 # check of hardware specific defines
 # we have e.g. CONFIG_LINUX and CONFIG_WIN32 for common cases
 # where they might be necessary.
 		if ($line =~ m@^.\s*\#\s*if.*\b__@) {
-			WARN("architecture specific defines should be avoided\n" .  $herecurr);
+			ERROR("architecture specific defines should be avoided\n" .  $herecurr);
 		}
 
 # Check that the storage class is at the beginning of a declaration
 		if ($line =~ /\b$Storage\b/ && $line !~ /^.\s*$Storage\b/) {
-			WARN("storage class should be at the beginning of the declaration\n" . $herecurr)
+			ERROR("storage class should be at the beginning of the declaration\n" . $herecurr)
 		}
 
 # check the location of the inline attribute, that it is between
@@ -2421,7 +2421,7 @@ sub process {
 
 # check for sizeof(&)
 		if ($line =~ /\bsizeof\s*\(\s*\&/) {
-			WARN("sizeof(& should be avoided\n" . $herecurr);
+			ERROR("sizeof(& should be avoided\n" . $herecurr);
 		}
 
 # check for new externs in .c files.
@@ -2438,40 +2438,40 @@ sub process {
 			if ($s =~ /^\s*;/ &&
 			    $function_name ne 'uninitialized_var')
 			{
-				WARN("externs should be avoided in .c files\n" .  $herecurr);
+				ERROR("externs should be avoided in .c files\n" .  $herecurr);
 			}
 
 			if ($paren_space =~ /\n/) {
-				WARN("arguments for function declarations should follow identifier\n" . $herecurr);
+				ERROR("arguments for function declarations should follow identifier\n" . $herecurr);
 			}
 
 		} elsif ($realfile =~ /\.c$/ && defined $stat &&
 		    $stat =~ /^.\s*extern\s+/)
 		{
-			WARN("externs should be avoided in .c files\n" .  $herecurr);
+			ERROR("externs should be avoided in .c files\n" .  $herecurr);
 		}
 
 # check for pointless casting of g_malloc return
 		if ($line =~ /\*\s*\)\s*g_(try)?(m|re)alloc(0?)(_n)?\b/) {
 			if ($2 == 'm') {
-				WARN("unnecessary cast may hide bugs, use g_$1new$3 instead\n" . $herecurr);
+				ERROR("unnecessary cast may hide bugs, use g_$1new$3 instead\n" . $herecurr);
 			} else {
-				WARN("unnecessary cast may hide bugs, use g_$1renew$3 instead\n" . $herecurr);
+				ERROR("unnecessary cast may hide bugs, use g_$1renew$3 instead\n" . $herecurr);
 			}
 		}
 
 # check for gcc specific __FUNCTION__
 		if ($line =~ /__FUNCTION__/) {
-			WARN("__func__ should be used instead of gcc specific __FUNCTION__\n"  . $herecurr);
+			ERROR("__func__ should be used instead of gcc specific __FUNCTION__\n"  . $herecurr);
 		}
 
 # recommend qemu_strto* over strto* for numeric conversions
 		if ($line =~ /\b(strto[^kd].*?)\s*\(/) {
-			WARN("consider using qemu_$1 in preference to $1\n" . $herecurr);
+			ERROR("consider using qemu_$1 in preference to $1\n" . $herecurr);
 		}
 # check for module_init(), use category-specific init macros explicitly please
 		if ($line =~ /^module_init\s*\(/) {
-			WARN("please use block_init(), type_init() etc. instead of module_init()\n" . $herecurr);
+			ERROR("please use block_init(), type_init() etc. instead of module_init()\n" . $herecurr);
 		}
 # check for various ops structs, ensure they are const.
 		my $struct_ops = qr{AIOCBInfo|
@@ -2496,7 +2496,7 @@ sub process {
 				VMStateInfo}x;
 		if ($line !~ /\bconst\b/ &&
 		    $line =~ /\b($struct_ops)\b/) {
-			WARN("struct $1 should normally be const\n" .
+			ERROR("struct $1 should normally be const\n" .
 				$herecurr);
 		}
 
@@ -2506,14 +2506,14 @@ sub process {
 			$string = substr($rawline, $-[1], $+[1] - $-[1]);
 			$string =~ s/%%/__/g;
 			if ($string =~ /(?<!%)%L[udi]/) {
-				WARN("\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr);
+				ERROR("\%Ld/%Lu are not-standard C, use %lld/%llu\n" . $herecurr);
 				last;
 			}
 		}
 
 # QEMU specific tests
 		if ($rawline =~ /\b(?:Qemu|QEmu)\b/) {
-			WARN("use QEMU instead of Qemu or QEmu\n" . $herecurr);
+			ERROR("use QEMU instead of Qemu or QEmu\n" . $herecurr);
 		}
 
 # Qemu error function tests
@@ -2530,7 +2530,7 @@ sub process {
 				error_report}x;
 
 	if ($rawline =~ /\b(?:$qemu_error_funcs)\s*\(.*\".*\\n/) {
-		WARN("Error messages should not contain newlines\n" . $herecurr);
+		ERROR("Error messages should not contain newlines\n" . $herecurr);
 	}
 
 	# Continue checking for error messages that contains newlines. This
@@ -2551,7 +2551,7 @@ sub process {
 		}
 
 		if ($rawlines[$i] =~ /\b(?:$qemu_error_funcs)\s*\(/) {
-			WARN("Error messages should not contain newlines\n" . $herecurr);
+			ERROR("Error messages should not contain newlines\n" . $herecurr);
 		}
 	}
 
-- 
1.8.3.1

[Qemu-devel] [PULL 14/14] checkpatch: default to success if only warnings

[Paolo Bonzini]

CHK-level checks have been removed from checkpatch or bumped to
errors, so there is no effect anymore for --strict/--subjective.
Furthermore, even most WARNs have been bumped to errors, with
WARN only reserved to things that patchew probably ought not
to complain about (and that maintainers probably will notice
anyway during review if they are extreme).

Default to exiting with success even if there are WARN-level
failures, and cause --strict to fail for warnings.  Maintainers
that want to have a strict 80-character limit for their subsystem
can add it to a commit hook for example.

The --subjective synonym is removed.

Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
---
 scripts/checkpatch.pl | 16 ++++------------
 1 file changed, 4 insertions(+), 12 deletions(-)

diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
index 714dbbe..b0096a4 100755
--- a/scripts/checkpatch.pl
+++ b/scripts/checkpatch.pl
@@ -22,7 +22,7 @@ my $tst_only;
 my $emacs = 0;
 my $terse = 0;
 my $file = 0;
-my $check = 0;
+my $no_warnings = 0;
 my $summary = 1;
 my $mailback = 0;
 my $summary_file = 0;
@@ -45,7 +45,7 @@ Options:
   --emacs                    emacs compile window format
   --terse                    one line per report
   -f, --file                 treat FILE as regular source file
-  --subjective, --strict     enable more subjective tests
+  --strict                   fail if only warnings are found
   --root=PATH                PATH to the kernel tree root
   --no-summary               suppress the per-file summary
   --mailback                 only produce a report in case of warnings/errors
@@ -71,8 +71,7 @@ GetOptions(
 	'emacs!'	=> \$emacs,
 	'terse!'	=> \$terse,
 	'f|file!'	=> \$file,
-	'subjective!'	=> \$check,
-	'strict!'	=> \$check,
+	'strict!'	=> \$no_warnings,
 	'root=s'	=> \$root,
 	'summary!'	=> \$summary,
 	'mailback!'	=> \$mailback,
@@ -1072,12 +1071,6 @@ sub WARN {
 		our $cnt_warn++;
 	}
 }
-sub CHK {
-	if ($check && report("CHECK: $_[0]\n")) {
-		our $clean = 0;
-		our $cnt_chk++;
-	}
-}
 
 sub process {
 	my $filename = shift;
@@ -2599,7 +2592,6 @@ sub process {
 	if ($summary && !($clean == 1 && $quiet == 1)) {
 		print "$filename " if ($summary_file);
 		print "total: $cnt_error errors, $cnt_warn warnings, " .
-			(($check)? "$cnt_chk checks, " : "") .
 			"$cnt_lines lines checked\n";
 		print "\n" if ($quiet == 0);
 	}
@@ -2622,5 +2614,5 @@ sub process {
 		print "CHECKPATCH in MAINTAINERS.\n";
 	}
 
-	return $clean;
+	return ($no_warnings ? $clean : $cnt_error == 0);
 }
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 00/14] checkpatch, warnings and other fixes

[Peter Maydell]

On 10 August 2016 at 14:57, Paolo Bonzini <pbonzini@redhat.com> wrote:
> The following changes since commit 2bb15bddf2607110820d5ce5aa43baac27292fb3:
>
>   Merge remote-tracking branch 'remotes/jnsnow/tags/ide-pull-request' into staging (2016-08-09 16:53:32 +0100)
>
> are available in the git repository at:
>
>
>   git://github.com/bonzini/qemu.git tags/for-upstream
>
> for you to fetch changes up to 141de8865488189ad9d75408b3e0ad24c6fff2bb:
>
>   checkpatch: default to success if only warnings (2016-08-10 12:44:51 +0200)
>
> ----------------------------------------------------------------
> * pc-bios/optionrom/Makefile fixes
> * warning fixes for __atomic_load and -1 << x in clang
> * missed interrupt fix from Gonglei
> * checkpatch fix from Radim and myself

Applied, thanks.

-- PMM