Hello Joe,
I'm adding Dan to Cc.
On 30.10.2015 01:52, Vladimir Zapolskiy wrote:
> On 30.10.2015 00:46, Joe Perches wrote:
>> On Thu, 2015-10-29 at 23:36 +0200, Vladimir Zapolskiy wrote:
>>> A simple search over the kernel souce displays a number of correctly
>>> defined multiline macro, which generally are used as an array element
>>> initializer:
>>>
>>> % find ../linux -type f | xargs grep -B1 -H '^[:space]*\[.*\\$'
>>>
>>> However checkpatch.pl unexpectedly complains about all these macro
>>> definitions:
>>>
>>> % ./scripts/checkpatch.pl --types COMPLEX_MACRO -f include/linux/perf/arm_pmu.h
>>>
>>> ERROR: Macros with complex values should be enclosed in parentheses
>>> +#define PERF_MAP_ALL_UNSUPPORTED \
>>> + [0 ... PERF_COUNT_HW_MAX - 1] = HW_OP_UNSUPPORTED
>>>
>>> The change intends to fix this type of false positives by flattening
>>> only array members and skipping array element designators.
>>>
>>> Signed-off-by: Vladimir Zapolskiy <[email protected]>
>>> ---
>>> scripts/checkpatch.pl | 5 +++--
>>> 1 file changed, 3 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
>>> index f2a1131..3882893 100755
>>> --- a/scripts/checkpatch.pl
>>> +++ b/scripts/checkpatch.pl
>>> @@ -4526,7 +4526,7 @@ sub process {
>>> # Flatten any parentheses and braces
>>> while ($dstat =~ s/\([^\(\)]*\)/1/ ||
>>> $dstat =~ s/\{[^\{\}]*\}/1/ ||
>>> - $dstat =~ s/\[[^\[\]]*\]/1/)
>>> + $dstat =~ s/.\[[^\[\]]*\]/1/)
>>
>> Perhaps the . before the [ might be a bit broad.
>
> At this point preceding spaces and parentheses are removed, probably any
> alphanumeric symbol should fit here.
>
> OTOH I believe in correct C code here before \[ symbol you may find only
> an alphanumeric symbol or ^, so I don't expect any false negatives, if .
> is used above.
>
I tested this version (COMPLEX_MACRO only) with . symbol over all .c and
.h files in the kernel (commit ce5c2d2c256a4) and the change fixes 230
false positives, see the list below in format $file:$lineno.
No new false negatives are introduced, so according to my test . is okay
here to have.
Please also note that the problem is quite old and it emerged earlier,
e.g. see http://permalink.gmane.org/gmane.linux.ports.arm.kernel/451646 etc.
>> I'm not sure there's a great way to handle this.
>>
>> Andy?
>>> {
>>> }
>>>
>>> @@ -4546,7 +4546,8 @@ sub process {
>>> union|
>>> struct|
>>> \.$Ident\s*=\s*|
>>> - ^\"|\"$
>>> + ^\"|\"$|
>>> + ^\[
>>> }x;
>>> #print "REST<$rest> dstat<$dstat> ctx<$ctx>\n";
>>> if ($dstat ne '' &&
arch/arc/kernel/sys.c:11
arch/arm64/kernel/smp.c:624
arch/arm64/kernel/sys32.c:43
arch/arm64/kernel/sys.c:46
arch/arm/kernel/smp.c:473
arch/arm/mach-davinci/mux.h:18
arch/arm/mach-davinci/mux.h:29
arch/arm/mach-davinci/mux.h:40
arch/arm/mach-imx/devices/platform-flexcan.c:19
arch/arm/mach-imx/devices/platform-imx2-wdt.c:20
arch/arm/mach-imx/devices/platform-imx-i2c.c:21
arch/arm/mach-imx/devices/platform-imx-ssi.c:12
arch/arm/mach-imx/devices/platform-imx-uart.c:12
arch/arm/mach-imx/devices/platform-imx-uart.c:22
arch/arm/mach-imx/devices/platform-mxc-mmc.c:23
arch/arm/mach-imx/devices/platform-sdhci-esdhc-imx.c:22
arch/arm/mach-imx/devices/platform-spi_imx.c:21
arch/avr32/mach-at32ap/at32ap700x.c:1732
arch/blackfin/mach-bf527/boards/ezkit.c:513
arch/blackfin/mach-bf533/boards/stamp.c:594
arch/blackfin/mach-bf533/boards/stamp.c:612
arch/blackfin/mach-bf537/boards/stamp.c:2546
arch/blackfin/mach-bf537/boards/stamp.c:2564
arch/blackfin/mach-bf548/boards/ezkit.c:1834
arch/blackfin/mach-bf548/boards/ezkit.c:1860
arch/c6x/kernel/sys_c6x.c:51
arch/h8300/kernel/syscalls.c:6
arch/hexagon/kernel/syscalltab.c:28
arch/ia64/kernel/module.c:111
arch/metag/kernel/sys_metag.c:160
arch/mips/bcm63xx/reset.c:19
arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h:1016
arch/mips/include/asm/mach-bcm63xx/bcm63xx_cpu.h:551
arch/nios2/kernel/syscall_table.c:25
arch/openrisc/kernel/sys_call_table.c:24
arch/score/kernel/sys_call_table.c:8
arch/tile/kernel/compat.c:90
arch/tile/kernel/sys.c:103
arch/unicore32/kernel/sys.c:33
arch/x86/entry/syscall_32.c:19
arch/x86/entry/syscall_64.c:21
arch/x86/include/asm/intel-mid.h:73
arch/x86/include/asm/paravirt_types.h:374
arch/x86/include/asm/paravirt_types.h:377
arch/x86/include/asm/stackprotector.h:51
arch/x86/include/asm/xen/hypercall.h:88
arch/x86/kernel/asm-offsets_32.c:10
arch/x86/kernel/asm-offsets_64.c:10
arch/x86/kernel/asm-offsets_64.c:17
arch/x86/kernel/asm-offsets_64.c:7
arch/x86/kernel/asm-offsets_64.c:8
arch/x86/kernel/cpu/perf_event_intel_pt.c:56
arch/x86/kernel/cpu/perf_event_p4.c:1127
arch/x86/kernel/cpu/perf_event_p4.c:36
arch/x86/kernel/perf_regs.c:16
arch/x86/kvm/vmx.c:618
arch/x86/kvm/vmx.c:619
arch/x86/kvm/vmx.c:924
arch/x86/tools/relocs.c:140
arch/x86/tools/relocs.c:160
arch/x86/tools/relocs.c:176
arch/x86/tools/relocs.c:193
arch/x86/um/sys_call_table_32.c:32
arch/x86/um/sys_call_table_64.c:45
arch/x86/um/user-offsets.c:12
arch/x86/um/user-offsets.c:17
arch/x86/um/user-offsets.c:18
arch/x86/xen/trace.c:5
arch/xtensa/kernel/syscall.c:35
Documentation/networking/timestamping/hwtstamp_config.c:46
Documentation/networking/timestamping/hwtstamp_config.c:55
drivers/gpu/drm/amd/amdkfd/kfd_chardev.c:871
drivers/gpu/drm/drm_ioctl.c:513
drivers/gpu/drm/drm_modes.c:1020
drivers/gpu/drm/msm/adreno/adreno_gpu.h:30
drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h:122
drivers/gpu/drm/radeon/r600_cs.c:86
drivers/gpu/drm/radeon/r600_cs.c:87
drivers/gpu/drm/radeon/r600_cs.c:88
drivers/gpu/drm/radeon/r600_cs.c:89
drivers/gpu/drm/radeon/r600_cs.c:90
drivers/gpu/drm/radeon/r600_cs.c:91
drivers/gpu/drm/radeon/r600_cs.c:92
drivers/gpu/drm/radeon/r600_cs.c:93
drivers/gpu/drm/vmwgfx/vmwgfx_drv.c:140
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c:97
drivers/hwmon/pmbus/pmbus.h:393
drivers/hwtracing/intel_th/gth.c:245
drivers/iio/adc/qcom-spmi-vadc.c:607
drivers/input/misc/ims-pcu.c:173
drivers/media/dvb-core/dvb_frontend.c:1079
drivers/media/pci/ivtv/ivtv-mailbox.c:50
drivers/media/v4l2-core/v4l2-ioctl.c:2403
drivers/media/v4l2-core/v4l2-ioctl.c:2412
drivers/mfd/asic3.c:55
drivers/mfd/axp20x.c:248
drivers/mfd/db8500-prcmu.c:279
drivers/mfd/db8500-prcmu.c:333
drivers/mfd/db8500-prcmu.c:467
drivers/mfd/max8997-irq.c:68
drivers/net/ethernet/renesas/sh_eth.c:55
drivers/net/ethernet/sfc/ef10.c:1093
drivers/net/ethernet/sfc/ef10.c:1096
drivers/net/ethernet/sfc/ef10.c:1099
drivers/net/ethernet/sfc/ef10.c:1101
drivers/net/ethernet/sfc/falcon.c:137
drivers/net/ethernet/sfc/falcon.c:143
drivers/net/ethernet/sfc/falcon.c:145
drivers/net/ethernet/sfc/mcdi_mon.c:43
drivers/net/ethernet/sfc/siena.c:431
drivers/net/ethernet/sfc/siena.c:434
drivers/net/ethernet/sfc/siena.c:436
drivers/net/irda/donauboe.c:184
drivers/net/irda/donauboe.c:185
drivers/net/irda/via-ircc.h:160
drivers/net/irda/via-ircc.h:162
drivers/net/irda/via-ircc.h:164
drivers/net/irda/via-ircc.h:174
drivers/net/irda/via-ircc.h:175
drivers/net/phy/bcm7xxx.c:32
drivers/net/usb/sierra_net.c:934
drivers/net/wireless/iwlegacy/3945.c:59
drivers/net/wireless/iwlegacy/4965-rs.c:76
drivers/net/wireless/iwlwifi/dvm/rs.c:76
drivers/net/wireless/iwlwifi/dvm/rx.c:40
drivers/net/wireless/iwlwifi/mvm/coex_legacy.c:75
drivers/net/wireless/iwlwifi/mvm/ops.c:271
drivers/net/wireless/iwlwifi/mvm/rs.c:77
drivers/net/wireless/iwlwifi/mvm/rs.c:86
drivers/net/wireless/iwlwifi/mvm/utils.c:198
drivers/net/wireless/prism54/oid_mgt.c:48
drivers/pinctrl/pinctrl-tz1090.c:693
drivers/pinctrl/pinctrl-tz1090.c:700
drivers/pinctrl/pinctrl-tz1090.c:788
drivers/pinctrl/pinctrl-tz1090-pdc.c:171
drivers/pinctrl/pinctrl-zynq.c:763
drivers/pinctrl/pinctrl-zynq.c:771
drivers/pinctrl/qcom/pinctrl-apq8064.c:220
drivers/pinctrl/qcom/pinctrl-apq8084.c:336
drivers/pinctrl/qcom/pinctrl-ipq8064.c:173
drivers/pinctrl/qcom/pinctrl-msm8660.c:386
drivers/pinctrl/qcom/pinctrl-msm8916.c:297
drivers/pinctrl/qcom/pinctrl-msm8960.c:345
drivers/pinctrl/qcom/pinctrl-msm8x74.c:332
drivers/pinctrl/sh-pfc/sh_pfc.h:288
drivers/pinctrl/sh-pfc/sh_pfc.h:321
drivers/power/bq27xxx_battery.c:428
drivers/regulator/ab8500.c:1798
drivers/regulator/act8865-regulator.c:176
drivers/regulator/as3711-regulator.c:133
drivers/regulator/axp20x-regulator.c:101
drivers/regulator/axp20x-regulator.c:115
drivers/regulator/axp20x-regulator.c:39
drivers/regulator/axp20x-regulator.c:61
drivers/regulator/axp20x-regulator.c:81
drivers/regulator/hi6421-regulator.c:169
drivers/regulator/hi6421-regulator.c:205
drivers/regulator/hi6421-regulator.c:242
drivers/regulator/hi6421-regulator.c:276
drivers/regulator/hi6421-regulator.c:310
drivers/regulator/ltc3589.c:199
drivers/regulator/max8907-regulator.c:186
drivers/regulator/max8907-regulator.c:39
drivers/regulator/max8907-regulator.c:49
drivers/regulator/max8907-regulator.c:66
drivers/regulator/max8907-regulator.c:78
drivers/regulator/max8907-regulator.c:92
drivers/regulator/mc13xxx.h:59
drivers/regulator/mc13xxx.h:77
drivers/regulator/mc13xxx.h:92
drivers/regulator/mt6397-regulator.c:43
drivers/regulator/mt6397-regulator.c:67
drivers/regulator/mt6397-regulator.c:87
drivers/regulator/palmas-regulator.c:318
drivers/regulator/palmas-regulator.c:354
drivers/regulator/pcap-regulator.c:107
drivers/regulator/pcap-regulator.c:222
drivers/regulator/pfuze100-regulator.c:162
drivers/regulator/pfuze100-regulator.c:177
drivers/regulator/pfuze100-regulator.c:195
drivers/regulator/pfuze100-regulator.c:212
drivers/regulator/rn5t618-regulator.c:31
drivers/regulator/tps65218-regulator.c:52
fs/f2fs/f2fs.h:80
fs/f2fs/f2fs.h:82
fs/nls/nls_euc-jp.c:31
fs/nls/nls_euc-jp.c:49
include/drm/drmP.h:265
include/linux/genl_magic_func.h:21
include/linux/genl_magic_func.h:34
include/linux/genl_magic_func.h:39
include/linux/perf/arm_pmu.h:44
include/linux/perf/arm_pmu.h:47
include/linux/regmap.h:797
include/linux/wimax/debug.h:340
include/net/fib_rules.h:83
include/uapi/linux/wireless.h:349
include/xen/hvm.h:10
kernel/cgroup.c:128
kernel/cgroup.c:135
kernel/cgroup.c:150
kernel/cgroup.c:156
kernel/cpu.c:742
kernel/locking/lockdep.c:459
kernel/trace/trace_probe.h:202
mm/slab.h:363
mm/vmalloc.c:1851
mm/vmalloc.c:1853
net/core/neighbour.c:3026
net/mac80211/debugfs.c:95
net/mac80211/rc80211_minstrel_ht.c:129
net/mac80211/rc80211_minstrel_ht.c:54
net/mac80211/rc80211_minstrel_ht.c:82
net/sunrpc/auth_gss/gss_rpc_upcall.c:55
sound/core/oss/mixer_oss.c:1117
sound/core/pcm.c:170
sound/core/pcm.c:236
sound/core/pcm.c:237
sound/core/pcm.c:238
sound/core/pcm.c:239
sound/core/pcm.c:240
sound/core/pcm.c:241
sound/core/pcm.c:242
sound/core/pcm.c:243
sound/core/pcm.c:244
sound/core/pcm_native.c:248
sound/oss/ad1848_mixer.h:93
sound/oss/ad1848_mixer.h:97
sound/pci/ice1712/quartet.c:666
sound/pci/ice1712/quartet.c:671
--
With best wishes,
Vladimir
On Tue, 2015-11-10 at 20:01 +0200, Vladimir Zapolskiy wrote:
> Hello Joe,
Hello and thanks Vladimir.
> I tested this version (COMPLEX_MACRO only) with . symbol over all .c and
> .h files in the kernel (commit ce5c2d2c256a4) and the change fixes 230
> false positives, see the list below in format $file:$lineno.
>
> No new false negatives are introduced, so according to my test . is okay
> here to have.
Andrew, can you please pick this up?