accel/accel.c | 20 +++--- chardev/char-fd.c | 1 - chardev/char.c | 2 +- exec.c | 12 ++-- hw/bt/sdp.c | 17 +++-- hw/scsi/vmw_pvscsi.c | 12 +--- hw/timer/mc146818rtc.c | 37 +++++----- include/chardev/char-fd.h | 2 +- include/chardev/char.h | 4 +- include/exec/ram_addr.h | 7 +- memory.c | 36 +++++++++- qemu-doc.texi | 175 ++++++++++++++++++++++++++++++++++++++++++++++ qemu-options.hx | 15 +++- target/i386/kvm.c | 35 ++++++---- tests/rtc-test.c | 156 +++++++++++++++++++++++++++++++++-------- vl.c | 12 ++-- 16 files changed, 437 insertions(+), 106 deletions(-)
The following changes since commit 7d48cf8102a10e4a54333811bafb5eb566509268: Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-08-01 14:33:56 +0100) are available in the git repository at: git://github.com/bonzini/qemu.git tags/for-upstream for you to fetch changes up to 33f21e4f044ac1c37f60edc1f1aee628be8f463b: mc146818rtc: implement UIP latching as intended (2017-08-01 17:27:34 +0200) ---------------------------------------------------------------- * Xen fix (Anthony) * chardev fixes (Anton, Marc-André) * small dead code removal (Zhongyi) * documentation (Dan) * bugfixes (David) * decrease migration downtime (Jay) * improved error output (Laurent) * RTC tests and bugfix (me) * Bluetooth clang analyzer fix (me) * KVM CPU hotplug race (Peng Hao) * Two other patches from Philippe's clang analyzer series ---------------------------------------------------------------- Anthony PERARD (1): exec: Add lock parameter to qemu_ram_ptr_length Anton Nefedov (1): char: don't exit on hmp 'chardev-add help' Daniel P. Berrange (2): docs: document deprecation policy & deprecated features in appendix qemu-options: document existance of versioned machine types Dr. David Alan Gilbert (2): vl.c/exit: pause cpus before closing block devices cpu_physical_memory_sync_dirty_bitmap: Fix alignment check Jay Zhou (1): migration: optimize the downtime Laurent Vivier (1): accel: cleanup error output Mao Zhongyi (2): hw/scsi/vmw_pvscsi: Remove the dead error handling hw/scsi/vmw_pvscsi: Convert to realize Marc-André Lureau (1): char-fd: remove useless chr pointer Paolo Bonzini (5): bt: stop the sdp memory allocation craziness rtc-test: cleanup register_b_set_flag test rtc-test: introduce more update tests mc146818rtc: simplify check_update_timer mc146818rtc: implement UIP latching as intended Peng Hao (1): target-i386: kvm_get/put_vcpu_events don't handle sipi_vector accel/accel.c | 20 +++--- chardev/char-fd.c | 1 - chardev/char.c | 2 +- exec.c | 12 ++-- hw/bt/sdp.c | 17 +++-- hw/scsi/vmw_pvscsi.c | 12 +--- hw/timer/mc146818rtc.c | 37 +++++----- include/chardev/char-fd.h | 2 +- include/chardev/char.h | 4 +- include/exec/ram_addr.h | 7 +- memory.c | 36 +++++++++- qemu-doc.texi | 175 ++++++++++++++++++++++++++++++++++++++++++++++ qemu-options.hx | 15 +++- target/i386/kvm.c | 35 ++++++---- tests/rtc-test.c | 156 +++++++++++++++++++++++++++++++++-------- vl.c | 12 ++-- 16 files changed, 437 insertions(+), 106 deletions(-) -- 1.8.3.1
Hi, This series seems to have some coding style problems. See output below for more information: Subject: [Qemu-devel] [PULL 00/17] Misc changes for QEMU 2.10-rc1 (?) Message-id: 1501604245-33460-1-git-send-email-pbonzini@redhat.com Type: series === 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 Switched to a new branch 'test' 3489547b43 mc146818rtc: implement UIP latching as intended ed88cc342f mc146818rtc: simplify check_update_timer 818434f924 rtc-test: introduce more update tests d2208f1403 rtc-test: cleanup register_b_set_flag test acd549e69e hw/scsi/vmw_pvscsi: Convert to realize 07345966fe hw/scsi/vmw_pvscsi: Remove the dead error handling 7651c26752 migration: optimize the downtime c24276b69b qemu-options: document existance of versioned machine types 6240b43885 bt: stop the sdp memory allocation craziness 8e3ab7f33e exec: Add lock parameter to qemu_ram_ptr_length c2d7f542e9 target-i386: kvm_get/put_vcpu_events don't handle sipi_vector caaa83b53e docs: document deprecation policy & deprecated features in appendix e085726add char: don't exit on hmp 'chardev-add help' b9ba7ecb28 char-fd: remove useless chr pointer ee56cb9f35 accel: cleanup error output 235bccb31d cpu_physical_memory_sync_dirty_bitmap: Fix alignment check 99135ab7e2 vl.c/exit: pause cpus before closing block devices === OUTPUT BEGIN === Checking PATCH 1/17: vl.c/exit: pause cpus before closing block devices... Checking PATCH 2/17: cpu_physical_memory_sync_dirty_bitmap: Fix alignment check... Checking PATCH 3/17: accel: cleanup error output... Checking PATCH 4/17: char-fd: remove useless chr pointer... Checking PATCH 5/17: char: don't exit on hmp 'chardev-add help'... Checking PATCH 6/17: docs: document deprecation policy & deprecated features in appendix... Checking PATCH 7/17: target-i386: kvm_get/put_vcpu_events don't handle sipi_vector... Checking PATCH 8/17: exec: Add lock parameter to qemu_ram_ptr_length... Checking PATCH 9/17: bt: stop the sdp memory allocation craziness... ERROR: space prohibited before that '++' (ctx:WxB) #78: FILE: hw/bt/sdp.c:741: + data[len ++] = attribute_id >> 8; ^ ERROR: space prohibited before that '++' (ctx:WxB) #79: FILE: hw/bt/sdp.c:742: + data[len ++] = attribute_id & 0xff; ^ total: 2 errors, 0 warnings, 48 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 10/17: qemu-options: document existance of versioned machine types... Checking PATCH 11/17: migration: optimize the downtime... Checking PATCH 12/17: hw/scsi/vmw_pvscsi: Remove the dead error handling... Checking PATCH 13/17: hw/scsi/vmw_pvscsi: Convert to realize... Checking PATCH 14/17: rtc-test: cleanup register_b_set_flag test... ERROR: space required before the open parenthesis '(' #73: FILE: tests/rtc-test.c:344: + } while(0) total: 1 errors, 0 warnings, 115 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 15/17: rtc-test: introduce more update tests... Checking PATCH 16/17: mc146818rtc: simplify check_update_timer... Checking PATCH 17/17: mc146818rtc: implement UIP latching as intended... === 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
On 01/08/2017 18:48, no-reply@patchew.org wrote: > === OUTPUT BEGIN === > Checking PATCH 1/17: vl.c/exit: pause cpus before closing block devices... > Checking PATCH 2/17: cpu_physical_memory_sync_dirty_bitmap: Fix alignment check... > Checking PATCH 3/17: accel: cleanup error output... > Checking PATCH 4/17: char-fd: remove useless chr pointer... > Checking PATCH 5/17: char: don't exit on hmp 'chardev-add help'... > Checking PATCH 6/17: docs: document deprecation policy & deprecated features in appendix... > Checking PATCH 7/17: target-i386: kvm_get/put_vcpu_events don't handle sipi_vector... > Checking PATCH 8/17: exec: Add lock parameter to qemu_ram_ptr_length... > Checking PATCH 9/17: bt: stop the sdp memory allocation craziness... > ERROR: space prohibited before that '++' (ctx:WxB) > #78: FILE: hw/bt/sdp.c:741: > + data[len ++] = attribute_id >> 8; > ^ > > ERROR: space prohibited before that '++' (ctx:WxB) > #79: FILE: hw/bt/sdp.c:742: > + data[len ++] = attribute_id & 0xff; This is the preexisting Bluetooth code... I didn't change the space, should I have done that? > 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 10/17: qemu-options: document existance of versioned machine types... > Checking PATCH 11/17: migration: optimize the downtime... > Checking PATCH 12/17: hw/scsi/vmw_pvscsi: Remove the dead error handling... > Checking PATCH 13/17: hw/scsi/vmw_pvscsi: Convert to realize... > Checking PATCH 14/17: rtc-test: cleanup register_b_set_flag test... > ERROR: space required before the open parenthesis '(' > #73: FILE: tests/rtc-test.c:344: > + } while(0) This seems to be more common than "while (0)" inside macros, should we allow it in checkpatch.pl? Paolo > total: 1 errors, 0 warnings, 115 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 15/17: rtc-test: introduce more update tests... > Checking PATCH 16/17: mc146818rtc: simplify check_update_timer... > Checking PATCH 17/17: mc146818rtc: implement UIP latching as intended... > === 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 >
On 1 August 2017 at 17:50, Paolo Bonzini <pbonzini@redhat.com> wrote: > On 01/08/2017 18:48, no-reply@patchew.org wrote: >> ERROR: space prohibited before that '++' (ctx:WxB) >> #78: FILE: hw/bt/sdp.c:741: >> + data[len ++] = attribute_id >> 8; >> ^ >> >> ERROR: space prohibited before that '++' (ctx:WxB) >> #79: FILE: hw/bt/sdp.c:742: >> + data[len ++] = attribute_id & 0xff; > > This is the preexisting Bluetooth code... I didn't change the space, > should I have done that? Judgement call -- I usually fix up existing errors if I'm touching a bit of code anyway, unless it's a whitespace-only change or a pure code-motion patch. >> ERROR: space required before the open parenthesis '(' >> #73: FILE: tests/rtc-test.c:344: >> + } while(0) > > This seems to be more common than "while (0)" inside macros, should we > allow it in checkpatch.pl? Overall the space is much more common: 551 examples with the space vs 90 without; so I don't think a relaxation of checkpatch is particularly justified. I don't think macros need to be any different from the rest of our code on things like spacing. thanks -- PMM
On 01/08/2017 19:10, Peter Maydell wrote: > On 1 August 2017 at 17:50, Paolo Bonzini <pbonzini@redhat.com> wrote: >> On 01/08/2017 18:48, no-reply@patchew.org wrote: >>> ERROR: space prohibited before that '++' (ctx:WxB) >>> #78: FILE: hw/bt/sdp.c:741: >>> + data[len ++] = attribute_id >> 8; >>> ^ >>> >>> ERROR: space prohibited before that '++' (ctx:WxB) >>> #79: FILE: hw/bt/sdp.c:742: >>> + data[len ++] = attribute_id & 0xff; >> >> This is the preexisting Bluetooth code... I didn't change the space, >> should I have done that? > > Judgement call -- I usually fix up existing errors if I'm touching > a bit of code anyway, unless it's a whitespace-only change or a > pure code-motion patch. Me too. In this case however the code is pretty much untouched so it's unlikely that it would become consistent one day (and I suspect no one wants to get on git blame for bluetooth emulation :)). >>> ERROR: space required before the open parenthesis '(' >>> #73: FILE: tests/rtc-test.c:344: >>> + } while(0) >> >> This seems to be more common than "while (0)" inside macros, should we >> allow it in checkpatch.pl? > > Overall the space is much more common: 551 examples with the > space vs 90 without; so I don't think a relaxation of checkpatch > is particularly justified. I don't think macros need to be any > different from the rest of our code on things like spacing. Ok, for this patch I kept it consistent within the file, but for 2.11 we can change everything to "while (0)". Paolo
On 1 August 2017 at 18:17, Paolo Bonzini <pbonzini@redhat.com> wrote: > Ok, for this patch I kept it consistent within the file, but for 2.11 we > can change everything to "while (0)". Is this a proposal to bite the bullet and fix all our code style issues in existing code? :-) It would be an interesting question to find out what % of QEMU's lines of code have not been touched in the six years since we added checkpatch, and how they're distributed in the codebase (are there any neat visualisations of this sort of thing?) Fix-when-touched works OK where we're actively working but can leave some big dead spots I suspect. thanks -- PMM
On 01/08/2017 19:22, Peter Maydell wrote: > On 1 August 2017 at 18:17, Paolo Bonzini <pbonzini@redhat.com> wrote: >> Ok, for this patch I kept it consistent within the file, but for 2.11 we >> can change everything to "while (0)". > > Is this a proposal to bite the bullet and fix all our > code style issues in existing code? :-) Only "while (0)" :) > It would be an interesting question to find out what % of > QEMU's lines of code have not been touched in the six > years since we added checkpatch, and how they're > distributed in the codebase (are there any neat > visualisations of this sort of thing?) Fix-when-touched > works OK where we're actively working but can leave > some big dead spots I suspect. That's an interesting idea. Since I've been volunteered to present about QEMU at KVM Forum, I might give it a shot. Paolo
On 1 August 2017 at 17:17, Paolo Bonzini <pbonzini@redhat.com> wrote: > The following changes since commit 7d48cf8102a10e4a54333811bafb5eb566509268: > > Merge remote-tracking branch 'remotes/stefanha/tags/tracing-pull-request' into staging (2017-08-01 14:33:56 +0100) > > are available in the git repository at: > > > git://github.com/bonzini/qemu.git tags/for-upstream > > for you to fetch changes up to 33f21e4f044ac1c37f60edc1f1aee628be8f463b: > > mc146818rtc: implement UIP latching as intended (2017-08-01 17:27:34 +0200) > > ---------------------------------------------------------------- > * Xen fix (Anthony) > * chardev fixes (Anton, Marc-André) > * small dead code removal (Zhongyi) > * documentation (Dan) > * bugfixes (David) > * decrease migration downtime (Jay) > * improved error output (Laurent) > * RTC tests and bugfix (me) > * Bluetooth clang analyzer fix (me) > * KVM CPU hotplug race (Peng Hao) > * Two other patches from Philippe's clang analyzer series > Applied, thanks, but can you check whatever it is in your workflow that's mangling non-ascii characters? thanks -- PMM
> > ---------------------------------------------------------------- > > * Xen fix (Anthony) > > * chardev fixes (Anton, Marc-André) > > * small dead code removal (Zhongyi) > > * documentation (Dan) > > * bugfixes (David) > > * decrease migration downtime (Jay) > > * improved error output (Laurent) > > * RTC tests and bugfix (me) > > * Bluetooth clang analyzer fix (me) > > * KVM CPU hotplug race (Peng Hao) > > * Two other patches from Philippe's clang analyzer series > > > > Applied, thanks, but can you check whatever it is in your > workflow that's mangling non-ascii characters? It's https://bugzilla.mozilla.org/show_bug.cgi?id=1017768. Usually I apply patches with patchew, which bypasses that bug. Paolo
© 2016 - 2024 Red Hat, Inc.