Re: [Cocci] [RFC] treewide: cleanup unreachable breaks

2020-10-17 Thread Julia Lawall



On Sat, 17 Oct 2020, Joe Perches wrote:

> On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote:
> > From: Tom Rix 
> >
> > This is a upcoming change to clean up a new warning treewide.
> > I am wondering if the change could be one mega patch (see below) or
> > normal patch per file about 100 patches or somewhere half way by collecting
> > early acks.
> >
> > clang has a number of useful, new warnings see
> > https://clang.llvm.org/docs/DiagnosticsReference.html
> >
> > This change cleans up -Wunreachable-code-break
> > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
> > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
>
> Early acks/individual patches by subsystem would be good.
> Better still would be an automated cocci script.

Coccinelle is not especially good at this, because it is based on control
flow, and a return or goto diverts the control flow away from the break.
A hack to solve the problem is to put an if around the return or goto, but
that gives the break a meaningless file name and line number.  I collected
the following list, but it only has 439 results, so fewer than clang.  But
maybe there are some files that are not considered by clang in the x86
allyesconfig configuration.

Probably checkpatch is the best solution here, since it is not
configuration sensitive and doesn't care about control flow.

julia

drivers/scsi/mvumi.c: function mvumi_cfg_hw_reg line 114
drivers/watchdog/geodewdt.c: function geodewdt_ioctl line 18
drivers/media/usb/b2c2/flexcop-usb.c: function flexcop_usb_init line 21
drivers/media/usb/b2c2/flexcop-usb.c: function flexcop_usb_memory_req line 20
drivers/tty/nozomi.c: function write_mem32 line 17
drivers/tty/nozomi.c: function write_mem32 line 25
drivers/tty/nozomi.c: function read_mem32 line 17
drivers/tty/nozomi.c: function read_mem32 line 21
sound/soc/codecs/wl1273.c: function wl1273_startup line 27
drivers/iio/adc/meson_saradc.c: function meson_sar_adc_iio_info_read_raw line 12
drivers/iio/adc/meson_saradc.c: function meson_sar_adc_iio_info_read_raw line 19
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 6
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 9
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 12
drivers/gpu/drm/amd/display/dc/dcn10/dcn10_resource.c: function 
map_transmitter_id_to_phy_instance line 15
drivers/media/tuners/mt2063.c: function mt2063_init line 81
drivers/nfc/st21nfca/core.c: function st21nfca_hci_im_transceive line 46
arch/sh/boards/mach-landisk/gio.c: function gio_ioctl line 53
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 11
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 15
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 18
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 22
drivers/gpu/drm/mgag200/mgag200_mode.c: function mgag200_crtc_set_plls line 25
drivers/media/dvb-frontends/cx24117.c: function cx24117_attach line 16
drivers/block/xen-blkback/blkback.c: function dispatch_rw_block_io line 48
drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get 
line 16
drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get 
line 22
drivers/platform/x86/sony-laptop.c: function __sony_nc_gfx_switch_status_get 
line 31
drivers/char/mwave/mwavedd.c: function mwave_ioctl line 288
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 15
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 19
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 22
drivers/scsi/be2iscsi/be_mgmt.c: function beiscsi_adap_family_disp line 27
drivers/iio/imu/bmi160/bmi160_core.c: function bmi160_write_raw line 11
drivers/block/z2ram.c: function z2_open line 138
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/hw.c: function 
_rtl8723e_set_media_status line 38
samples/hidraw/hid-example.c: function bus_str line 6
samples/hidraw/hid-example.c: function bus_str line 9
samples/hidraw/hid-example.c: function bus_str line 12
samples/hidraw/hid-example.c: function bus_str line 15
samples/hidraw/hid-example.c: function bus_str line 18
drivers/scsi/ipr.c: function ipr_pci_error_detected line 10
drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 11
drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 17
drivers/gpio/gpio-bd70528.c: function bd70528_gpio_set_config line 21
drivers/pinctrl/pinctrl-rockchip.c: function rockchip_pinconf_get line 71
drivers/pinctrl/pinctrl-rockchip.c: function rockchip_pinconf_set line 74
drivers/gpu/drm/amd/display/include/signal_types.h: function dc_is_dvi_signal 
line 6
security/keys/trusted-keys/trusted_tpm1.c: function datablob_parse line 63
arch/x86/math-emu/fpu_trig.c: function 

Re: [Cocci] [RFC] treewide: cleanup unreachable breaks

2020-10-17 Thread Joe Perches
On Sat, 2020-10-17 at 20:21 +0200, Julia Lawall wrote:
> On Sat, 17 Oct 2020, Joe Perches wrote:
> > On Sat, 2020-10-17 at 09:09 -0700, t...@redhat.com wrote:
> > > From: Tom Rix 
> > > 
> > > This is a upcoming change to clean up a new warning treewide.
> > > I am wondering if the change could be one mega patch (see below) or
> > > normal patch per file about 100 patches or somewhere half way by 
> > > collecting
> > > early acks.
> > > 
> > > clang has a number of useful, new warnings see
> > > https://clang.llvm.org/docs/DiagnosticsReference.html
> > > 
> > > This change cleans up -Wunreachable-code-break
> > > https://clang.llvm.org/docs/DiagnosticsReference.html#wunreachable-code-break
> > > for 266 of 485 warnings in this week's linux-next, allyesconfig on x86_64.
> > 
> > Early acks/individual patches by subsystem would be good.
> > Better still would be an automated cocci script.
> 
> Coccinelle is not especially good at this, because it is based on control
> flow, and a return or goto diverts the control flow away from the break.
> A hack to solve the problem is to put an if around the return or goto, but
> that gives the break a meaningless file name and line number.  I collected
> the following list, but it only has 439 results, so fewer than clang.  But
> maybe there are some files that are not considered by clang in the x86
> allyesconfig configuration.
> 
> Probably checkpatch is the best solution here, since it is not
> configuration sensitive and doesn't care about control flow.

Likely the clang compiler is the best option here.

It might be useful to add -Wunreachable-code-break to W=1
or just always enable it if it isn't already enabled.

diff --git a/scripts/Makefile.extrawarn b/scripts/Makefile.extrawarn
index 95e4cdb94fe9..3819787579d5 100644
--- a/scripts/Makefile.extrawarn
+++ b/scripts/Makefile.extrawarn
@@ -32,6 +32,7 @@ KBUILD_CFLAGS += $(call cc-option, -Wunused-but-set-variable)
 KBUILD_CFLAGS += $(call cc-option, -Wunused-const-variable)
 KBUILD_CFLAGS += $(call cc-option, -Wpacked-not-aligned)
 KBUILD_CFLAGS += $(call cc-option, -Wstringop-truncation)
+KBUILD_CFLAGS += $(call cc-option, -Wunreachable-code-break)
 # The following turn off the warnings enabled by -Wextra
 KBUILD_CFLAGS += -Wno-missing-field-initializers
 KBUILD_CFLAGS += -Wno-sign-compare

(and thank you Tom for pushing this forward)

checkpatch can't find instances like:

case FOO:
if (foo)
return 1;
else
return 2;
break;

As it doesn't track flow and relies on the number
of tabs to be the same for any goto/return and break;

checkpatch will warn on:

case FOO:
...
goto bar;
break;


___
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel