Re: [PATCH v2 00/48] Use g_assert_not_reached instead of (g_)assert(0, false)

[Pierrick Bouvier <pierrick.bouvier@linaro.org>]

On 9/12/24 18:37, Xingtao Yao (Fujitsu) wrote:
> 
> 
>> -----Original Message-----
>> From: qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org
>> <qemu-devel-bounces+yaoxt.fnst=fujitsu.com@nongnu.org> On Behalf Of
>> Pierrick Bouvier
>> Sent: Thursday, September 12, 2024 3:39 PM
>> To: qemu-devel@nongnu.org
>> Cc: Jason Wang <jasowang@redhat.com>; Alex Bennée <alex.bennee@linaro.org>;
>> Laurent Vivier <lvivier@redhat.com>; Marcelo Tosatti <mtosatti@redhat.com>;
>> Nicholas Piggin <npiggin@gmail.com>; Klaus Jensen <its@irrelevant.dk>; WANG
>> Xuerui <git@xen0n.name>; Halil Pasic <pasic@linux.ibm.com>; Rob Herring
>> <robh@kernel.org>; Michael Rolnik <mrolnik@gmail.com>; Zhao Liu
>> <zhao1.liu@intel.com>; Peter Maydell <peter.maydell@linaro.org>; Richard
>> Henderson <richard.henderson@linaro.org>; Fabiano Rosas <farosas@suse.de>;
>> Corey Minyard <minyard@acm.org>; Keith Busch <kbusch@kernel.org>; Thomas
>> Huth <thuth@redhat.com>; Maciej S. Szmigiero <maciej.szmigiero@oracle.com>;
>> Harsh Prateek Bora <harshpb@linux.ibm.com>; Kevin Wolf <kwolf@redhat.com>;
>> Paolo Bonzini <pbonzini@redhat.com>; Jesper Devantier <foss@defmacro.it>;
>> Hyman Huang <yong.huang@smartx.com>; Philippe Mathieu-Daudé
>> <philmd@linaro.org>; Palmer Dabbelt <palmer@dabbelt.com>;
>> qemu-s390x@nongnu.org; Laurent Vivier <laurent@vivier.eu>;
>> qemu-riscv@nongnu.org; Richard W.M. Jones <rjones@redhat.com>; Liu Zhiwei
>> <zhiwei_liu@linux.alibaba.com>; Aurelien Jarno <aurelien@aurel32.net>; Daniel P.
>> Berrangé <berrange@redhat.com>; Marcel Apfelbaum
>> <marcel.apfelbaum@gmail.com>; kvm@vger.kernel.org; Christian Borntraeger
>> <borntraeger@linux.ibm.com>; Akihiko Odaki <akihiko.odaki@daynix.com>;
>> Daniel Henrique Barboza <dbarboza@ventanamicro.com>; Hanna Reitz
>> <hreitz@redhat.com>; Ani Sinha <anisinha@redhat.com>;
>> qemu-ppc@nongnu.org; Marc-André Lureau <marcandre.lureau@redhat.com>;
>> Alistair Francis <alistair.francis@wdc.com>; Bin Meng <bmeng.cn@gmail.com>;
>> Michael S. Tsirkin <mst@redhat.com>; Helge Deller <deller@gmx.de>; Peter Xu
>> <peterx@redhat.com>; Daniel Henrique Barboza <danielhb413@gmail.com>;
>> Dmitry Fleytman <dmitry.fleytman@gmail.com>; Nina Schoetterl-Glausch
>> <nsg@linux.ibm.com>; Yanan Wang <wangyanan55@huawei.com>;
>> qemu-arm@nongnu.org; Igor Mammedov <imammedo@redhat.com>;
>> Jean-Christophe Dubois <jcd@tribudubois.net>; Eric Farman
>> <farman@linux.ibm.com>; Sriram Yagnaraman
>> <sriram.yagnaraman@ericsson.com>; qemu-block@nongnu.org; Stefan Berger
>> <stefanb@linux.vnet.ibm.com>; Joel Stanley <joel@jms.id.au>; Eduardo Habkost
>> <eduardo@habkost.net>; David Gibson <david@gibson.dropbear.id.au>; Fam
>> Zheng <fam@euphon.net>; Weiwei Li <liwei1518@gmail.com>; Markus
>> Armbruster <armbru@redhat.com>; Pierrick Bouvier <pierrick.bouvier@linaro.org>
>> Subject: [PATCH v2 00/48] Use g_assert_not_reached instead of (g_)assert(0,
>> false)
>>
>> This series cleans up all usages of assert/g_assert who are supposed to stop
>> execution of QEMU. We replace those by g_assert_not_reached().
>> It was suggested recently when cleaning codebase to build QEMU with gcc
>> and tsan:
>> https://lore.kernel.org/qemu-devel/54bb02a6-1b12-460a-97f6-3f478ef766c6@lin
>> aro.org/.
>>
>> In more, cleanup useless break and return after g_assert_not_reached();
> I found that not all of the break or return after g_assert_not_reached() were cleaned up, don't they need to be cleaned up?
>>
>> And finally, ensure with scripts/checkpatch.pl that we don't reintroduce
>> (g_)assert(false) in the future.
>>
>> New commits (removing return) need review.
>>
>> Tested that it build warning free with gcc and clang.
>>
>> v2
>> - align backslashes for some changes
>> - add summary in all commits message
>> - remove redundant comment
>>
>> v1
>> https://lore.kernel.org/qemu-devel/20240910221606.1817478-1-pierrick.bouvier@
>> linaro.org/T/#t
>>
>> Pierrick Bouvier (48):
>>    docs/spin: replace assert(0) with g_assert_not_reached()
>>    hw/acpi: replace assert(0) with g_assert_not_reached()
>>    hw/arm: replace assert(0) with g_assert_not_reached()
>>    hw/char: replace assert(0) with g_assert_not_reached()
>>    hw/core: replace assert(0) with g_assert_not_reached()
>>    hw/net: replace assert(0) with g_assert_not_reached()
>>    hw/watchdog: replace assert(0) with g_assert_not_reached()
>>    migration: replace assert(0) with g_assert_not_reached()
>>    qobject: replace assert(0) with g_assert_not_reached()
>>    system: replace assert(0) with g_assert_not_reached()
>>    target/ppc: replace assert(0) with g_assert_not_reached()
>>    tests/qtest: replace assert(0) with g_assert_not_reached()
>>    tests/unit: replace assert(0) with g_assert_not_reached()
>>    include/hw/s390x: replace assert(false) with g_assert_not_reached()
>>    block: replace assert(false) with g_assert_not_reached()
>>    hw/hyperv: replace assert(false) with g_assert_not_reached()
>>    hw/net: replace assert(false) with g_assert_not_reached()
>>    hw/nvme: replace assert(false) with g_assert_not_reached()
>>    hw/pci: replace assert(false) with g_assert_not_reached()
>>    hw/ppc: replace assert(false) with g_assert_not_reached()
>>    migration: replace assert(false) with g_assert_not_reached()
>>    target/i386/kvm: replace assert(false) with g_assert_not_reached()
>>    tests/qtest: replace assert(false) with g_assert_not_reached()
>>    accel/tcg: remove break after g_assert_not_reached()
>>    block: remove break after g_assert_not_reached()
>>    hw/acpi: remove break after g_assert_not_reached()
>>    hw/gpio: remove break after g_assert_not_reached()
>>    hw/misc: remove break after g_assert_not_reached()
>>    hw/net: remove break after g_assert_not_reached()
>>    hw/pci-host: remove break after g_assert_not_reached()
>>    hw/scsi: remove break after g_assert_not_reached()
>>    hw/tpm: remove break after g_assert_not_reached()
>>    target/arm: remove break after g_assert_not_reached()
>>    target/riscv: remove break after g_assert_not_reached()
>>    tests/qtest: remove break after g_assert_not_reached()
>>    ui: remove break after g_assert_not_reached()
>>    fpu: remove break after g_assert_not_reached()
>>    tcg/loongarch64: remove break after g_assert_not_reached()
>>    include/qemu: remove return after g_assert_not_reached()
>>    hw/hyperv: remove return after g_assert_not_reached()
>>    hw/net: remove return after g_assert_not_reached()
>>    hw/pci: remove return after g_assert_not_reached()
>>    hw/ppc: remove return after g_assert_not_reached()
>>    migration: remove return after g_assert_not_reached()
>>    qobject: remove return after g_assert_not_reached()
>>    qom: remove return after g_assert_not_reached()
>>    tests/qtest: remove return after g_assert_not_reached()
>>    scripts/checkpatch.pl: emit error when using assert(false)
>>
>>   docs/spin/aio_notify_accept.promela     |  6 +++---
>>   docs/spin/aio_notify_bug.promela        |  6 +++---
>>   include/hw/s390x/cpu-topology.h         |  2 +-
>>   include/qemu/pmem.h                     |  1 -
>>   accel/tcg/plugin-gen.c                  |  1 -
>>   block/qcow2.c                           |  2 +-
>>   block/ssh.c                             |  1 -
>>   hw/acpi/aml-build.c                     |  3 +--
>>   hw/arm/highbank.c                       |  2 +-
>>   hw/char/avr_usart.c                     |  2 +-
>>   hw/core/numa.c                          |  2 +-
>>   hw/gpio/nrf51_gpio.c                    |  1 -
>>   hw/hyperv/hyperv_testdev.c              |  7 +++----
>>   hw/hyperv/vmbus.c                       | 15 ++++++---------
>>   hw/misc/imx6_ccm.c                      |  1 -
>>   hw/misc/mac_via.c                       |  2 --
>>   hw/net/e1000e_core.c                    |  4 +---
>>   hw/net/i82596.c                         |  2 +-
>>   hw/net/igb_core.c                       |  4 +---
>>   hw/net/net_rx_pkt.c                     |  3 +--
>>   hw/net/vmxnet3.c                        |  1 -
>>   hw/nvme/ctrl.c                          |  8 ++++----
>>   hw/pci-host/gt64120.c                   |  2 --
>>   hw/pci/pci-stub.c                       |  6 ++----
>>   hw/ppc/ppc.c                            |  1 -
>>   hw/ppc/spapr_events.c                   |  3 +--
>>   hw/scsi/virtio-scsi.c                   |  1 -
>>   hw/tpm/tpm_spapr.c                      |  1 -
>>   hw/watchdog/watchdog.c                  |  2 +-
>>   migration/dirtyrate.c                   |  3 +--
>>   migration/migration-hmp-cmds.c          |  2 +-
>>   migration/postcopy-ram.c                | 21 +++++++--------------
>>   migration/ram.c                         |  8 +++-----
>>   qobject/qlit.c                          |  2 +-
>>   qobject/qnum.c                          | 12 ++++--------
>>   qom/object.c                            |  1 -
>>   system/rtc.c                            |  2 +-
>>   target/arm/hyp_gdbstub.c                |  1 -
>>   target/i386/kvm/kvm.c                   |  4 ++--
>>   target/ppc/dfp_helper.c                 |  8 ++++----
>>   target/ppc/mmu_helper.c                 |  2 +-
>>   target/riscv/monitor.c                  |  1 -
>>   tests/qtest/acpi-utils.c                |  1 -
>>   tests/qtest/ipmi-bt-test.c              |  2 +-
>>   tests/qtest/ipmi-kcs-test.c             |  4 ++--
>>   tests/qtest/migration-helpers.c         |  1 -
>>   tests/qtest/numa-test.c                 | 10 +++++-----
>>   tests/qtest/rtl8139-test.c              |  2 +-
>>   tests/unit/test-xs-node.c               |  4 ++--
>>   ui/qemu-pixman.c                        |  1 -
>>   fpu/softfloat-parts.c.inc               |  2 --
>>   target/riscv/insn_trans/trans_rvv.c.inc |  2 --
>>   tcg/loongarch64/tcg-target.c.inc        |  1 -
>>   scripts/checkpatch.pl                   |  3 +++
>>   54 files changed, 72 insertions(+), 120 deletions(-)
>>
>> --
>> 2.39.2
>>
> 

Which one did you find? Using which grep command?