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

[Xingtao Yao (Fujitsu)]

> -----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
>