On Thu, 2020-08-20 at 19:03 +0200, Julia Lawall wrote:
> > > I have a bunch of variations of this that are more complicated than I
> > > would have expected. One shorter variant that I have is:
> > >
> > > @@
> > > expression e1,e2;
> > > statement S;
> > > @@
> > >
> > > S
> > > e1
> > > -,
> > > +;
> > > (<+... e2 ...+>);
> > >
> > > This will miss cases where the first statement is the comma thing. But I
> > > think it is possible to improve this now. I will check.
> >
> > Hi Julia.
> >
> > Right, thanks, this adds a dependency on a statement
> > before the expression. Any stragglers would be easily
> > found using slightly different form.
> > There are not very many of these in linux kernel.
> >
> > Another nicety would be to allow the s/,/;/ conversion to
> > find both b and c in this sequence:
> > a = 1;
> > b = 2,
> > c = 3,
> > d = 4;
> > without running the script multiple times.
> > There are many dozen uses of this style in linux kernel.
> >
> > I tried variants of adding a comma after the e2 expression,
> > but cocci seems to have parsing problems with:
> >
> > @@
> > expression e1;
> > expression e2;
> > @@
> > e1
> > - ,
> > + ;
> > e2,
>
> This doesn't work because it's not a valid expression.
>
> The problem is solved by doing:
>
> e1
> - ,
> + ;
> e2
> ... when any
>
> But that doesn't work in the current version of Coccinelle. I have fixed
> the problem, though and it will work shortly.
Great, thanks.
> > I do appreciate that coccinelle adds braces for multiple
> > expression comma use after an if.
> >
> > i.e.:
> > if (foo)
> > a = 1, b = 2;
> > becomes
> > if (foo) {
> > a = 1; b = 2;
> > }
>
> I wasn't sure what was wanted for such things. Should b = 2 now be on a
> separate line?
Ideally for linux kernel style, yes.
> I took the strategy of avoiding the problem and leaving those cases as is.
> That also solves the LIST_HEAD problem. But if it is wanted to change
> these commas under ifs, then that is probably possible too.
I would probably just do those by hand, I believe
there are only a few dozen and they are easily found
using the original script.
> My current complete solution is as follows. The first rule avoids changing
> commas in macros, where thebody of the macro is just an expression. The
> second rule uses position variables to ensure that the two expression are
> on different lines.
>
> @r@
> expression e1,e2;
> statement S;
> position p;
> @@
>
> e1 ,@S@p e2;
>
> @@
> expression e1,e2;
> position p1;
> position p2 :
> script:ocaml(p1) { not((List.hd p1).line_end = (List.hd p2).line) };
> statement S;
> position r.p;
> @@
>
> e1@p1
> -,@S@p
> +;
> e2@p2
> ... when any
>
> The generated patch is below.
(150kb patch removed)
There sure are a bunch of these...
This output patch is very difficult to read as it's unordered.
Perhaps it'd be simpler using --in-place and git diff --stat -p
Thanks again, Joe
btw: the ordered diffstat for Julia's removed patch is:
$ git diff --stat
arch/alpha/kernel/process.c | 2 +-
arch/arm/mach-davinci/pm.c | 2 +-
arch/arm/mach-ixp4xx/ixdp425-setup.c | 2 +-
arch/arm/mach-pxa/eseries.c | 6 +--
arch/arm/mach-pxa/palm27x.c | 4 +-
arch/arm/mach-pxa/z2.c | 2 +-
arch/arm/vfp/vfp.h | 2 +-
arch/ia64/kernel/setup.c | 2 +-
arch/m68k/lib/muldi3.c | 2 +-
arch/mips/bcm63xx/dev-spi.c | 4 +-
arch/mips/kernel/cevt-txx9.c | 2 +-
arch/mips/kernel/vpe-cmp.c | 4 +-
arch/mips/kernel/vpe-mt.c | 4 +-
arch/mips/pci/pci-ar2315.c | 6 +--
arch/openrisc/kernel/time.c | 8 ++--
arch/powerpc/kexec/core.c | 2 +-
arch/powerpc/lib/feature-fixups.c | 8 ++--
arch/sparc/kernel/pci_sun4v.c | 2 +-
arch/x86/kernel/cpu/mtrr/cleanup.c | 4 +-
.../platform/intel-mid/device_libs/platform_bt.c | 4 +-
block/bsg-lib.c | 2 +-
drivers/base/regmap/regmap-debugfs.c | 2 +-
drivers/bcma/driver_pci_host.c | 4 +-
drivers/char/agp/amd-k7-agp.c | 2 +-
drivers/char/agp/nvidia-agp.c | 2 +-
drivers/char/agp/sworks-agp.c | 2 +-
drivers/char/hw_random/iproc-rng200.c | 8 ++--
drivers/char/hw_random/mxc-rnga.c | 6 +--
drivers/char/hw_random/stm32-rng.c | 8 ++--
drivers/char/ipmi/bt-bmc.c | 6 +--
drivers/clk/meson/meson-aoclk.c | 2 +-
drivers/clk/mvebu/ap-cpu-clk.c | 2 +-
drivers/clk/uniphier/clk-uniphier-cpugear.c | 2 +-
drivers/clk/uniphier/clk-uniphier-mux.c | 2 +-
drivers/clocksource/mps2-timer.c | 6 +--
drivers/clocksource/timer-armada-370-xp.c | 8 ++--
drivers/counter/ti-eqep.c | 2 +-
drivers/crypto/amcc/crypto4xx_alg.c | 2 +-
drivers/crypto/atmel-tdes.c | 2 +-
drivers/crypto/hifn_795x.c | 4 +-
drivers/crypto/talitos.c | 8 ++--
drivers/edac/ppc4xx_edac.c | 2 +-
drivers/firmware/arm_scmi/base.c | 2 +-
drivers/gpio/gpio-max77620.c | 2 +-
drivers/gpio/gpio-mc33880.c | 2 +-
drivers/gpio/gpio-omap.c | 18 ++++-----
drivers/gpio/gpio-rc5t583.c | 20 +++++-----
drivers/gpio/gpio-rda.c | 12 +++---
drivers/gpio/gpio-sama5d2-piobu.c | 18 ++++-----
drivers/gpio/gpio-tegra186.c | 2 +-
drivers/gpio/gpio-vx855.c | 2 +-
drivers/gpio/gpio-wcove.c | 2 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_sa.c | 2 +-
drivers/gpu/drm/i915/display/intel_hdmi.c | 4 +-
drivers/gpu/drm/i915/intel_region_lmem.c | 2 +-
drivers/gpu/drm/i915/selftests/i915_buddy.c | 2 +-
drivers/gpu/drm/imx/parallel-display.c | 2 +-
.../gpu/drm/nouveau/nvkm/engine/disp/channv50.c | 2 +-
drivers/gpu/drm/radeon/radeon_sa.c | 2 +-
drivers/gpu/drm/vmwgfx/vmwgfx_execbuf.c | 2 +-
drivers/hid/hid-sensor-custom.c | 2 +-
drivers/hwmon/pc87360.c | 4 +-
drivers/i2c/busses/i2c-imx.c | 2 +-
drivers/iio/chemical/pms7003.c | 2 +-
drivers/infiniband/hw/hfi1/qsfp.c | 4 +-
drivers/infiniband/hw/qib/qib_iba6120.c | 2 +-
drivers/infiniband/hw/qib/qib_iba7220.c | 2 +-
drivers/infiniband/sw/siw/siw_main.c | 2 +-
drivers/infiniband/ulp/ipoib/ipoib_multicast.c | 2 +-
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 2 +-
drivers/input/input.c | 2 +-
drivers/input/misc/ixp4xx-beeper.c | 2 +-
drivers/input/misc/pm8941-pwrkey.c | 2 +-
drivers/input/serio/parkbd.c | 2 +-
drivers/iommu/amd/init.c | 4 +-
drivers/irqchip/irq-renesas-rza1.c | 12 +++---
drivers/leds/leds-ariel.c | 6 +--
drivers/leds/leds-lm3533.c | 2 +-
drivers/leds/leds-lm3642.c | 4 +-
drivers/md/bcache/sysfs.c | 2 +-
drivers/md/raid10.c | 2 +-
drivers/media/dvb-frontends/m88ds3103.c | 6 +--
drivers/media/dvb-frontends/rtl2832.c | 14 +++----
drivers/media/dvb-frontends/ts2020.c | 10 ++---
drivers/media/platform/coda/coda-common.c | 2 +-
drivers/media/platform/mtk-jpeg/mtk_jpeg_core.c | 2 +-
.../media/platform/sunxi/sun4i-csi/sun4i_v4l2.c | 2 +-
drivers/media/radio/radio-sf16fmr2.c | 2 +-
drivers/media/tuners/mt2060.c | 2 +-
drivers/media/usb/au0828/au0828-video.c | 2 +-
drivers/media/usb/dvb-usb-v2/dvbsky.c | 22 +++++------
drivers/media/usb/dvb-usb-v2/lmedm04.c | 2 +-
drivers/media/usb/dvb-usb-v2/rtl28xxu.c | 4 +-
drivers/media/usb/dvb-usb/dib0700_devices.c | 4 +-
drivers/media/usb/em28xx/em28xx-audio.c | 14 +++----
drivers/media/usb/gspca/ov534.c | 12 +++---
drivers/media/usb/pvrusb2/pvrusb2-devattr.c | 2 +-
drivers/mfd/88pm860x-core.c | 10 ++---
drivers/mfd/atmel-flexcom.c | 2 +-
drivers/mfd/rave-sp.c | 2 +-
drivers/mfd/rk808.c | 2 +-
drivers/misc/pci_endpoint_test.c | 2 +-
drivers/mmc/core/core.c | 8 ++--
drivers/mmc/host/sdhci-pci-core.c | 2 +-
drivers/mtd/devices/st_spi_fsm.c | 2 +-
drivers/mtd/maps/pci.c | 8 ++--
drivers/mtd/nand/raw/marvell_nand.c | 2 +-
drivers/mtd/nand/raw/mxc_nand.c | 2 +-
drivers/net/dsa/mv88e6xxx/global1_atu.c | 2 +-
drivers/net/ethernet/8390/axnet_cs.c | 2 +-
drivers/net/ethernet/8390/lib8390.c | 2 +-
drivers/net/ethernet/aquantia/atlantic/aq_ring.c | 6 +--
.../net/ethernet/freescale/fs_enet/mii-bitbang.c | 2 +-
drivers/net/ethernet/freescale/fsl_pq_mdio.c | 2 +-
drivers/net/ethernet/hisilicon/hns/hns_dsaf_misc.c | 12 +++---
.../ethernet/hisilicon/hns3/hns3pf/hclge_main.c | 2 +-
drivers/net/ethernet/intel/i40e/i40e_ethtool.c | 2 +-
.../net/ethernet/mellanox/mlx5/core/esw/chains.c | 2 +-
.../ethernet/mellanox/mlx5/core/eswitch_offloads.c | 2 +-
drivers/net/ethernet/micrel/ks8851_common.c | 2 +-
drivers/net/ethernet/qlogic/netxen/netxen_nic_hw.c | 28 ++++++-------
drivers/net/ethernet/sfc/ptp.c | 2 +-
drivers/net/ethernet/ti/davinci_mdio.c | 6 +--
drivers/net/ipa/ipa_qmi.c | 2 +-
drivers/net/thunderbolt.c | 2 +-
drivers/net/usb/cdc-phonet.c | 2 +-
drivers/net/wan/sbni.c | 46 +++++++++++-----------
drivers/net/wireless/intel/iwlwifi/mvm/debugfs.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt7615/mcu.c | 6 +--
.../net/wireless/mediatek/mt76/mt7615/sdio_mcu.c | 2 +-
.../net/wireless/mediatek/mt76/mt7615/usb_mcu.c | 2 +-
drivers/net/wireless/mediatek/mt76/mt7915/mcu.c | 6 +--
drivers/net/wireless/ralink/rt2x00/rt2800lib.c | 2 +-
drivers/net/wireless/st/cw1200/txrx.c | 2 +-
drivers/nfc/pn533/usb.c | 2 +-
drivers/nvmem/imx-iim.c | 10 ++---
drivers/nvmem/qcom-spmi-sdam.c | 2 +-
drivers/nvmem/snvs_lpgpr.c | 2 +-
drivers/phy/ti/phy-j721e-wiz.c | 4 +-
drivers/pinctrl/berlin/berlin-bg4ct.c | 6 +--
drivers/pinctrl/berlin/pinctrl-as370.c | 6 +--
drivers/pinctrl/intel/pinctrl-baytrail.c | 12 +++---
drivers/pinctrl/intel/pinctrl-merrifield.c | 2 +-
drivers/pinctrl/mediatek/pinctrl-moore.c | 4 +-
drivers/pinctrl/mediatek/pinctrl-paris.c | 4 +-
drivers/pinctrl/pinctrl-at91.c | 2 +-
drivers/pinctrl/pinctrl-digicolor.c | 8 ++--
drivers/pinctrl/qcom/pinctrl-spmi-gpio.c | 14 +++----
drivers/pinctrl/ti/pinctrl-ti-iodelay.c | 2 +-
.../x86/intel_speed_select_if/isst_if_common.c | 6 +--
drivers/power/supply/ab8500_fg.c | 2 +-
drivers/power/supply/bq24190_charger.c | 2 +-
drivers/power/supply/wm831x_power.c | 2 +-
drivers/regulator/mc13892-regulator.c | 4 +-
drivers/regulator/wm831x-isink.c | 8 ++--
drivers/remoteproc/remoteproc_virtio.c | 4 +-
drivers/reset/hisilicon/reset-hi3660.c | 2 +-
drivers/reset/reset-lpc18xx.c | 4 +-
drivers/reset/reset-npcm.c | 4 +-
drivers/reset/sti/reset-syscfg.c | 2 +-
drivers/s390/crypto/zcrypt_cex2a.c | 2 +-
drivers/s390/crypto/zcrypt_cex4.c | 2 +-
drivers/s390/crypto/zcrypt_msgtype6.h | 4 +-
drivers/scsi/arm/cumana_2.c | 8 ++--
drivers/scsi/arm/eesox.c | 4 +-
drivers/scsi/arm/powertec.c | 4 +-
drivers/scsi/hpsa.c | 2 +-
drivers/scsi/isci/host.c | 2 +-
drivers/scsi/pmcraid.c | 2 +-
drivers/spi/spi-meson-spicc.c | 22 +++++------
drivers/staging/media/hantro/hantro_v4l2.c | 2 +-
drivers/staging/media/ipu3/ipu3-css-params.c | 2 +-
drivers/staging/media/rkvdec/rkvdec.c | 2 +-
drivers/tty/hvc/hvsi_lib.c | 2 +-
drivers/tty/moxa.c | 8 ++--
drivers/tty/mxser.c | 18 ++++-----
drivers/tty/serial/imx.c | 2 +-
drivers/tty/serial/lantiq.c | 2 +-
drivers/usb/class/cdc-acm.c | 12 +++---
drivers/usb/gadget/udc/net2280.c | 4 +-
drivers/usb/host/isp1362-hcd.c | 2 +-
drivers/usb/phy/phy-isp1301-omap.c | 10 ++---
drivers/usb/roles/intel-xhci-usb-role-switch.c | 6 +--
drivers/usb/typec/tcpm/tcpm.c | 12 +++---
drivers/usb/typec/ucsi/psy.c | 6 +--
drivers/video/backlight/sky81452-backlight.c | 2 +-
drivers/video/fbdev/imsttfb.c | 4 +-
drivers/video/fbdev/pxa3xx-gcu.c | 4 +-
drivers/video/fbdev/s3c2410fb.c | 2 +-
drivers/watchdog/iTCO_wdt.c | 4 +-
drivers/watchdog/mpc8xxx_wdt.c | 4 +-
drivers/watchdog/pm8916_wdt.c | 2 +-
drivers/watchdog/rza_wdt.c | 4 +-
fs/afs/inode.c | 4 +-
fs/ceph/dir.c | 2 +-
fs/lockd/host.c | 2 +-
fs/nfs/nfs42proc.c | 2 +-
fs/omfs/file.c | 4 +-
fs/reiserfs/do_balan.c | 4 +-
fs/xfs/libxfs/xfs_attr_remote.c | 2 +-
fs/xfs/libxfs/xfs_btree.c | 2 +-
kernel/audit.c | 2 +-
kernel/dma/debug.c | 4 +-
kernel/sched/fair.c | 2 +-
kernel/time/alarmtimer.c | 2 +-
lib/test_rhashtable.c | 2 +-
mm/hugetlb_cgroup.c | 4 +-
net/ipv4/tcp_vegas.c | 8 ++--
net/ipv6/calipso.c | 2 +-
net/ipv6/route.c | 2 +-
net/mac80211/debugfs_sta.c | 2 +-
net/rxrpc/recvmsg.c | 2 +-
net/smc/smc_clc.c | 2 +-
net/tls/tls_main.c | 2 +-
samples/v4l/v4l2-pci-skeleton.c | 4 +-
sound/firewire/fireworks/fireworks_pcm.c | 2 +-
sound/pci/hda/patch_ca0132.c | 2 +-
sound/pci/hda/patch_hdmi.c | 2 +-
sound/soc/codecs/madera.c | 4 +-
sound/soc/intel/boards/bytcr_rt5651.c | 2 +-
sound/soc/samsung/snow.c | 2 +-
sound/soc/soc-dapm.c | 2 +-
sound/soc/sof/intel/hda-dsp.c | 2 +-
tools/perf/builtin-diff.c | 4 +-
tools/perf/builtin-inject.c | 2 +-
tools/perf/ui/browsers/annotate.c | 2 +-
tools/perf/ui/tui/util.c | 2 +-
tools/perf/util/annotate.c | 2 +-
tools/perf/util/evsel.c | 2 +-
tools/testing/nvdimm/test/nfit.c | 2 +-
.../testing/selftests/bpf/benchs/bench_ringbufs.c | 2 +-
tools/testing/selftests/bpf/test_verifier.c | 4 +-
tools/testing/selftests/net/psock_fanout.c | 6 +--
tools/testing/selftests/vm/userfaultfd.c | 12 +++---
234 files changed, 509 insertions(+), 509 deletions(-)
