* [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
@ 2016-07-22 9:05 ` Nicholas Mc Guire
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Mc Guire @ 2016-07-22 9:05 UTC (permalink / raw)
To: Julia Lawall
Cc: Gilles Muller, Nicolas Palix, Michal Marek, cocci, linux-kernel,
Nicholas Mc Guire
Issue a warning if the if and else branch are identical - this can deliver
false positives as such constructs are sometime legitimate. In such cases
they should be documented so detecting false positives should be trivial
and if not it is a doc bug.
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
condition_with_no_effect.cocci was tested with
spatch version 1.0.5 compiled with OCaml version 4.01.0
Flags passed to the configure script: [none]
Python scripting support: yes
Syntax of regular expressions: PCRE
Some details:
In the kernel there are a number of cases where the if branch and the else
branch are identical code. Looking through the roughly 100 cases some do
seem legitimate (documented cases) but most seem to be bugs - code-bugs or
doc bugs. Below are some as examples followed by the full list.
So the question is - should this case be mentioned in CodingStyle or maybe
in SubmittingPatches (for the case of not-yet-implemented cases) ? That it
should be avoided if possible, I guess, is clear but given that its not that
uncommon it might need explicit treatment.
Properly documented cases:
./arch/sh/kernel/traps_64.c:59 positional side effect in use
./fs/kernfs/file.c:668 not yet implemented behavior
./drivers/media/platform/arv.c:221 clearly marked as intended default
./drivers/net/wireless/ray_cs.c:2126 ...or at least has a TBD description
Some of the undocumented cases are probably simply intended "defaults"
which may well be reasonable but simply lack the documentation like
drivers/video/fbdev/sis/init301.c:7968
if(resindex == 0x04) {
SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
} else {
SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
}
And some do carry some sort of documentation but of really very limited help...
drivers/net/wireless/broadcom/b43/xmit.c:187
if (phy->type == B43_PHYTYPE_LP)
bw = B43_TXH_PHY1_BW_20;
else /* FIXME */
bw = B43_TXH_PHY1_BW_20;
Where documentation indicates a difference but code does not this is IMHO a bug
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
if (BTC_WIFI_BW_LEGACY == wifi_bw) {
/* for HID at 11b/g mode */
halbtc8821a2ant_coex_table(btcoexist, NORMAL_EXEC, 0x55ff55ff,
0x5a5a5a5a, 0xffff, 0x3);
} else {
/* for HID quality & wifi performance balance at 11n mode */
halbtc8821a2ant_coex_table(btcoexist, NORMAL_EXEC, 0x55ff55ff,
0x5a5a5a5a, 0xffff, 0x3);
}
Where there is no documentation but it also does not make sense as a default
it also is to qualified as a bug
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:694
case IEEE80211_STYPE_PROBE_REQ:
if (check_fwstate(pmlmepriv, WIFI_AP_STATE))
_mgt_dispatcher23a(padapter, ptable, precv_frame);
else
_mgt_dispatcher23a(padapter, ptable, precv_frame);
break;
The most impressive case of identical nested if-else is to be enjoyed in
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c starting
at line 2982.
finally the list of possible bugs / documentation omissions is as of -next 4.7-rc7:
./drivers/video/fbdev/sis/init301.c:9660
./drivers/video/fbdev/sis/init301.c:7968
./drivers/video/fbdev/sis/init301.c:6851
./drivers/net/wireless/realtek/rtlwifi/base.c:822
./drivers/net/wireless/realtek/rtlwifi/base.c:834
./drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c:886
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2184
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2206
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2230
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2252
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2105
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2127
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2151
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2173
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c:1838
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c:2213
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2984
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2984
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2986
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2996
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3024
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3034
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2097
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2119
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2143
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2165
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3090
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3090
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3092
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3102
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3128
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3128
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3130
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3141
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2626
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2796
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2806
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2834
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2844
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2889
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2737
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2340
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2366
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:1729
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:2081
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:225
./drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c:1378
./drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c:3570
./drivers/net/wireless/realtek/rtlwifi/rtl8192c/dm_common.c:1686
./drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c:3391 *
./drivers/net/wireless/broadcom/b43/xmit.c:187
./drivers/net/wireless/broadcom/b43/phy_n.c:4617
./drivers/net/wireless/broadcom/b43/phy_n.c:4617
./drivers/net/wireless/broadcom/b43/phy_n.c:4650
./drivers/net/irda/via-ircc.c:598
./drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:489
./drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:500
./drivers/net/ethernet/nvidia/forcedeth.c:3392
./drivers/net/ethernet/qlogic/qlge/qlge_dbg.c:147
./drivers/infiniband/core/cm.c:522
./drivers/infiniband/core/cm.c:493
./drivers/infiniband/hw/i40iw/i40iw_virtchnl.c:434
./drivers/usb/isp1760/isp1760-hcd.c:1036
./drivers/usb/misc/ftdi-elan.c:1007
./drivers/usb/misc/ftdi-elan.c:1007
./drivers/usb/misc/ftdi-elan.c:1018
./drivers/usb/misc/ftdi-elan.c:2091
./drivers/usb/misc/ftdi-elan.c:898
./drivers/misc/vmw_vmci/vmci_queue_pair.c:2232
./drivers/staging/comedi/drivers/das1800.c:1311
./drivers/staging/rtl8723au/core/rtw_mlme_ext.c:694
./drivers/staging/rtl8723au/hal/odm.c:519
./drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:1389
./drivers/staging/xgifb/vb_setmode.c:2574
./drivers/mmc/host/vub300.c:643
./drivers/pcmcia/pxa2xx_trizeps4.c:64
./drivers/scsi/dc395x.c:3387
./drivers/scsi/pmcraid.c:1604
./drivers/media/pci/bt8xx/dvb-bt8xx.c:393
./drivers/media/usb/dvb-usb/dib0700_devices.c:1739
./drivers/media/usb/s2255/s2255drv.c:811
./drivers/media/usb/s2255/s2255drv.c:828
./drivers/media/dvb-frontends/dib0090.c:2443
./drivers/media/dvb-frontends/mb86a16.c:1479
./drivers/media/dvb-frontends/au8522_decoder.c:327
./drivers/gpu/drm/exynos/exynos_mixer.c:398
./drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1134
.../tests/condition_with_no_effect.cocci | 60 ++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 scripts/coccinelle/tests/condition_with_no_effect.cocci
diff --git a/scripts/coccinelle/tests/condition_with_no_effect.cocci b/scripts/coccinelle/tests/condition_with_no_effect.cocci
new file mode 100644
index 0000000..f910114
--- /dev/null
+++ b/scripts/coccinelle/tests/condition_with_no_effect.cocci
@@ -0,0 +1,60 @@
+///Find conditions where if and else branch match
+// There can be false positives in cases where the positional
+// information is used (as with lockdep) or where the identity
+// is a placeholder for not yet handled cases.
+//
+// In the Linux kernel though it did not seem to actually report
+// false positives except for those that that were documented as
+// being intentional.
+// the two known cases are:
+// arch/sh/kernel/traps_64.c:read_opcode()
+// } else if ((pc & 1) == 0) {
+// /* SHcompact */
+// /* TODO : provide handling for this. We don't really support
+// user-mode SHcompact yet, and for a kernel fault, this would
+// have to come from a module built for SHcompact. */
+// return -EFAULT;
+// } else {
+// /* misaligned */
+// return -EFAULT;
+// }
+// fs/kernfs/file.c:kernfs_fop_open()
+// * Both paths of the branch look the same. They're supposed to
+// * look that way and give @of->mutex different static lockdep keys.
+// */
+// if (has_mmap)
+// mutex_init(&of->mutex);
+// else
+// mutex_init(&of->mutex);
+//
+// All other cases look like bugs or at least lack of documentation
+//
+// Confidence: Moderate
+// Copyright: (C) 2016 Nicholas Mc Guire, OSADL. GPLv2.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual context
+virtual org
+virtual report
+
+@cond@
+statement S1;
+position p;
+@@
+
+<+...
+* if@p (...) S1 else S1
+...+>
+
+@script:python depends on org@
+p << cond.p;
+@@
+
+cocci.print_main("WARNING: possible condition with no effect (if == else)",p)
+
+@script:python depends on report@
+p << cond.p;
+@@
+
+coccilib.report.print_report(p[0],"WARNING: possible condition with no effect (if == else)")
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* [Cocci] [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
@ 2016-07-22 9:05 ` Nicholas Mc Guire
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Mc Guire @ 2016-07-22 9:05 UTC (permalink / raw)
To: cocci
Issue a warning if the if and else branch are identical - this can deliver
false positives as such constructs are sometime legitimate. In such cases
they should be documented so detecting false positives should be trivial
and if not it is a doc bug.
Signed-off-by: Nicholas Mc Guire <hofrat@osadl.org>
---
condition_with_no_effect.cocci was tested with
spatch version 1.0.5 compiled with OCaml version 4.01.0
Flags passed to the configure script: [none]
Python scripting support: yes
Syntax of regular expressions: PCRE
Some details:
In the kernel there are a number of cases where the if branch and the else
branch are identical code. Looking through the roughly 100 cases some do
seem legitimate (documented cases) but most seem to be bugs - code-bugs or
doc bugs. Below are some as examples followed by the full list.
So the question is - should this case be mentioned in CodingStyle or maybe
in SubmittingPatches (for the case of not-yet-implemented cases) ? That it
should be avoided if possible, I guess, is clear but given that its not that
uncommon it might need explicit treatment.
Properly documented cases:
./arch/sh/kernel/traps_64.c:59 positional side effect in use
./fs/kernfs/file.c:668 not yet implemented behavior
./drivers/media/platform/arv.c:221 clearly marked as intended default
./drivers/net/wireless/ray_cs.c:2126 ...or at least has a TBD description
Some of the undocumented cases are probably simply intended "defaults"
which may well be reasonable but simply lack the documentation like
drivers/video/fbdev/sis/init301.c:7968
if(resindex == 0x04) {
SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
} else {
SiS_SetCH70xxANDOR(SiS_Pr,0x20,0x00,0xEF); /* loop filter off */
SiS_SetCH70xxANDOR(SiS_Pr,0x21,0x01,0xFE); /* ACIV on */
}
And some do carry some sort of documentation but of really very limited help...
drivers/net/wireless/broadcom/b43/xmit.c:187
if (phy->type == B43_PHYTYPE_LP)
bw = B43_TXH_PHY1_BW_20;
else /* FIXME */
bw = B43_TXH_PHY1_BW_20;
Where documentation indicates a difference but code does not this is IMHO a bug
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
if (BTC_WIFI_BW_LEGACY == wifi_bw) {
/* for HID at 11b/g mode */
halbtc8821a2ant_coex_table(btcoexist, NORMAL_EXEC, 0x55ff55ff,
0x5a5a5a5a, 0xffff, 0x3);
} else {
/* for HID quality & wifi performance balance at 11n mode */
halbtc8821a2ant_coex_table(btcoexist, NORMAL_EXEC, 0x55ff55ff,
0x5a5a5a5a, 0xffff, 0x3);
}
Where there is no documentation but it also does not make sense as a default
it also is to qualified as a bug
drivers/staging/rtl8723au/core/rtw_mlme_ext.c:694
case IEEE80211_STYPE_PROBE_REQ:
if (check_fwstate(pmlmepriv, WIFI_AP_STATE))
_mgt_dispatcher23a(padapter, ptable, precv_frame);
else
_mgt_dispatcher23a(padapter, ptable, precv_frame);
break;
The most impressive case of identical nested if-else is to be enjoyed in
drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c starting
at line 2982.
finally the list of possible bugs / documentation omissions is as of -next 4.7-rc7:
./drivers/video/fbdev/sis/init301.c:9660
./drivers/video/fbdev/sis/init301.c:7968
./drivers/video/fbdev/sis/init301.c:6851
./drivers/net/wireless/realtek/rtlwifi/base.c:822
./drivers/net/wireless/realtek/rtlwifi/base.c:834
./drivers/net/wireless/realtek/rtlwifi/rtl8723be/dm.c:886
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2184
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2206
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2230
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8192e2ant.c:2252
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2105
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2127
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2151
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b2ant.c:2173
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c:1838
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8723b1ant.c:2213
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2972
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2984
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2984
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2986
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2996
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3024
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3034
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2097
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2119
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2143
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2165
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3078
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3090
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3090
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3092
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3102
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3128
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3128
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3130
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:3141
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2626
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2782
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2796
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2806
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2834
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2844
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2889
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2737
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2340
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a2ant.c:2366
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:1729
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtc8821a1ant.c:2081
./drivers/net/wireless/realtek/rtlwifi/btcoexist/halbtcoutsrc.c:225
./drivers/net/wireless/realtek/rtlwifi/rtl8192se/hw.c:1378
./drivers/net/wireless/realtek/rtlwifi/rtl8821ae/phy.c:3570
./drivers/net/wireless/realtek/rtlwifi/rtl8192c/dm_common.c:1686
./drivers/net/wireless/broadcom/brcm80211/brcmsmac/phy/phy_lcn.c:3391 *
./drivers/net/wireless/broadcom/b43/xmit.c:187
./drivers/net/wireless/broadcom/b43/phy_n.c:4617
./drivers/net/wireless/broadcom/b43/phy_n.c:4617
./drivers/net/wireless/broadcom/b43/phy_n.c:4650
./drivers/net/irda/via-ircc.c:598
./drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:489
./drivers/net/ethernet/cavium/liquidio/lio_ethtool.c:500
./drivers/net/ethernet/nvidia/forcedeth.c:3392
./drivers/net/ethernet/qlogic/qlge/qlge_dbg.c:147
./drivers/infiniband/core/cm.c:522
./drivers/infiniband/core/cm.c:493
./drivers/infiniband/hw/i40iw/i40iw_virtchnl.c:434
./drivers/usb/isp1760/isp1760-hcd.c:1036
./drivers/usb/misc/ftdi-elan.c:1007
./drivers/usb/misc/ftdi-elan.c:1007
./drivers/usb/misc/ftdi-elan.c:1018
./drivers/usb/misc/ftdi-elan.c:2091
./drivers/usb/misc/ftdi-elan.c:898
./drivers/misc/vmw_vmci/vmci_queue_pair.c:2232
./drivers/staging/comedi/drivers/das1800.c:1311
./drivers/staging/rtl8723au/core/rtw_mlme_ext.c:694
./drivers/staging/rtl8723au/hal/odm.c:519
./drivers/staging/rtl8723au/hal/rtl8723a_hal_init.c:1389
./drivers/staging/xgifb/vb_setmode.c:2574
./drivers/mmc/host/vub300.c:643
./drivers/pcmcia/pxa2xx_trizeps4.c:64
./drivers/scsi/dc395x.c:3387
./drivers/scsi/pmcraid.c:1604
./drivers/media/pci/bt8xx/dvb-bt8xx.c:393
./drivers/media/usb/dvb-usb/dib0700_devices.c:1739
./drivers/media/usb/s2255/s2255drv.c:811
./drivers/media/usb/s2255/s2255drv.c:828
./drivers/media/dvb-frontends/dib0090.c:2443
./drivers/media/dvb-frontends/mb86a16.c:1479
./drivers/media/dvb-frontends/au8522_decoder.c:327
./drivers/gpu/drm/exynos/exynos_mixer.c:398
./drivers/gpu/drm/bridge/analogix/analogix_dp_core.c:1134
.../tests/condition_with_no_effect.cocci | 60 ++++++++++++++++++++++
1 file changed, 60 insertions(+)
create mode 100644 scripts/coccinelle/tests/condition_with_no_effect.cocci
diff --git a/scripts/coccinelle/tests/condition_with_no_effect.cocci b/scripts/coccinelle/tests/condition_with_no_effect.cocci
new file mode 100644
index 0000000..f910114
--- /dev/null
+++ b/scripts/coccinelle/tests/condition_with_no_effect.cocci
@@ -0,0 +1,60 @@
+///Find conditions where if and else branch match
+// There can be false positives in cases where the positional
+// information is used (as with lockdep) or where the identity
+// is a placeholder for not yet handled cases.
+//
+// In the Linux kernel though it did not seem to actually report
+// false positives except for those that that were documented as
+// being intentional.
+// the two known cases are:
+// arch/sh/kernel/traps_64.c:read_opcode()
+// } else if ((pc & 1) == 0) {
+// /* SHcompact */
+// /* TODO : provide handling for this. We don't really support
+// user-mode SHcompact yet, and for a kernel fault, this would
+// have to come from a module built for SHcompact. */
+// return -EFAULT;
+// } else {
+// /* misaligned */
+// return -EFAULT;
+// }
+// fs/kernfs/file.c:kernfs_fop_open()
+// * Both paths of the branch look the same. They're supposed to
+// * look that way and give @of->mutex different static lockdep keys.
+// */
+// if (has_mmap)
+// mutex_init(&of->mutex);
+// else
+// mutex_init(&of->mutex);
+//
+// All other cases look like bugs or at least lack of documentation
+//
+// Confidence: Moderate
+// Copyright: (C) 2016 Nicholas Mc Guire, OSADL. GPLv2.
+// Comments:
+// Options: --no-includes --include-headers
+
+virtual context
+virtual org
+virtual report
+
+ at cond@
+statement S1;
+position p;
+@@
+
+<+...
+* if@p (...) S1 else S1
+...+>
+
+ at script:python depends on org@
+p << cond.p;
+@@
+
+cocci.print_main("WARNING: possible condition with no effect (if == else)",p)
+
+@script:python depends on report@
+p << cond.p;
+@@
+
+coccilib.report.print_report(p[0],"WARNING: possible condition with no effect (if == else)")
--
2.1.4
^ permalink raw reply related [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
2016-07-22 9:05 ` [Cocci] " Nicholas Mc Guire
@ 2016-07-22 16:56 ` Julia Lawall
-1 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2016-07-22 16:56 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Gilles Muller, Nicolas Palix, Michal Marek, cocci, linux-kernel
> +virtual context
> +virtual org
> +virtual report
> +
> +@cond@
> +statement S1;
> +position p;
> +@@
> +
> +<+...
> +* if@p (...) S1 else S1
> +...+>
You don't need the <+... ...+>. Just put the if by itself.
julia
> +
> +@script:python depends on org@
> +p << cond.p;
> +@@
> +
> +cocci.print_main("WARNING: possible condition with no effect (if == else)",p)
> +
> +@script:python depends on report@
> +p << cond.p;
> +@@
> +
> +coccilib.report.print_report(p[0],"WARNING: possible condition with no effect (if == else)")
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
@ 2016-07-22 16:56 ` Julia Lawall
0 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2016-07-22 16:56 UTC (permalink / raw)
To: cocci
> +virtual context
> +virtual org
> +virtual report
> +
> + at cond@
> +statement S1;
> +position p;
> +@@
> +
> +<+...
> +* if at p (...) S1 else S1
> +...+>
You don't need the <+... ...+>. Just put the if by itself.
julia
> +
> + at script:python depends on org@
> +p << cond.p;
> +@@
> +
> +cocci.print_main("WARNING: possible condition with no effect (if == else)",p)
> +
> + at script:python depends on report@
> +p << cond.p;
> +@@
> +
> +coccilib.report.print_report(p[0],"WARNING: possible condition with no effect (if == else)")
> --
> 2.1.4
>
>
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
2016-07-22 16:56 ` [Cocci] " Julia Lawall
@ 2016-07-22 19:08 ` Nicholas Mc Guire
-1 siblings, 0 replies; 12+ messages in thread
From: Nicholas Mc Guire @ 2016-07-22 19:08 UTC (permalink / raw)
To: Julia Lawall
Cc: Nicholas Mc Guire, Gilles Muller, Nicolas Palix, Michal Marek,
cocci, linux-kernel
On Fri, Jul 22, 2016 at 06:56:47PM +0200, Julia Lawall wrote:
> > +virtual context
> > +virtual org
> > +virtual report
> > +
> > +@cond@
> > +statement S1;
> > +position p;
> > +@@
> > +
> > +<+...
> > +* if@p (...) S1 else S1
> > +...+>
>
> You don't need the <+... ...+>. Just put the if by itself.
>
will drop that then - though those would be needed for the cases that
do this recursively. Will fix it and resend.
thx!
hofrat
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
@ 2016-07-22 19:08 ` Nicholas Mc Guire
0 siblings, 0 replies; 12+ messages in thread
From: Nicholas Mc Guire @ 2016-07-22 19:08 UTC (permalink / raw)
To: cocci
On Fri, Jul 22, 2016 at 06:56:47PM +0200, Julia Lawall wrote:
> > +virtual context
> > +virtual org
> > +virtual report
> > +
> > + at cond@
> > +statement S1;
> > +position p;
> > +@@
> > +
> > +<+...
> > +* if at p (...) S1 else S1
> > +...+>
>
> You don't need the <+... ...+>. Just put the if by itself.
>
will drop that then - though those would be needed for the cases that
do this recursively. Will fix it and resend.
thx!
hofrat
^ permalink raw reply [flat|nested] 12+ messages in thread
* Re: [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
2016-07-22 19:08 ` [Cocci] " Nicholas Mc Guire
@ 2016-07-23 10:46 ` Julia Lawall
-1 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2016-07-23 10:46 UTC (permalink / raw)
To: Nicholas Mc Guire
Cc: Nicholas Mc Guire, Gilles Muller, Nicolas Palix, Michal Marek,
cocci, linux-kernel
On Fri, 22 Jul 2016, Nicholas Mc Guire wrote:
> On Fri, Jul 22, 2016 at 06:56:47PM +0200, Julia Lawall wrote:
> > > +virtual context
> > > +virtual org
> > > +virtual report
> > > +
> > > +@cond@
> > > +statement S1;
> > > +position p;
> > > +@@
> > > +
> > > +<+...
> > > +* if@p (...) S1 else S1
> > > +...+>
> >
> > You don't need the <+... ...+>. Just put the if by itself.
> >
>
> will drop that then - though those would be needed for the cases that
> do this recursively. Will fix it and resend.
Sorry, I don't get your point about recursiveness at all. Even if you
have bizarrely
if (e1)
if (e2) S else S
else
if (e2) S else S
the version without <+... ...+> will still work, finding three matches.
The <+... ...+> starts the matching process at the beginning of the
function and ends it at the end of the function, instead of just working
on each if one by one. Thus <+... ...+> should be much less efficient.
Also with <+... ...+> if you put a position variable on eg the if, you
will get a single position array with all the matches, whereas without it
you get one position array per if. The latter is probably easier to
manage.
julia
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical
@ 2016-07-23 10:46 ` Julia Lawall
0 siblings, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2016-07-23 10:46 UTC (permalink / raw)
To: cocci
On Fri, 22 Jul 2016, Nicholas Mc Guire wrote:
> On Fri, Jul 22, 2016 at 06:56:47PM +0200, Julia Lawall wrote:
> > > +virtual context
> > > +virtual org
> > > +virtual report
> > > +
> > > + at cond@
> > > +statement S1;
> > > +position p;
> > > +@@
> > > +
> > > +<+...
> > > +* if at p (...) S1 else S1
> > > +...+>
> >
> > You don't need the <+... ...+>. Just put the if by itself.
> >
>
> will drop that then - though those would be needed for the cases that
> do this recursively. Will fix it and resend.
Sorry, I don't get your point about recursiveness at all. Even if you
have bizarrely
if (e1)
if (e2) S else S
else
if (e2) S else S
the version without <+... ...+> will still work, finding three matches.
The <+... ...+> starts the matching process at the beginning of the
function and ends it at the end of the function, instead of just working
on each if one by one. Thus <+... ...+> should be much less efficient.
Also with <+... ...+> if you put a position variable on eg the if, you
will get a single position array with all the matches, whereas without it
you get one position array per if. The latter is probably easier to
manage.
julia
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] Better documentation around SmPL ellipsis usage?
2016-07-23 10:46 ` [Cocci] " Julia Lawall
(?)
@ 2016-07-23 13:30 ` SF Markus Elfring
2016-07-23 13:42 ` Julia Lawall
2016-07-23 13:44 ` Julia Lawall
-1 siblings, 2 replies; 12+ messages in thread
From: SF Markus Elfring @ 2016-07-23 13:30 UTC (permalink / raw)
To: cocci
> The <+... ...+> starts the matching process at the beginning of the
> function and ends it at the end of the function, instead of just working
> on each if one by one. Thus <+... ...+> should be much less efficient.
> Also with <+... ...+> if you put a position variable on eg the if, you
> will get a single position array with all the matches, whereas without it
> you get one position array per if.
How are the chances that such kind of information will also find its way
into the section "Dot variants" of the manual?
https://github.com/coccinelle/coccinelle/blob/3b52bd3c6960dc996186e1c58c96355db288825c/docs/manual/cocci_syntax.tex#L632
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] Better documentation around SmPL ellipsis usage?
2016-07-23 13:30 ` [Cocci] Better documentation around SmPL ellipsis usage? SF Markus Elfring
@ 2016-07-23 13:42 ` Julia Lawall
2016-07-23 13:50 ` SF Markus Elfring
2016-07-23 13:44 ` Julia Lawall
1 sibling, 1 reply; 12+ messages in thread
From: Julia Lawall @ 2016-07-23 13:42 UTC (permalink / raw)
To: cocci
On Sat, 23 Jul 2016, SF Markus Elfring wrote:
> > The <+... ...+> starts the matching process at the beginning of the
> > function and ends it at the end of the function, instead of just working
> > on each if one by one. Thus <+... ...+> should be much less efficient.
> > Also with <+... ...+> if you put a position variable on eg the if, you
> > will get a single position array with all the matches, whereas without it
> > you get one position array per if.
>
> How are the chances that such kind of information will also find its way
> into the section "Dot variants" of the manual?
> https://github.com/coccinelle/coccinelle/blob/3b52bd3c6960dc996186e1c58c96355db288825c/docs/manual/cocci_syntax.tex#L632
About the same as the chance that someone will make a patch suitable for
adding the information.
julia
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] Better documentation around SmPL ellipsis usage?
2016-07-23 13:30 ` [Cocci] Better documentation around SmPL ellipsis usage? SF Markus Elfring
2016-07-23 13:42 ` Julia Lawall
@ 2016-07-23 13:44 ` Julia Lawall
1 sibling, 0 replies; 12+ messages in thread
From: Julia Lawall @ 2016-07-23 13:44 UTC (permalink / raw)
To: cocci
On Sat, 23 Jul 2016, SF Markus Elfring wrote:
> > The <+... ...+> starts the matching process at the beginning of the
> > function and ends it at the end of the function, instead of just working
> > on each if one by one. Thus <+... ...+> should be much less efficient.
> > Also with <+... ...+> if you put a position variable on eg the if, you
> > will get a single position array with all the matches, whereas without it
> > you get one position array per if.
>
> How are the chances that such kind of information will also find its way
> into the section "Dot variants" of the manual?
> https://github.com/coccinelle/coccinelle/blob/3b52bd3c6960dc996186e1c58c96355db288825c/docs/manual/cocci_syntax.tex#L632
The cited text already discusses the case where <+... matches back to the
beginning of the function. So perhaps with a little imagination one could
infer on one's own that@...+> at the end of a pattern would match to
the end of the function...
julia
^ permalink raw reply [flat|nested] 12+ messages in thread
* [Cocci] Better documentation around SmPL ellipsis usage?
2016-07-23 13:42 ` Julia Lawall
@ 2016-07-23 13:50 ` SF Markus Elfring
0 siblings, 0 replies; 12+ messages in thread
From: SF Markus Elfring @ 2016-07-23 13:50 UTC (permalink / raw)
To: cocci
>> https://github.com/coccinelle/coccinelle/blob/3b52bd3c6960dc996186e1c58c96355db288825c/docs/manual/cocci_syntax.tex#L632
>
> About the same as the chance that someone will make a patch suitable for
> adding the information.
I would find it hard to extract mentioned constraints from the OCaml
source files directly.
Are you mostly the one who will need to share any more background information
for this software detail so that it can be converted to a corresponding TeX format?
Regards,
Markus
^ permalink raw reply [flat|nested] 12+ messages in thread
end of thread, other threads:[~2016-07-23 13:50 UTC | newest]
Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-22 9:05 [PATCH RFC] coccinelle: tests: if and else branch should probably not be identical Nicholas Mc Guire
2016-07-22 9:05 ` [Cocci] " Nicholas Mc Guire
2016-07-22 16:56 ` Julia Lawall
2016-07-22 16:56 ` [Cocci] " Julia Lawall
2016-07-22 19:08 ` Nicholas Mc Guire
2016-07-22 19:08 ` [Cocci] " Nicholas Mc Guire
2016-07-23 10:46 ` Julia Lawall
2016-07-23 10:46 ` [Cocci] " Julia Lawall
2016-07-23 13:30 ` [Cocci] Better documentation around SmPL ellipsis usage? SF Markus Elfring
2016-07-23 13:42 ` Julia Lawall
2016-07-23 13:50 ` SF Markus Elfring
2016-07-23 13:44 ` Julia Lawall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.