* [PATCH 00/26] constify local structures
@ 2016-09-11 13:05 Julia Lawall
2016-09-11 13:05 ` [PATCH 05/26] ARCNET: " Julia Lawall
` (8 more replies)
0 siblings, 9 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:05 UTC (permalink / raw)
To: linux-renesas-soc
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, platform-driver-x86, devel, linux-scsi, linux-rdma,
Jason Gunthorpe, linux-acpi, tpmdd-devel, linux-media, linux-pm,
linux-can, Shiraz Saleem, Sergei Shtylyov, netdev, Chien Tin Tung,
linux-wireless, linux-kernel, linux-spi, linux-usb, joe
Constify local structures.
The semantic patch that makes this change is as follows:
(http://coccinelle.lip6.fr/)
// <smpl>
// The first rule ignores some cases that posed problems
@r disable optional_qualifier@
identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
identifier i != {s5k5baf_cis_rect,smtcfb_fix};
position p;
@@
static struct s i@p = { ... };
@lstruct@
identifier r.s;
@@
struct s { ... };
@used depends on lstruct@
identifier r.i;
@@
i
@bad1@
expression e;
identifier r.i;
assignment operator a;
@@
(<+...i...+>) a e
@bad2@
identifier r.i;
@@
&(<+...i...+>)
@bad3@
identifier r.i;
declarer d;
@@
d(...,<+...i...+>,...);
@bad4@
identifier r.i;
type T;
T[] e;
identifier f;
position p;
@@
f@p(...,
(
(<+...i...+>)
&
e
)
,...)
@bad4a@
identifier r.i;
type T;
T *e;
identifier f;
position p;
@@
f@p(...,
(
(<+...i...+>)
&
e
)
,...)
@ok5@
expression *e;
identifier r.i;
position p;
@@
e =@p i
@bad5@
expression *e;
identifier r.i;
position p != ok5.p;
@@
e =@p (<+...i...+>)
@rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
identifier s,r.i;
position r.p;
@@
static
+const
struct s i@p = { ... };
@depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
disable optional_qualifier@
identifier rr.s,r.i;
@@
static
+const
struct s i;
// </smpl>
---
drivers/acpi/acpi_apd.c | 8 +++---
drivers/char/tpm/tpm-interface.c | 10 ++++----
drivers/char/tpm/tpm-sysfs.c | 2 -
drivers/cpufreq/intel_pstate.c | 8 +++---
drivers/infiniband/hw/i40iw/i40iw_uk.c | 6 ++---
drivers/media/i2c/tvp514x.c | 2 -
drivers/media/pci/ddbridge/ddbridge-core.c | 18 +++++++--------
drivers/media/pci/ngene/ngene-cards.c | 14 ++++++------
drivers/media/pci/smipcie/smipcie-main.c | 8 +++---
drivers/misc/sgi-xp/xpc_uv.c | 2 -
drivers/net/arcnet/com20020-pci.c | 10 ++++----
drivers/net/can/c_can/c_can_pci.c | 4 +--
drivers/net/can/sja1000/plx_pci.c | 20 ++++++++---------
drivers/net/ethernet/mellanox/mlx4/main.c | 4 +--
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 -
drivers/net/ethernet/renesas/sh_eth.c | 14 ++++++------
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 -
drivers/net/wireless/ath/dfs_pattern_detector.c | 2 -
drivers/net/wireless/intel/iwlegacy/3945.c | 4 +--
drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 2 -
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 2 -
drivers/platform/chrome/chromeos_laptop.c | 22 +++++++++----------
drivers/platform/x86/intel_scu_ipc.c | 6 ++---
drivers/platform/x86/intel_telemetry_debugfs.c | 2 -
drivers/scsi/esas2r/esas2r_flash.c | 2 -
drivers/scsi/hptiop.c | 6 ++---
drivers/spi/spi-dw-pci.c | 4 +--
drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 -
drivers/usb/misc/ezusb.c | 2 -
drivers/video/fbdev/matrox/matroxfb_g450.c | 2 -
lib/crc64_ecma.c | 2 -
sound/pci/ctxfi/ctatc.c | 2 -
sound/pci/hda/patch_ca0132.c | 10 ++++----
sound/pci/riptide/riptide.c | 2 -
40 files changed, 110 insertions(+), 110 deletions(-)
^ permalink raw reply [flat|nested] 35+ messages in thread
* [PATCH 05/26] ARCNET: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
@ 2016-09-11 13:05 ` Julia Lawall
2016-09-12 12:31 ` Julia Lawall
2016-09-11 13:05 ` [PATCH 06/26] ath: " Julia Lawall
` (7 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:05 UTC (permalink / raw)
To: Michael Grzeschik; +Cc: joe, kernel-janitors, netdev, linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/arcnet/com20020-pci.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
index 239de38..32b8406 100644
--- a/drivers/net/arcnet/com20020-pci.c
+++ b/drivers/net/arcnet/com20020-pci.c
@@ -264,7 +264,7 @@ static void com20020pci_remove(struct pci_dev *pdev)
}
}
-static struct com20020_pci_card_info card_info_10mbit = {
+static const struct com20020_pci_card_info card_info_10mbit = {
.name = "ARC-PCI",
.devcount = 1,
.chan_map_tbl = {
@@ -277,7 +277,7 @@ static struct com20020_pci_card_info card_info_10mbit = {
.flags = ARC_CAN_10MBIT,
};
-static struct com20020_pci_card_info card_info_5mbit = {
+static const struct com20020_pci_card_info card_info_5mbit = {
.name = "ARC-PCI",
.devcount = 1,
.chan_map_tbl = {
@@ -290,7 +290,7 @@ static struct com20020_pci_card_info card_info_5mbit = {
.flags = ARC_IS_5MBIT,
};
-static struct com20020_pci_card_info card_info_sohard = {
+static const struct com20020_pci_card_info card_info_sohard = {
.name = "PLX-PCI",
.devcount = 1,
/* SOHARD needs PCI base addr 4 */
@@ -304,7 +304,7 @@ static struct com20020_pci_card_info card_info_sohard = {
.flags = ARC_CAN_10MBIT,
};
-static struct com20020_pci_card_info card_info_eae_arc1 = {
+static const struct com20020_pci_card_info card_info_eae_arc1 = {
.name = "EAE PLX-PCI ARC1",
.devcount = 1,
.chan_map_tbl = {
@@ -329,7 +329,7 @@ static struct com20020_pci_card_info card_info_eae_arc1 = {
.flags = ARC_CAN_10MBIT,
};
-static struct com20020_pci_card_info card_info_eae_ma1 = {
+static const struct com20020_pci_card_info card_info_eae_ma1 = {
.name = "EAE PLX-PCI MA1",
.devcount = 2,
.chan_map_tbl = {
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 06/26] ath: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
2016-09-11 13:05 ` [PATCH 05/26] ARCNET: " Julia Lawall
@ 2016-09-11 13:05 ` Julia Lawall
2016-09-14 17:02 ` [06/26] " Kalle Valo
2016-09-11 13:05 ` [PATCH 07/26] net/mlx4_core: " Julia Lawall
` (6 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:05 UTC (permalink / raw)
To: Luis R. Rodriguez
Cc: joe, kernel-janitors, Kalle Valo, linux-wireless, netdev,
linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/wireless/ath/dfs_pattern_detector.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ath/dfs_pattern_detector.c b/drivers/net/wireless/ath/dfs_pattern_detector.c
index 2f8136d..4100ffd 100644
--- a/drivers/net/wireless/ath/dfs_pattern_detector.c
+++ b/drivers/net/wireless/ath/dfs_pattern_detector.c
@@ -338,7 +338,7 @@ static bool dpd_set_domain(struct dfs_pattern_detector *dpd,
return true;
}
-static struct dfs_pattern_detector default_dpd = {
+static const struct dfs_pattern_detector default_dpd = {
.exit = dpd_exit,
.set_dfs_domain = dpd_set_domain,
.add_pulse = dpd_add_pulse,
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 07/26] net/mlx4_core: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
2016-09-11 13:05 ` [PATCH 05/26] ARCNET: " Julia Lawall
2016-09-11 13:05 ` [PATCH 06/26] ath: " Julia Lawall
@ 2016-09-11 13:05 ` Julia Lawall
2016-09-12 4:59 ` Leon Romanovsky
2016-09-11 13:05 ` [PATCH 08/26] iwlegacy: " Julia Lawall
` (5 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:05 UTC (permalink / raw)
To: Yishai Hadas; +Cc: joe, kernel-janitors, netdev, linux-rdma, linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/ethernet/mellanox/mlx4/main.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/ethernet/mellanox/mlx4/main.c b/drivers/net/ethernet/mellanox/mlx4/main.c
index 75dd2e3..9a3c359 100644
--- a/drivers/net/ethernet/mellanox/mlx4/main.c
+++ b/drivers/net/ethernet/mellanox/mlx4/main.c
@@ -120,7 +120,7 @@ static char mlx4_version[] =
DRV_NAME ": Mellanox ConnectX core driver v"
DRV_VERSION " (" DRV_RELDATE ")\n";
-static struct mlx4_profile default_profile = {
+static const struct mlx4_profile default_profile = {
.num_qp = 1 << 18,
.num_srq = 1 << 16,
.rdmarc_per_qp = 1 << 4,
@@ -130,7 +130,7 @@ static struct mlx4_profile default_profile = {
.num_mtt = 1 << 20, /* It is really num mtt segements */
};
-static struct mlx4_profile low_mem_profile = {
+static const struct mlx4_profile low_mem_profile = {
.num_qp = 1 << 17,
.num_srq = 1 << 6,
.rdmarc_per_qp = 1 << 4,
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 08/26] iwlegacy: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
` (2 preceding siblings ...)
2016-09-11 13:05 ` [PATCH 07/26] net/mlx4_core: " Julia Lawall
@ 2016-09-11 13:05 ` Julia Lawall
2016-09-12 11:24 ` Stanislaw Gruszka
2016-09-11 13:05 ` [PATCH 11/26] can: " Julia Lawall
` (4 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:05 UTC (permalink / raw)
To: Stanislaw Gruszka
Cc: joe, kernel-janitors, Kalle Valo, linux-wireless, netdev,
linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/wireless/intel/iwlegacy/3945.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/wireless/intel/iwlegacy/3945.c b/drivers/net/wireless/intel/iwlegacy/3945.c
index 209dc99..4db327a 100644
--- a/drivers/net/wireless/intel/iwlegacy/3945.c
+++ b/drivers/net/wireless/intel/iwlegacy/3945.c
@@ -2671,7 +2671,7 @@ const struct il_ops il3945_ops = {
.send_led_cmd = il3945_send_led_cmd,
};
-static struct il_cfg il3945_bg_cfg = {
+static const struct il_cfg il3945_bg_cfg = {
.name = "3945BG",
.fw_name_pre = IL3945_FW_PRE,
.ucode_api_max = IL3945_UCODE_API_MAX,
@@ -2700,7 +2700,7 @@ static struct il_cfg il3945_bg_cfg = {
},
};
-static struct il_cfg il3945_abg_cfg = {
+static const struct il_cfg il3945_abg_cfg = {
.name = "3945ABG",
.fw_name_pre = IL3945_FW_PRE,
.ucode_api_max = IL3945_UCODE_API_MAX,
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 11/26] can: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
` (3 preceding siblings ...)
2016-09-11 13:05 ` [PATCH 08/26] iwlegacy: " Julia Lawall
@ 2016-09-11 13:05 ` Julia Lawall
2016-09-12 12:33 ` Julia Lawall
2016-09-11 13:06 ` [PATCH 20/26] stmmac: pci: " Julia Lawall
` (3 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:05 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: joe, kernel-janitors, Marc Kleine-Budde, linux-can, netdev,
linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/can/c_can/c_can_pci.c | 4 ++--
drivers/net/can/sja1000/plx_pci.c | 20 ++++++++++----------
2 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c
index 7be393c..4bc345d 100644
--- a/drivers/net/can/c_can/c_can_pci.c
+++ b/drivers/net/can/c_can/c_can_pci.c
@@ -251,14 +251,14 @@ static void c_can_pci_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-static struct c_can_pci_data c_can_sta2x11= {
+static const struct c_can_pci_data c_can_sta2x11 = {
.type = BOSCH_C_CAN,
.reg_align = C_CAN_REG_ALIGN_32,
.freq = 52000000, /* 52 Mhz */
.bar = 0,
};
-static struct c_can_pci_data c_can_pch = {
+static const struct c_can_pci_data c_can_pch = {
.type = BOSCH_C_CAN,
.reg_align = C_CAN_REG_32,
.freq = 50000000, /* 50 MHz */
diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
index 3eb7430..59bc378 100644
--- a/drivers/net/can/sja1000/plx_pci.c
+++ b/drivers/net/can/sja1000/plx_pci.c
@@ -170,7 +170,7 @@ struct plx_pci_card_info {
void (*reset_func)(struct pci_dev *pdev);
};
-static struct plx_pci_card_info plx_pci_card_info_adlink = {
+static const struct plx_pci_card_info plx_pci_card_info_adlink = {
"Adlink PCI-7841/cPCI-7841", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{1, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x80, 0x80} },
@@ -178,7 +178,7 @@ static struct plx_pci_card_info plx_pci_card_info_adlink = {
/* based on PLX9052 */
};
-static struct plx_pci_card_info plx_pci_card_info_adlink_se = {
+static const struct plx_pci_card_info plx_pci_card_info_adlink_se = {
"Adlink PCI-7841/cPCI-7841 SE", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x80, 0x80} },
@@ -186,7 +186,7 @@ static struct plx_pci_card_info plx_pci_card_info_adlink_se = {
/* based on PLX9052 */
};
-static struct plx_pci_card_info plx_pci_card_info_esd200 = {
+static const struct plx_pci_card_info plx_pci_card_info_esd200 = {
"esd CAN-PCI/CPCI/PCI104/200", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
@@ -194,7 +194,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd200 = {
/* based on PLX9030/9050 */
};
-static struct plx_pci_card_info plx_pci_card_info_esd266 = {
+static const struct plx_pci_card_info plx_pci_card_info_esd266 = {
"esd CAN-PCI/PMC/266", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
@@ -202,7 +202,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd266 = {
/* based on PLX9056 */
};
-static struct plx_pci_card_info plx_pci_card_info_esd2000 = {
+static const struct plx_pci_card_info plx_pci_card_info_esd2000 = {
"esd CAN-PCIe/2000", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
@@ -210,7 +210,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd2000 = {
/* based on PEX8311 */
};
-static struct plx_pci_card_info plx_pci_card_info_ixxat = {
+static const struct plx_pci_card_info plx_pci_card_info_ixxat = {
"IXXAT PC-I 04/PCI", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x200, 0x80} },
@@ -218,7 +218,7 @@ static struct plx_pci_card_info plx_pci_card_info_ixxat = {
/* based on PLX9050 */
};
-static struct plx_pci_card_info plx_pci_card_info_marathon_pci = {
+static const struct plx_pci_card_info plx_pci_card_info_marathon_pci = {
"Marathon CAN-bus-PCI", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x00, 0x00}, {4, 0x00, 0x00} },
@@ -234,7 +234,7 @@ static struct plx_pci_card_info plx_pci_card_info_marathon_pcie = {
/* based on PEX8311 */
};
-static struct plx_pci_card_info plx_pci_card_info_tews = {
+static const struct plx_pci_card_info plx_pci_card_info_tews = {
"TEWS TECHNOLOGIES TPMC810", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x000, 0x80}, {2, 0x100, 0x80} },
@@ -242,7 +242,7 @@ static struct plx_pci_card_info plx_pci_card_info_tews = {
/* based on PLX9030 */
};
-static struct plx_pci_card_info plx_pci_card_info_cti = {
+static const struct plx_pci_card_info plx_pci_card_info_cti = {
"Connect Tech Inc. CANpro/104-Plus Opto (CRG001)", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{0, 0x00, 0x00}, { {2, 0x000, 0x80}, {2, 0x100, 0x80} },
@@ -250,7 +250,7 @@ static struct plx_pci_card_info plx_pci_card_info_cti = {
/* based on PLX9030 */
};
-static struct plx_pci_card_info plx_pci_card_info_elcus = {
+static const struct plx_pci_card_info plx_pci_card_info_elcus = {
"Eclus CAN-200-PCI", 2,
PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
{1, 0x00, 0x00}, { {2, 0x00, 0x80}, {3, 0x00, 0x80} },
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 20/26] stmmac: pci: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
` (4 preceding siblings ...)
2016-09-11 13:05 ` [PATCH 11/26] can: " Julia Lawall
@ 2016-09-11 13:06 ` Julia Lawall
2016-09-12 12:08 ` Julia Lawall
2016-09-11 13:06 ` [PATCH 23/26] sh_eth: " Julia Lawall
` (2 subsequent siblings)
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:06 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: joe, kernel-janitors, Alexandre Torgue, netdev, linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
index 56c8a23..5c612c3 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
@@ -141,7 +141,7 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
{}
};
-static struct stmmac_pci_info quark_pci_info = {
+static const struct stmmac_pci_info quark_pci_info = {
.setup = quark_default_data,
.dmi = quark_pci_dmi_data,
};
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 21/26] rtlwifi: rtl818x: constify local structures
[not found] ` <1473599168-30561-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
@ 2016-09-11 13:06 ` Julia Lawall
2016-09-11 17:21 ` [PATCH 00/26] " Jarkko Sakkinen
2016-09-11 17:56 ` Joe Perches
2 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:06 UTC (permalink / raw)
To: Larry Finger
Cc: joe-6d6DIl74uiNBDgjK7y7TUQ,
kernel-janitors-u79uwXL29TY76Z2rM5mHXA, Chaoming Li, Kalle Valo,
linux-wireless-u79uwXL29TY76Z2rM5mHXA,
netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe-6d6DIl74uiNBDgjK7y7TUQ@public.gmane.org>.
Signed-off-by: Julia Lawall <Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 2 +-
drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 2 +-
8 files changed, 8 insertions(+), 8 deletions(-)
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c
index 47e32cb..e7b11b4 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c
@@ -280,7 +280,7 @@ static struct rtl_mod_params rtl88ee_mod_params = {
.debug = DBG_EMERG,
};
-static struct rtl_hal_cfg rtl88ee_hal_cfg = {
+static const struct rtl_hal_cfg rtl88ee_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl88e_pci",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c
index 4780bdc..87aa209 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c
@@ -258,7 +258,7 @@ static struct rtl_mod_params rtl92ce_mod_params = {
.debug = DBG_EMERG,
};
-static struct rtl_hal_cfg rtl92ce_hal_cfg = {
+static const struct rtl_hal_cfg rtl92ce_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl92c_pci",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c
index c6e09a1..0538a4d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c
@@ -262,7 +262,7 @@ static struct rtl_mod_params rtl92de_mod_params = {
.debug = DBG_EMERG,
};
-static struct rtl_hal_cfg rtl92de_hal_cfg = {
+static const struct rtl_hal_cfg rtl92de_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8192de",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c
index c31c6bf..ac299cb 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c
@@ -262,7 +262,7 @@ static struct rtl_mod_params rtl92ee_mod_params = {
.debug = DBG_EMERG,
};
-static struct rtl_hal_cfg rtl92ee_hal_cfg = {
+static const struct rtl_hal_cfg rtl92ee_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl92ee_pci",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c
index 31baca41..5e8e02d 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c
@@ -306,7 +306,7 @@ static struct rtl_mod_params rtl92se_mod_params = {
/* Because memory R/W bursting will cause system hang/crash
* for 92se, so we don't read back after every write action */
-static struct rtl_hal_cfg rtl92se_hal_cfg = {
+static const struct rtl_hal_cfg rtl92se_hal_cfg = {
.bar_id = 1,
.write_readback = false,
.name = "rtl92s_pci",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c
index ff49a8c..89c828a 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c
@@ -276,7 +276,7 @@ static struct rtl_mod_params rtl8723e_mod_params = {
.disable_watchdog = false,
};
-static struct rtl_hal_cfg rtl8723e_hal_cfg = {
+static const struct rtl_hal_cfg rtl8723e_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8723e_pci",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c
index 2101793..20b53f0 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c
@@ -276,7 +276,7 @@ static struct rtl_mod_params rtl8723be_mod_params = {
.ant_sel = 0,
};
-static struct rtl_hal_cfg rtl8723be_hal_cfg = {
+static const struct rtl_hal_cfg rtl8723be_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8723be_pci",
diff --git a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c
index 4159f9b..22f687b1 100644
--- a/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c
+++ b/drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c
@@ -316,7 +316,7 @@ static struct rtl_mod_params rtl8821ae_mod_params = {
.disable_watchdog = 0,
};
-static struct rtl_hal_cfg rtl8821ae_hal_cfg = {
+static const struct rtl_hal_cfg rtl8821ae_hal_cfg = {
.bar_id = 2,
.write_readback = true,
.name = "rtl8821ae_pci",
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 23/26] sh_eth: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
` (5 preceding siblings ...)
2016-09-11 13:06 ` [PATCH 20/26] stmmac: pci: " Julia Lawall
@ 2016-09-11 13:06 ` Julia Lawall
2016-09-11 18:14 ` Sergei Shtylyov
2016-09-11 13:06 ` [PATCH 25/26] pch_gbe: " Julia Lawall
[not found] ` <1473599168-30561-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
8 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:06 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: joe, kernel-janitors, netdev, linux-renesas-soc, linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/ethernet/renesas/sh_eth.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/renesas/sh_eth.c b/drivers/net/ethernet/renesas/sh_eth.c
index 1f8240a..d2ed57f 100644
--- a/drivers/net/ethernet/renesas/sh_eth.c
+++ b/drivers/net/ethernet/renesas/sh_eth.c
@@ -654,7 +654,7 @@ static void sh_eth_set_rate_sh7724(struct net_device *ndev)
}
/* SH7724 */
-static struct sh_eth_cpu_data sh7724_data = {
+static const struct sh_eth_cpu_data sh7724_data = {
.set_duplex = sh_eth_set_duplex,
.set_rate = sh_eth_set_rate_sh7724,
@@ -692,7 +692,7 @@ static void sh_eth_set_rate_sh7757(struct net_device *ndev)
}
/* SH7757 */
-static struct sh_eth_cpu_data sh7757_data = {
+static const struct sh_eth_cpu_data sh7757_data = {
.set_duplex = sh_eth_set_duplex,
.set_rate = sh_eth_set_rate_sh7757,
@@ -757,7 +757,7 @@ static void sh_eth_set_rate_giga(struct net_device *ndev)
}
/* SH7757(GETHERC) */
-static struct sh_eth_cpu_data sh7757_data_giga = {
+static const struct sh_eth_cpu_data sh7757_data_giga = {
.chip_reset = sh_eth_chip_reset_giga,
.set_duplex = sh_eth_set_duplex,
.set_rate = sh_eth_set_rate_giga,
@@ -788,7 +788,7 @@ static struct sh_eth_cpu_data sh7757_data_giga = {
};
/* SH7734 */
-static struct sh_eth_cpu_data sh7734_data = {
+static const struct sh_eth_cpu_data sh7734_data = {
.chip_reset = sh_eth_chip_reset,
.set_duplex = sh_eth_set_duplex,
.set_rate = sh_eth_set_rate_gether,
@@ -817,7 +817,7 @@ static struct sh_eth_cpu_data sh7734_data = {
};
/* SH7763 */
-static struct sh_eth_cpu_data sh7763_data = {
+static const struct sh_eth_cpu_data sh7763_data = {
.chip_reset = sh_eth_chip_reset,
.set_duplex = sh_eth_set_duplex,
.set_rate = sh_eth_set_rate_gether,
@@ -844,7 +844,7 @@ static struct sh_eth_cpu_data sh7763_data = {
.irq_flags = IRQF_SHARED,
};
-static struct sh_eth_cpu_data sh7619_data = {
+static const struct sh_eth_cpu_data sh7619_data = {
.register_type = SH_ETH_REG_FAST_SH3_SH2,
.eesipr_value = DMAC_M_RFRMER | DMAC_M_ECI | 0x003fffff,
@@ -855,7 +855,7 @@ static struct sh_eth_cpu_data sh7619_data = {
.hw_swap = 1,
};
-static struct sh_eth_cpu_data sh771x_data = {
+static const struct sh_eth_cpu_data sh771x_data = {
.register_type = SH_ETH_REG_FAST_SH3_SH2,
.eesipr_value = DMAC_M_RFRMER | DMAC_M_ECI | 0x003fffff,
^ permalink raw reply related [flat|nested] 35+ messages in thread
* [PATCH 25/26] pch_gbe: constify local structures
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
` (6 preceding siblings ...)
2016-09-11 13:06 ` [PATCH 23/26] sh_eth: " Julia Lawall
@ 2016-09-11 13:06 ` Julia Lawall
2016-09-12 2:48 ` David Miller
2016-09-12 12:25 ` Julia Lawall
[not found] ` <1473599168-30561-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
8 siblings, 2 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 13:06 UTC (permalink / raw)
To: netdev; +Cc: joe, kernel-janitors, linux-kernel
For structure types defined in the same file or local header files, find
top-level static structure declarations that have the following
properties:
1. Never reassigned.
2. Address never taken
3. Not passed to a top-level macro call
4. No pointer or array-typed field passed to a function or stored in a
variable.
Declare structures having all of these properties as const.
Done using Coccinelle.
Based on a suggestion by Joe Perches <joe@perches.com>.
Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
---
The semantic patch seems too long for a commit log, but is in the cover
letter.
drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
index 3cd87a4..6f33258 100644
--- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
+++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
@@ -2729,7 +2729,7 @@ static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
return ret;
}
-static struct pch_gbe_privdata pch_gbe_minnow_privdata = {
+static const struct pch_gbe_privdata pch_gbe_minnow_privdata = {
.phy_tx_clk_delay = true,
.phy_disable_hibernate = true,
.platform_init = pch_gbe_minnow_platform_init,
^ permalink raw reply related [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
[not found] ` <1473599168-30561-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
2016-09-11 13:06 ` [PATCH 21/26] rtlwifi: rtl818x: " Julia Lawall
@ 2016-09-11 17:21 ` Jarkko Sakkinen
2016-09-12 8:54 ` Julia Lawall
2016-09-11 17:56 ` Joe Perches
2 siblings, 1 reply; 35+ messages in thread
From: Jarkko Sakkinen @ 2016-09-11 17:21 UTC (permalink / raw)
To: Julia Lawall
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Mustafa Ismail,
Tatyana Nikolova, kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-can-u79uwXL29TY76Z2rM5mHXA,
Shiraz Saleem, Sergei Shtylyov, netdev-u79uwXL29TY76Z2rM5mHXA,
Chien Tin Tung, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA, joe-6d6DIl74uiNBDgjK7y7TUQ
On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> Constify local structures.
>
> The semantic patch that makes this change is as follows:
> (http://coccinelle.lip6.fr/)
Just my two cents but:
1. You *can* use a static analysis too to find bugs or other issues.
2. However, you should manually do the commits and proper commit
messages to subsystems based on your findings. And I generally think
that if one contributes code one should also at least smoke test changes
somehow.
I don't know if I'm alone with my opinion. I just think that one should
also do the analysis part and not blindly create and submit patches.
Anyway, I'll apply the TPM change at some point. As I said they were
for better. Thanks.
/Jarkko
> // <smpl>
> // The first rule ignores some cases that posed problems
> @r disable optional_qualifier@
> identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
> identifier i != {s5k5baf_cis_rect,smtcfb_fix};
> position p;
> @@
> static struct s i@p = { ... };
>
> @lstruct@
> identifier r.s;
> @@
> struct s { ... };
>
> @used depends on lstruct@
> identifier r.i;
> @@
> i
>
> @bad1@
> expression e;
> identifier r.i;
> assignment operator a;
> @@
> (<+...i...+>) a e
>
> @bad2@
> identifier r.i;
> @@
> &(<+...i...+>)
>
> @bad3@
> identifier r.i;
> declarer d;
> @@
> d(...,<+...i...+>,...);
>
> @bad4@
> identifier r.i;
> type T;
> T[] e;
> identifier f;
> position p;
> @@
>
> f@p(...,
> (
> (<+...i...+>)
> &
> e
> )
> ,...)
>
> @bad4a@
> identifier r.i;
> type T;
> T *e;
> identifier f;
> position p;
> @@
>
> f@p(...,
> (
> (<+...i...+>)
> &
> e
> )
> ,...)
>
> @ok5@
> expression *e;
> identifier r.i;
> position p;
> @@
> e =@p i
>
> @bad5@
> expression *e;
> identifier r.i;
> position p != ok5.p;
> @@
> e =@p (<+...i...+>)
>
> @rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
> identifier s,r.i;
> position r.p;
> @@
>
> static
> +const
> struct s i@p = { ... };
>
> @depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
> disable optional_qualifier@
> identifier rr.s,r.i;
> @@
>
> static
> +const
> struct s i;
> // </smpl>
>
> ---
>
> drivers/acpi/acpi_apd.c | 8 +++---
> drivers/char/tpm/tpm-interface.c | 10 ++++----
> drivers/char/tpm/tpm-sysfs.c | 2 -
> drivers/cpufreq/intel_pstate.c | 8 +++---
> drivers/infiniband/hw/i40iw/i40iw_uk.c | 6 ++---
> drivers/media/i2c/tvp514x.c | 2 -
> drivers/media/pci/ddbridge/ddbridge-core.c | 18 +++++++--------
> drivers/media/pci/ngene/ngene-cards.c | 14 ++++++------
> drivers/media/pci/smipcie/smipcie-main.c | 8 +++---
> drivers/misc/sgi-xp/xpc_uv.c | 2 -
> drivers/net/arcnet/com20020-pci.c | 10 ++++----
> drivers/net/can/c_can/c_can_pci.c | 4 +--
> drivers/net/can/sja1000/plx_pci.c | 20 ++++++++---------
> drivers/net/ethernet/mellanox/mlx4/main.c | 4 +--
> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 -
> drivers/net/ethernet/renesas/sh_eth.c | 14 ++++++------
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 -
> drivers/net/wireless/ath/dfs_pattern_detector.c | 2 -
> drivers/net/wireless/intel/iwlegacy/3945.c | 4 +--
> drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 2 -
> drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 2 -
> drivers/platform/chrome/chromeos_laptop.c | 22 +++++++++----------
> drivers/platform/x86/intel_scu_ipc.c | 6 ++---
> drivers/platform/x86/intel_telemetry_debugfs.c | 2 -
> drivers/scsi/esas2r/esas2r_flash.c | 2 -
> drivers/scsi/hptiop.c | 6 ++---
> drivers/spi/spi-dw-pci.c | 4 +--
> drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 -
> drivers/usb/misc/ezusb.c | 2 -
> drivers/video/fbdev/matrox/matroxfb_g450.c | 2 -
> lib/crc64_ecma.c | 2 -
> sound/pci/ctxfi/ctatc.c | 2 -
> sound/pci/hda/patch_ca0132.c | 10 ++++----
> sound/pci/riptide/riptide.c | 2 -
> 40 files changed, 110 insertions(+), 110 deletions(-)
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
[not found] ` <1473599168-30561-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
2016-09-11 13:06 ` [PATCH 21/26] rtlwifi: rtl818x: " Julia Lawall
2016-09-11 17:21 ` [PATCH 00/26] " Jarkko Sakkinen
@ 2016-09-11 17:56 ` Joe Perches
2016-09-11 19:11 ` Julia Lawall
2 siblings, 1 reply; 35+ messages in thread
From: Joe Perches @ 2016-09-11 17:56 UTC (permalink / raw)
To: Julia Lawall, linux-renesas-soc-u79uwXL29TY76Z2rM5mHXA
Cc: alsa-devel-K7yf7f+aM1XWsZ/bQMPhNw, Mustafa Ismail,
Tatyana Nikolova, kernel-janitors-u79uwXL29TY76Z2rM5mHXA,
linux-fbdev-u79uwXL29TY76Z2rM5mHXA,
platform-driver-x86-u79uwXL29TY76Z2rM5mHXA,
devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b,
linux-scsi-u79uwXL29TY76Z2rM5mHXA,
linux-rdma-u79uwXL29TY76Z2rM5mHXA,
linux-acpi-u79uwXL29TY76Z2rM5mHXA,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-media-u79uwXL29TY76Z2rM5mHXA,
linux-pm-u79uwXL29TY76Z2rM5mHXA, linux-can-u79uwXL29TY76Z2rM5mHXA,
Shiraz Saleem, Sergei Shtylyov, netdev-u79uwXL29TY76Z2rM5mHXA,
Chien Tin Tung, linux-wireless-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-spi-u79uwXL29TY76Z2rM5mHXA,
linux-usb-u79uwXL29TY76Z2rM5mHXA
On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote:
> Constify local structures.
Thanks Julia.
A few suggestions & questions:
Perhaps the script should go into scripts/coccinelle/
so that future cases could be caught by the robot
and commit message referenced by the patch instances.
Can you please compile the files modified using the
appropriate defconfig/allyesconfig and show the
movement from data to const by using
$ size <object>.new/old
and include that in the changelogs (maybe next time)?
Is it possible for a rule to trace the instances where
an address of a struct or struct member is taken by
locally defined and declared function call where the
callee does not modify any dereferenced object?
ie:
struct foo {
int bar;
char *baz;
};
struct foo qux[] = {
{ 1, "description 1" },
{ 2, "dewcription 2" },
[ n, "etc" ]...,
};
void message(struct foo *msg)
{
printk("%d %s\n", msg->bar, msg->baz);
}
where some code uses
message(qux[index]);
So could a coccinelle script change:
struct foo qux[] = { to const struct foo quz[] = {
and
void message(struct foo *msg) to void message(const struct foo *msg)
------------------------------------------------------------------------------
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 23/26] sh_eth: constify local structures
2016-09-11 13:06 ` [PATCH 23/26] sh_eth: " Julia Lawall
@ 2016-09-11 18:14 ` Sergei Shtylyov
2016-09-12 8:55 ` Julia Lawall
2016-09-12 9:43 ` Julia Lawall
0 siblings, 2 replies; 35+ messages in thread
From: Sergei Shtylyov @ 2016-09-11 18:14 UTC (permalink / raw)
To: Julia Lawall
Cc: joe, kernel-janitors, netdev, linux-renesas-soc, linux-kernel
On 09/11/2016 04:06 PM, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
Really?
> 2. Address never taken
Really?
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
>
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
NAK, see sh_eth_set_default_cpu_data().
MBR, Sergei
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-11 17:56 ` Joe Perches
@ 2016-09-11 19:11 ` Julia Lawall
0 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-11 19:11 UTC (permalink / raw)
To: Joe Perches
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, platform-driver-x86, devel, linux-scsi, linux-rdma,
Jason Gunthorpe, linux-acpi, tpmdd-devel, linux-media, linux-pm,
linux-can, Julia Lawall, Shiraz Saleem, Sergei Shtylyov, netdev,
Chien Tin Tung, linux-wireless, linux-kernel, linux-spi,
linux-renesas-soc, linux-usb
On Sun, 11 Sep 2016, Joe Perches wrote:
> On Sun, 2016-09-11 at 15:05 +0200, Julia Lawall wrote:
> > Constify local structures.
>
> Thanks Julia.
>
> A few suggestions & questions:
>
> Perhaps the script should go into scripts/coccinelle/
> so that future cases could be caught by the robot
> and commit message referenced by the patch instances.
OK.
> Can you please compile the files modified using the
> appropriate defconfig/allyesconfig and show the
I currently send patches for this issue only for files that compile using
the x86 allyesconfig.
> movement from data to const by using
> $ size <object>.new/old
> and include that in the changelogs (maybe next time)?
OK, thanks for the suggestion.
> Is it possible for a rule to trace the instances where
> an address of a struct or struct member is taken by
> locally defined and declared function call where the
> callee does not modify any dereferenced object?
>
> ie:
>
> struct foo {
> int bar;
> char *baz;
> };
>
> struct foo qux[] = {
> { 1, "description 1" },
> { 2, "dewcription 2" },
> [ n, "etc" ]...,
> };
>
> void message(struct foo *msg)
> {
> printk("%d %s\n", msg->bar, msg->baz);
> }
>
> where some code uses
>
> message(qux[index]);
>
> So could a coccinelle script change:
>
> struct foo qux[] = { to const struct foo quz[] = {
>
> and
>
> void message(struct foo *msg) to void message(const struct foo *msg)
Yes, this could be possible too.
Thanks for the feedback.
julia
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 25/26] pch_gbe: constify local structures
2016-09-11 13:06 ` [PATCH 25/26] pch_gbe: " Julia Lawall
@ 2016-09-12 2:48 ` David Miller
2016-09-12 9:07 ` Julia Lawall
2016-09-12 12:25 ` Julia Lawall
1 sibling, 1 reply; 35+ messages in thread
From: David Miller @ 2016-09-12 2:48 UTC (permalink / raw)
To: Julia.Lawall; +Cc: netdev, joe, kernel-janitors, linux-kernel
Julia, I went over the networking driver patches in this series and
I have to say that I'd rather see these changes be more durable
and self-checking.
By this I mean that I want you to also make the driver private pointer
that holds these structures be const too.
Then if there are really any assignments to the objects being marked
const, it will show immediately.
Thank you.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 07/26] net/mlx4_core: constify local structures
2016-09-11 13:05 ` [PATCH 07/26] net/mlx4_core: " Julia Lawall
@ 2016-09-12 4:59 ` Leon Romanovsky
0 siblings, 0 replies; 35+ messages in thread
From: Leon Romanovsky @ 2016-09-12 4:59 UTC (permalink / raw)
To: Julia Lawall
Cc: Yishai Hadas, joe, kernel-janitors, netdev, linux-rdma,
linux-kernel
[-- Attachment #1: Type: text/plain, Size: 654 bytes --]
On Sun, Sep 11, 2016 at 03:05:49PM +0200, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
>
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Thanks,
Reviewed-by: Leon Romanovsky <leonro@mellanox.com>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-11 17:21 ` [PATCH 00/26] " Jarkko Sakkinen
@ 2016-09-12 8:54 ` Julia Lawall
2016-09-12 13:16 ` Jarkko Sakkinen
0 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 8:54 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Julia Lawall, linux-renesas-soc, joe, kernel-janitors,
Sergei Shtylyov, linux-pm, platform-driver-x86, linux-media,
linux-can, Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail,
Chien Tin Tung, linux-rdma, netdev, devel, alsa-devel,
linux-kernel, linux-fbdev, linux-wireless, Jason Gunthorpe,
tpmdd-devel, linux-scsi
On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > Constify local structures.
> >
> > The semantic patch that makes this change is as follows:
> > (http://coccinelle.lip6.fr/)
>
> Just my two cents but:
>
> 1. You *can* use a static analysis too to find bugs or other issues.
> 2. However, you should manually do the commits and proper commit
> messages to subsystems based on your findings. And I generally think
> that if one contributes code one should also at least smoke test changes
> somehow.
>
> I don't know if I'm alone with my opinion. I just think that one should
> also do the analysis part and not blindly create and submit patches.
All of the patches are compile tested. And the individual patches are
submitted to the relevant maintainers. The individual commit messages
give a more detailed explanation of the strategy used to decide that the
structure was constifiable. It seemed redundant to put that in the cover
letter, which will not be committed anyway.
julia
>
> Anyway, I'll apply the TPM change at some point. As I said they were
> for better. Thanks.
>
> /Jarkko
>
> > // <smpl>
> > // The first rule ignores some cases that posed problems
> > @r disable optional_qualifier@
> > identifier s != {peri_clk_data,threshold_attr,tracer_flags,tracer};
> > identifier i != {s5k5baf_cis_rect,smtcfb_fix};
> > position p;
> > @@
> > static struct s i@p = { ... };
> >
> > @lstruct@
> > identifier r.s;
> > @@
> > struct s { ... };
> >
> > @used depends on lstruct@
> > identifier r.i;
> > @@
> > i
> >
> > @bad1@
> > expression e;
> > identifier r.i;
> > assignment operator a;
> > @@
> > (<+...i...+>) a e
> >
> > @bad2@
> > identifier r.i;
> > @@
> > &(<+...i...+>)
> >
> > @bad3@
> > identifier r.i;
> > declarer d;
> > @@
> > d(...,<+...i...+>,...);
> >
> > @bad4@
> > identifier r.i;
> > type T;
> > T[] e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> > (<+...i...+>)
> > &
> > e
> > )
> > ,...)
> >
> > @bad4a@
> > identifier r.i;
> > type T;
> > T *e;
> > identifier f;
> > position p;
> > @@
> >
> > f@p(...,
> > (
> > (<+...i...+>)
> > &
> > e
> > )
> > ,...)
> >
> > @ok5@
> > expression *e;
> > identifier r.i;
> > position p;
> > @@
> > e =@p i
> >
> > @bad5@
> > expression *e;
> > identifier r.i;
> > position p != ok5.p;
> > @@
> > e =@p (<+...i...+>)
> >
> > @rr depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5@
> > identifier s,r.i;
> > position r.p;
> > @@
> >
> > static
> > +const
> > struct s i@p = { ... };
> >
> > @depends on used && !bad1 && !bad2 && !bad3 && !bad4 && !bad4a && !bad5
> > disable optional_qualifier@
> > identifier rr.s,r.i;
> > @@
> >
> > static
> > +const
> > struct s i;
> > // </smpl>
> >
> > ---
> >
> > drivers/acpi/acpi_apd.c | 8 +++---
> > drivers/char/tpm/tpm-interface.c | 10 ++++----
> > drivers/char/tpm/tpm-sysfs.c | 2 -
> > drivers/cpufreq/intel_pstate.c | 8 +++---
> > drivers/infiniband/hw/i40iw/i40iw_uk.c | 6 ++---
> > drivers/media/i2c/tvp514x.c | 2 -
> > drivers/media/pci/ddbridge/ddbridge-core.c | 18 +++++++--------
> > drivers/media/pci/ngene/ngene-cards.c | 14 ++++++------
> > drivers/media/pci/smipcie/smipcie-main.c | 8 +++---
> > drivers/misc/sgi-xp/xpc_uv.c | 2 -
> > drivers/net/arcnet/com20020-pci.c | 10 ++++----
> > drivers/net/can/c_can/c_can_pci.c | 4 +--
> > drivers/net/can/sja1000/plx_pci.c | 20 ++++++++---------
> > drivers/net/ethernet/mellanox/mlx4/main.c | 4 +--
> > drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 -
> > drivers/net/ethernet/renesas/sh_eth.c | 14 ++++++------
> > drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 -
> > drivers/net/wireless/ath/dfs_pattern_detector.c | 2 -
> > drivers/net/wireless/intel/iwlegacy/3945.c | 4 +--
> > drivers/net/wireless/realtek/rtlwifi/rtl8188ee/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8192ce/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8192de/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8192ee/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8192se/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8723ae/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8723be/sw.c | 2 -
> > drivers/net/wireless/realtek/rtlwifi/rtl8821ae/sw.c | 2 -
> > drivers/platform/chrome/chromeos_laptop.c | 22 +++++++++----------
> > drivers/platform/x86/intel_scu_ipc.c | 6 ++---
> > drivers/platform/x86/intel_telemetry_debugfs.c | 2 -
> > drivers/scsi/esas2r/esas2r_flash.c | 2 -
> > drivers/scsi/hptiop.c | 6 ++---
> > drivers/spi/spi-dw-pci.c | 4 +--
> > drivers/staging/rtl8192e/rtl8192e/rtl_core.c | 2 -
> > drivers/usb/misc/ezusb.c | 2 -
> > drivers/video/fbdev/matrox/matroxfb_g450.c | 2 -
> > lib/crc64_ecma.c | 2 -
> > sound/pci/ctxfi/ctatc.c | 2 -
> > sound/pci/hda/patch_ca0132.c | 10 ++++----
> > sound/pci/riptide/riptide.c | 2 -
> > 40 files changed, 110 insertions(+), 110 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 23/26] sh_eth: constify local structures
2016-09-11 18:14 ` Sergei Shtylyov
@ 2016-09-12 8:55 ` Julia Lawall
2016-09-12 9:43 ` Julia Lawall
1 sibling, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 8:55 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: joe, kernel-janitors, netdev, linux-renesas-soc, linux-kernel
On Sun, 11 Sep 2016, Sergei Shtylyov wrote:
> On 09/11/2016 04:06 PM, Julia Lawall wrote:
> > For structure types defined in the same file or local header files, find
> > top-level static structure declarations that have the following
> > properties:
> > 1. Never reassigned.
>
> Really?
>
> > 2. Address never taken
>
> Really?
>
> > 3. Not passed to a top-level macro call
> > 4. No pointer or array-typed field passed to a function or stored in a
> > variable.
> > Declare structures having all of these properties as const.
> >
> > Done using Coccinelle.
> > Based on a suggestion by Joe Perches <joe@perches.com>.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> NAK, see sh_eth_set_default_cpu_data().
Thanks for the feedback. I will check on why this slipped through.
Perhaps it is a variability (ifdef) problem.
julia
>
> MBR, Sergei
>
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 25/26] pch_gbe: constify local structures
2016-09-12 2:48 ` David Miller
@ 2016-09-12 9:07 ` Julia Lawall
2016-09-12 16:33 ` David Miller
0 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 9:07 UTC (permalink / raw)
To: David Miller; +Cc: netdev, joe, kernel-janitors, linux-kernel
On Sun, 11 Sep 2016, David Miller wrote:
>
> Julia, I went over the networking driver patches in this series and
> I have to say that I'd rather see these changes be more durable
> and self-checking.
>
> By this I mean that I want you to also make the driver private pointer
> that holds these structures be const too.
Sorry, I'm not sure what you are asking for. In these cases, we often end
up with something like:
static const struct foo = { ... };
and then later
xxx.ops = foo;
So foo is protected, but its lifetime of interest is quite short. But we
can't set the ops field of the type of xxx to be const either, because it
is obviously not - the code above modifies it. Everything would be fine
if ops were of pointer type but not structure type, but that is not the
case in this patch series, because the semantic patch disallows &foo.
There is the __ro_after_init annotation that might help in some cases, but
I have often seen these assignments in probe functions that are not
__init. Kees Cook mentioned some code that could be inserted before and
after an assignment to make a field temporarily writeable, but I haven't
looked into that possibility yet.
Have I misunderstood something?
thanks,
julia
> Then if there are really any assignments to the objects being marked
> const, it will show immediately.
>
> Thank you.
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 23/26] sh_eth: constify local structures
2016-09-11 18:14 ` Sergei Shtylyov
2016-09-12 8:55 ` Julia Lawall
@ 2016-09-12 9:43 ` Julia Lawall
1 sibling, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 9:43 UTC (permalink / raw)
To: Sergei Shtylyov
Cc: joe, kernel-janitors, netdev, linux-renesas-soc, linux-kernel
On Sun, 11 Sep 2016, Sergei Shtylyov wrote:
> On 09/11/2016 04:06 PM, Julia Lawall wrote:
> > For structure types defined in the same file or local header files, find
> > top-level static structure declarations that have the following
> > properties:
> > 1. Never reassigned.
>
> Really?
>
> > 2. Address never taken
>
> Really?
OK, I see the problem. The code has, eg:
{ "sh7724-ether", (kernel_ulong_t)&sh7724_data }
Coccinelle doesn't know anything about kernel_ulong_t, so it assumes that
it is an identifier, and that the whole expression is a bit and, rather
than an address computation. And at the same time, the cast causes the
compiler to discard the const annotation, so it doesn't complain either.
I can update the rule to avoid this case, since taking a bit and on the
address of a top-level structure is not likely to be useful, and I will
check whether any of the other patches are affected by this issue.
Thanks for the report.
julia
>
> > 3. Not passed to a top-level macro call
> > 4. No pointer or array-typed field passed to a function or stored in a
> > variable.
> > Declare structures having all of these properties as const.
> >
> > Done using Coccinelle.
> > Based on a suggestion by Joe Perches <joe@perches.com>.
> >
> > Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> NAK, see sh_eth_set_default_cpu_data().
>
> MBR, Sergei
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 08/26] iwlegacy: constify local structures
2016-09-11 13:05 ` [PATCH 08/26] iwlegacy: " Julia Lawall
@ 2016-09-12 11:24 ` Stanislaw Gruszka
0 siblings, 0 replies; 35+ messages in thread
From: Stanislaw Gruszka @ 2016-09-12 11:24 UTC (permalink / raw)
To: Julia Lawall
Cc: joe, kernel-janitors, Kalle Valo, linux-wireless, netdev,
linux-kernel
On Sun, Sep 11, 2016 at 03:05:50PM +0200, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
>
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Acked-by: Stanislaw Gruszka <sgruszka@redhat.com>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 20/26] stmmac: pci: constify local structures
2016-09-11 13:06 ` [PATCH 20/26] stmmac: pci: " Julia Lawall
@ 2016-09-12 12:08 ` Julia Lawall
0 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 12:08 UTC (permalink / raw)
To: Giuseppe Cavallaro
Cc: joe, kernel-janitors, Alexandre Torgue, netdev, linux-kernel
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Please ignore this patch. Coccinelle incorrectly interpreted
{PCI_VDEVICE(INTEL, STMMAC_QUARK_ID), (kernel_ulong_t)&quark_pci_info}
as a bit and operation, since it did not recognize kernel_ulong_t as a
type.
julia
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> index 56c8a23..5c612c3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_pci.c
> @@ -141,7 +141,7 @@ static struct stmmac_pci_dmi_data quark_pci_dmi_data[] = {
> {}
> };
>
> -static struct stmmac_pci_info quark_pci_info = {
> +static const struct stmmac_pci_info quark_pci_info = {
> .setup = quark_default_data,
> .dmi = quark_pci_dmi_data,
> };
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 25/26] pch_gbe: constify local structures
2016-09-11 13:06 ` [PATCH 25/26] pch_gbe: " Julia Lawall
2016-09-12 2:48 ` David Miller
@ 2016-09-12 12:25 ` Julia Lawall
1 sibling, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 12:25 UTC (permalink / raw)
To: Julia Lawall; +Cc: netdev, joe, kernel-janitors, linux-kernel
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Actually, this patch should be dropped. Coccinelle did not recognize
kernel_ulong_t as a type, so it interpreted
(kernel_ulong_t)&pch_gbe_minnow_privdata
as a bit and operation.
julia
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> index 3cd87a4..6f33258 100644
> --- a/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> +++ b/drivers/net/ethernet/oki-semi/pch_gbe/pch_gbe_main.c
> @@ -2729,7 +2729,7 @@ static int pch_gbe_minnow_platform_init(struct pci_dev *pdev)
> return ret;
> }
>
> -static struct pch_gbe_privdata pch_gbe_minnow_privdata = {
> +static const struct pch_gbe_privdata pch_gbe_minnow_privdata = {
> .phy_tx_clk_delay = true,
> .phy_disable_hibernate = true,
> .platform_init = pch_gbe_minnow_platform_init,
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 05/26] ARCNET: constify local structures
2016-09-11 13:05 ` [PATCH 05/26] ARCNET: " Julia Lawall
@ 2016-09-12 12:31 ` Julia Lawall
0 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 12:31 UTC (permalink / raw)
To: Michael Grzeschik; +Cc: joe, kernel-janitors, netdev, linux-kernel
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Actually, this patch should be dropped. Coccinelle did not recognize
kernel_ulong_t as a type, so it interpreted things like
(kernel_ulong_t)&card_info_10mbit
as a bit and operation.
julia
>
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/arcnet/com20020-pci.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/arcnet/com20020-pci.c b/drivers/net/arcnet/com20020-pci.c
> index 239de38..32b8406 100644
> --- a/drivers/net/arcnet/com20020-pci.c
> +++ b/drivers/net/arcnet/com20020-pci.c
> @@ -264,7 +264,7 @@ static void com20020pci_remove(struct pci_dev *pdev)
> }
> }
>
> -static struct com20020_pci_card_info card_info_10mbit = {
> +static const struct com20020_pci_card_info card_info_10mbit = {
> .name = "ARC-PCI",
> .devcount = 1,
> .chan_map_tbl = {
> @@ -277,7 +277,7 @@ static struct com20020_pci_card_info card_info_10mbit = {
> .flags = ARC_CAN_10MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_5mbit = {
> +static const struct com20020_pci_card_info card_info_5mbit = {
> .name = "ARC-PCI",
> .devcount = 1,
> .chan_map_tbl = {
> @@ -290,7 +290,7 @@ static struct com20020_pci_card_info card_info_5mbit = {
> .flags = ARC_IS_5MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_sohard = {
> +static const struct com20020_pci_card_info card_info_sohard = {
> .name = "PLX-PCI",
> .devcount = 1,
> /* SOHARD needs PCI base addr 4 */
> @@ -304,7 +304,7 @@ static struct com20020_pci_card_info card_info_sohard = {
> .flags = ARC_CAN_10MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_eae_arc1 = {
> +static const struct com20020_pci_card_info card_info_eae_arc1 = {
> .name = "EAE PLX-PCI ARC1",
> .devcount = 1,
> .chan_map_tbl = {
> @@ -329,7 +329,7 @@ static struct com20020_pci_card_info card_info_eae_arc1 = {
> .flags = ARC_CAN_10MBIT,
> };
>
> -static struct com20020_pci_card_info card_info_eae_ma1 = {
> +static const struct com20020_pci_card_info card_info_eae_ma1 = {
> .name = "EAE PLX-PCI MA1",
> .devcount = 2,
> .chan_map_tbl = {
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 11/26] can: constify local structures
2016-09-11 13:05 ` [PATCH 11/26] can: " Julia Lawall
@ 2016-09-12 12:33 ` Julia Lawall
0 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 12:33 UTC (permalink / raw)
To: Wolfgang Grandegger
Cc: joe, kernel-janitors, Marc Kleine-Budde, linux-can, netdev,
linux-kernel
On Sun, 11 Sep 2016, Julia Lawall wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
Actually, this patch should be dropped. Coccinelle did not recognize
kernel_ulong_t as a type, so it interpreted eg
(kernel_ulong_t)&plx_pci_card_info_adlink
as a bit and operation.
julia
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
>
> ---
> The semantic patch seems too long for a commit log, but is in the cover
> letter.
>
> drivers/net/can/c_can/c_can_pci.c | 4 ++--
> drivers/net/can/sja1000/plx_pci.c | 20 ++++++++++----------
> 2 files changed, 12 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/net/can/c_can/c_can_pci.c b/drivers/net/can/c_can/c_can_pci.c
> index 7be393c..4bc345d 100644
> --- a/drivers/net/can/c_can/c_can_pci.c
> +++ b/drivers/net/can/c_can/c_can_pci.c
> @@ -251,14 +251,14 @@ static void c_can_pci_remove(struct pci_dev *pdev)
> pci_disable_device(pdev);
> }
>
> -static struct c_can_pci_data c_can_sta2x11= {
> +static const struct c_can_pci_data c_can_sta2x11 = {
> .type = BOSCH_C_CAN,
> .reg_align = C_CAN_REG_ALIGN_32,
> .freq = 52000000, /* 52 Mhz */
> .bar = 0,
> };
>
> -static struct c_can_pci_data c_can_pch = {
> +static const struct c_can_pci_data c_can_pch = {
> .type = BOSCH_C_CAN,
> .reg_align = C_CAN_REG_32,
> .freq = 50000000, /* 50 MHz */
> diff --git a/drivers/net/can/sja1000/plx_pci.c b/drivers/net/can/sja1000/plx_pci.c
> index 3eb7430..59bc378 100644
> --- a/drivers/net/can/sja1000/plx_pci.c
> +++ b/drivers/net/can/sja1000/plx_pci.c
> @@ -170,7 +170,7 @@ struct plx_pci_card_info {
> void (*reset_func)(struct pci_dev *pdev);
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_adlink = {
> +static const struct plx_pci_card_info plx_pci_card_info_adlink = {
> "Adlink PCI-7841/cPCI-7841", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {1, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x80, 0x80} },
> @@ -178,7 +178,7 @@ static struct plx_pci_card_info plx_pci_card_info_adlink = {
> /* based on PLX9052 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_adlink_se = {
> +static const struct plx_pci_card_info plx_pci_card_info_adlink_se = {
> "Adlink PCI-7841/cPCI-7841 SE", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x80, 0x80} },
> @@ -186,7 +186,7 @@ static struct plx_pci_card_info plx_pci_card_info_adlink_se = {
> /* based on PLX9052 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_esd200 = {
> +static const struct plx_pci_card_info plx_pci_card_info_esd200 = {
> "esd CAN-PCI/CPCI/PCI104/200", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
> @@ -194,7 +194,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd200 = {
> /* based on PLX9030/9050 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_esd266 = {
> +static const struct plx_pci_card_info plx_pci_card_info_esd266 = {
> "esd CAN-PCI/PMC/266", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
> @@ -202,7 +202,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd266 = {
> /* based on PLX9056 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_esd2000 = {
> +static const struct plx_pci_card_info plx_pci_card_info_esd2000 = {
> "esd CAN-PCIe/2000", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x100, 0x80} },
> @@ -210,7 +210,7 @@ static struct plx_pci_card_info plx_pci_card_info_esd2000 = {
> /* based on PEX8311 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_ixxat = {
> +static const struct plx_pci_card_info plx_pci_card_info_ixxat = {
> "IXXAT PC-I 04/PCI", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x80}, {2, 0x200, 0x80} },
> @@ -218,7 +218,7 @@ static struct plx_pci_card_info plx_pci_card_info_ixxat = {
> /* based on PLX9050 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_marathon_pci = {
> +static const struct plx_pci_card_info plx_pci_card_info_marathon_pci = {
> "Marathon CAN-bus-PCI", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x00, 0x00}, {4, 0x00, 0x00} },
> @@ -234,7 +234,7 @@ static struct plx_pci_card_info plx_pci_card_info_marathon_pcie = {
> /* based on PEX8311 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_tews = {
> +static const struct plx_pci_card_info plx_pci_card_info_tews = {
> "TEWS TECHNOLOGIES TPMC810", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x000, 0x80}, {2, 0x100, 0x80} },
> @@ -242,7 +242,7 @@ static struct plx_pci_card_info plx_pci_card_info_tews = {
> /* based on PLX9030 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_cti = {
> +static const struct plx_pci_card_info plx_pci_card_info_cti = {
> "Connect Tech Inc. CANpro/104-Plus Opto (CRG001)", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {0, 0x00, 0x00}, { {2, 0x000, 0x80}, {2, 0x100, 0x80} },
> @@ -250,7 +250,7 @@ static struct plx_pci_card_info plx_pci_card_info_cti = {
> /* based on PLX9030 */
> };
>
> -static struct plx_pci_card_info plx_pci_card_info_elcus = {
> +static const struct plx_pci_card_info plx_pci_card_info_elcus = {
> "Eclus CAN-200-PCI", 2,
> PLX_PCI_CAN_CLOCK, PLX_PCI_OCR, PLX_PCI_CDR,
> {1, 0x00, 0x00}, { {2, 0x00, 0x80}, {3, 0x00, 0x80} },
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 8:54 ` Julia Lawall
@ 2016-09-12 13:16 ` Jarkko Sakkinen
2016-09-12 13:23 ` Julia Lawall
2016-09-12 13:43 ` Felipe Balbi
0 siblings, 2 replies; 35+ messages in thread
From: Jarkko Sakkinen @ 2016-09-12 13:16 UTC (permalink / raw)
To: Julia Lawall
Cc: linux-renesas-soc, joe, kernel-janitors, Sergei Shtylyov,
linux-pm, platform-driver-x86, linux-media, linux-can,
Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail, Chien Tin Tung,
linux-rdma, netdev, devel, alsa-devel, linux-kernel, linux-fbdev,
linux-wireless, Jason Gunthorpe, tpmdd-devel, linux-scsi,
linux-spi, l
On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
>
>
> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
>
> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > Constify local structures.
> > >
> > > The semantic patch that makes this change is as follows:
> > > (http://coccinelle.lip6.fr/)
> >
> > Just my two cents but:
> >
> > 1. You *can* use a static analysis too to find bugs or other issues.
> > 2. However, you should manually do the commits and proper commit
> > messages to subsystems based on your findings. And I generally think
> > that if one contributes code one should also at least smoke test changes
> > somehow.
> >
> > I don't know if I'm alone with my opinion. I just think that one should
> > also do the analysis part and not blindly create and submit patches.
>
> All of the patches are compile tested. And the individual patches are
Compile-testing is not testing. If you are not able to test a commit,
you should explain why.
> submitted to the relevant maintainers. The individual commit messages
> give a more detailed explanation of the strategy used to decide that the
> structure was constifiable. It seemed redundant to put that in the cover
> letter, which will not be committed anyway.
I don't mean to be harsh but I do not care about your thought process
*that much* when I review a commit (sometimes it might make sense to
explain that but it depends on the context).
I mostly only care why a particular change makes sense for this
particular subsystem. The report given by a static analysis tool can
be a starting point for making a commit but it's not sufficient.
Based on the report you should look subsystems as individuals.
> julia
/Jarkko
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 13:16 ` Jarkko Sakkinen
@ 2016-09-12 13:23 ` Julia Lawall
2016-09-12 13:43 ` Felipe Balbi
1 sibling, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 13:23 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, platform-driver-x86, devel, linux-scsi, linux-rdma,
Jason Gunthorpe, linux-acpi, tpmdd-devel, linux-media, linux-pm,
linux-can, Shiraz Saleem, Sergei Shtylyov, netdev, Chien Tin Tung,
linux-wireless, linux-kernel, linux-spi, linux-renesas-soc,
linux-usb, joe
On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:
> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >
> >
> > On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >
> > > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > > > Constify local structures.
> > > >
> > > > The semantic patch that makes this change is as follows:
> > > > (http://coccinelle.lip6.fr/)
> > >
> > > Just my two cents but:
> > >
> > > 1. You *can* use a static analysis too to find bugs or other issues.
> > > 2. However, you should manually do the commits and proper commit
> > > messages to subsystems based on your findings. And I generally think
> > > that if one contributes code one should also at least smoke test changes
> > > somehow.
> > >
> > > I don't know if I'm alone with my opinion. I just think that one should
> > > also do the analysis part and not blindly create and submit patches.
> >
> > All of the patches are compile tested. And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
>
> > submitted to the relevant maintainers. The individual commit messages
> > give a more detailed explanation of the strategy used to decide that the
> > structure was constifiable. It seemed redundant to put that in the cover
> > letter, which will not be committed anyway.
>
> I don't mean to be harsh but I do not care about your thought process
> *that much* when I review a commit (sometimes it might make sense to
> explain that but it depends on the context).
>
> I mostly only care why a particular change makes sense for this
> particular subsystem. The report given by a static analysis tool can
> be a starting point for making a commit but it's not sufficient.
> Based on the report you should look subsystems as individuals.
OK, thanks for the feedback.
julia
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 13:16 ` Jarkko Sakkinen
2016-09-12 13:23 ` Julia Lawall
@ 2016-09-12 13:43 ` Felipe Balbi
2016-09-12 13:52 ` Julia Lawall
` (2 more replies)
1 sibling, 3 replies; 35+ messages in thread
From: Felipe Balbi @ 2016-09-12 13:43 UTC (permalink / raw)
To: Jarkko Sakkinen, Julia Lawall
Cc: linux-renesas-soc, joe, kernel-janitors, Sergei Shtylyov,
linux-pm, platform-driver-x86, linux-media, linux-can,
Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail, Chien Tin Tung,
linux-rdma, netdev, devel, alsa-devel, linux-kernel, linux-fbdev,
linux-wireless, Jason Gunthorpe, tpmdd-devel, linux-scsi,
linux-spi, l
[-- Attachment #1: Type: text/plain, Size: 1837 bytes --]
Hi,
Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
>>
>>
>> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
>>
>> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
>> > > Constify local structures.
>> > >
>> > > The semantic patch that makes this change is as follows:
>> > > (http://coccinelle.lip6.fr/)
>> >
>> > Just my two cents but:
>> >
>> > 1. You *can* use a static analysis too to find bugs or other issues.
>> > 2. However, you should manually do the commits and proper commit
>> > messages to subsystems based on your findings. And I generally think
>> > that if one contributes code one should also at least smoke test changes
>> > somehow.
>> >
>> > I don't know if I'm alone with my opinion. I just think that one should
>> > also do the analysis part and not blindly create and submit patches.
>>
>> All of the patches are compile tested. And the individual patches are
>
> Compile-testing is not testing. If you are not able to test a commit,
> you should explain why.
Dude, Julia has been doing semantic patching for years already and
nobody has raised any concerns so far. There's already an expectation
that Coccinelle *works* and Julia's sematic patches are sound.
Besides, adding 'const' is something that causes virtually no functional
changes to the point that build-testing is really all you need. Any
problems caused by adding 'const' to a definition will be seen by build
errors or warnings.
Really, just stop with the pointless discussion and go read a bit about
Coccinelle and what semantic patches are giving you. The work done by
Julia and her peers are INRIA have measurable benefits.
You're really making a thunderstorm in a glass of water.
--
balbi
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 800 bytes --]
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 13:43 ` Felipe Balbi
@ 2016-09-12 13:52 ` Julia Lawall
2016-09-12 18:50 ` Jarkko Sakkinen
2016-09-12 13:57 ` Geert Uytterhoeven
2016-09-12 20:14 ` Jarkko Sakkinen
2 siblings, 1 reply; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 13:52 UTC (permalink / raw)
To: Felipe Balbi
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, Jarkko Sakkinen, platform-driver-x86, devel,
linux-scsi, linux-rdma, Jason Gunthorpe, linux-acpi, tpmdd-devel,
linux-media, linux-pm, linux-can, Julia Lawall, Shiraz Saleem,
Sergei Shtylyov, netdev, Chien Tin Tung, linux-wireless,
linux-kernel, linux-spi, linux-renesas-soc, linux-usb
On Mon, 12 Sep 2016, Felipe Balbi wrote:
>
> Hi,
>
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> > messages to subsystems based on your findings. And I generally think
> >> > that if one contributes code one should also at least smoke test changes
> >> > somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested. And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.
Thanks for the defense, but since a lot of these patches torned out to be
wrong, due to an incorrect parse by Coccinelle, combined with an
unpleasantly lax compiler, Jarkko does have a point that I should have
looked at the patches more carefully. In any case, I have written to the
maintainers relevant to the patches that turned out to be incorrect.
julia
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 13:43 ` Felipe Balbi
2016-09-12 13:52 ` Julia Lawall
@ 2016-09-12 13:57 ` Geert Uytterhoeven
2016-09-12 20:14 ` Jarkko Sakkinen
2 siblings, 0 replies; 35+ messages in thread
From: Geert Uytterhoeven @ 2016-09-12 13:57 UTC (permalink / raw)
To: Felipe Balbi
Cc: Jarkko Sakkinen, Julia Lawall, Linux-Renesas, Joe Perches,
kernel-janitors@vger.kernel.org, Sergei Shtylyov, Linux PM list,
platform-driver-x86, Linux Media Mailing List, linux-can,
Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail, Chien Tin Tung,
linux-rdma, netdev@vger.kernel.org, driverdevel
On Mon, Sep 12, 2016 at 3:43 PM, Felipe Balbi
<felipe.balbi@linux.intel.com> wrote:
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
>> On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
>>> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
>>> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
>>> > > Constify local structures.
>>> > >
>>> > > The semantic patch that makes this change is as follows:
>>> > > (http://coccinelle.lip6.fr/)
>>> >
>>> > Just my two cents but:
>>> >
>>> > 1. You *can* use a static analysis too to find bugs or other issues.
>>> > 2. However, you should manually do the commits and proper commit
>>> > messages to subsystems based on your findings. And I generally think
>>> > that if one contributes code one should also at least smoke test changes
>>> > somehow.
>>> >
>>> > I don't know if I'm alone with my opinion. I just think that one should
>>> > also do the analysis part and not blindly create and submit patches.
>>>
>>> All of the patches are compile tested. And the individual patches are
>>
>> Compile-testing is not testing. If you are not able to test a commit,
>> you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
+1
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
Unfortunately in this particular case they could lead to failures that can only
be detected at runtime, when failing o write to a read-only piece of memory,
due to the casting away of the constness of the pointers later.
Fortunately this was detected during code review (doh...).
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 25/26] pch_gbe: constify local structures
2016-09-12 9:07 ` Julia Lawall
@ 2016-09-12 16:33 ` David Miller
0 siblings, 0 replies; 35+ messages in thread
From: David Miller @ 2016-09-12 16:33 UTC (permalink / raw)
To: julia.lawall; +Cc: netdev, joe, kernel-janitors, linux-kernel
From: Julia Lawall <julia.lawall@lip6.fr>
Date: Mon, 12 Sep 2016 11:07:15 +0200 (CEST)
> So foo is protected, but its lifetime of interest is quite short. But we
> can't set the ops field of the type of xxx to be const either, because it
> is obviously not - the code above modifies it.
You can definitely make it a pointer to a const thing.
Just like "netdev_ops" in struct net_device is const.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 13:52 ` Julia Lawall
@ 2016-09-12 18:50 ` Jarkko Sakkinen
0 siblings, 0 replies; 35+ messages in thread
From: Jarkko Sakkinen @ 2016-09-12 18:50 UTC (permalink / raw)
To: Julia Lawall
Cc: Felipe Balbi, linux-renesas-soc, joe, kernel-janitors,
Sergei Shtylyov, linux-pm, platform-driver-x86, linux-media,
linux-can, Tatyana Nikolova, Shiraz Saleem, Mustafa Ismail,
Chien Tin Tung, linux-rdma, netdev, devel, alsa-devel,
linux-kernel, linux-fbdev, linux-wireless, Jason Gunthorpe,
tpmdd-devel, linux-scsi@
On Mon, Sep 12, 2016 at 03:52:08PM +0200, Julia Lawall wrote:
>
>
> On Mon, 12 Sep 2016, Felipe Balbi wrote:
>
> >
> > Hi,
> >
> > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> > >>
> > >>
> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> > >>
> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > >> > > Constify local structures.
> > >> > >
> > >> > > The semantic patch that makes this change is as follows:
> > >> > > (http://coccinelle.lip6.fr/)
> > >> >
> > >> > Just my two cents but:
> > >> >
> > >> > 1. You *can* use a static analysis too to find bugs or other issues.
> > >> > 2. However, you should manually do the commits and proper commit
> > >> > messages to subsystems based on your findings. And I generally think
> > >> > that if one contributes code one should also at least smoke test changes
> > >> > somehow.
> > >> >
> > >> > I don't know if I'm alone with my opinion. I just think that one should
> > >> > also do the analysis part and not blindly create and submit patches.
> > >>
> > >> All of the patches are compile tested. And the individual patches are
> > >
> > > Compile-testing is not testing. If you are not able to test a commit,
> > > you should explain why.
> >
> > Dude, Julia has been doing semantic patching for years already and
> > nobody has raised any concerns so far. There's already an expectation
> > that Coccinelle *works* and Julia's sematic patches are sound.
> >
> > Besides, adding 'const' is something that causes virtually no functional
> > changes to the point that build-testing is really all you need. Any
> > problems caused by adding 'const' to a definition will be seen by build
> > errors or warnings.
> >
> > Really, just stop with the pointless discussion and go read a bit about
> > Coccinelle and what semantic patches are giving you. The work done by
> > Julia and her peers are INRIA have measurable benefits.
> >
> > You're really making a thunderstorm in a glass of water.
>
> Thanks for the defense, but since a lot of these patches torned out to be
> wrong, due to an incorrect parse by Coccinelle, combined with an
> unpleasantly lax compiler, Jarkko does have a point that I should have
> looked at the patches more carefully. In any case, I have written to the
> maintainers relevant to the patches that turned out to be incorrect.
Exactly. I'm not excepting that every commit would require extensive
analysis but it would be good to quickly at least skim through commits
and see if they make sense (or ask if unsure) :)
And I'm fine with compile testing if it is mentioned in the commit msg.
> julia
/Jarkko
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 13:43 ` Felipe Balbi
2016-09-12 13:52 ` Julia Lawall
2016-09-12 13:57 ` Geert Uytterhoeven
@ 2016-09-12 20:14 ` Jarkko Sakkinen
2016-09-12 21:11 ` Julia Lawall
2 siblings, 1 reply; 35+ messages in thread
From: Jarkko Sakkinen @ 2016-09-12 20:14 UTC (permalink / raw)
To: Felipe Balbi
Cc: alsa-devel, Mustafa Ismail, Tatyana Nikolova, kernel-janitors,
linux-fbdev, platform-driver-x86, devel, linux-scsi, linux-rdma,
Jason Gunthorpe, linux-acpi, tpmdd-devel, linux-media, linux-pm,
linux-can, Julia Lawall, Shiraz Saleem, Sergei Shtylyov, netdev,
Chien Tin Tung, linux-wireless, linux-kernel, linux-spi,
linux-renesas-soc, linux-usb, joe
On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
>
> Hi,
>
> Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> >>
> >>
> >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> >>
> >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> >> > > Constify local structures.
> >> > >
> >> > > The semantic patch that makes this change is as follows:
> >> > > (http://coccinelle.lip6.fr/)
> >> >
> >> > Just my two cents but:
> >> >
> >> > 1. You *can* use a static analysis too to find bugs or other issues.
> >> > 2. However, you should manually do the commits and proper commit
> >> > messages to subsystems based on your findings. And I generally think
> >> > that if one contributes code one should also at least smoke test changes
> >> > somehow.
> >> >
> >> > I don't know if I'm alone with my opinion. I just think that one should
> >> > also do the analysis part and not blindly create and submit patches.
> >>
> >> All of the patches are compile tested. And the individual patches are
> >
> > Compile-testing is not testing. If you are not able to test a commit,
> > you should explain why.
>
> Dude, Julia has been doing semantic patching for years already and
> nobody has raised any concerns so far. There's already an expectation
> that Coccinelle *works* and Julia's sematic patches are sound.
>
> Besides, adding 'const' is something that causes virtually no functional
> changes to the point that build-testing is really all you need. Any
> problems caused by adding 'const' to a definition will be seen by build
> errors or warnings.
>
> Really, just stop with the pointless discussion and go read a bit about
> Coccinelle and what semantic patches are giving you. The work done by
> Julia and her peers are INRIA have measurable benefits.
>
> You're really making a thunderstorm in a glass of water.
Hmm... I've been using coccinelle in cyclic basis for some time now.
My comment was oversized but I didn't mean it to be impolite or attack
of any kind for that matter.
> --
> balbi
/Jarkko
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [PATCH 00/26] constify local structures
2016-09-12 20:14 ` Jarkko Sakkinen
@ 2016-09-12 21:11 ` Julia Lawall
0 siblings, 0 replies; 35+ messages in thread
From: Julia Lawall @ 2016-09-12 21:11 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: Felipe Balbi, Julia Lawall, linux-renesas-soc, joe,
kernel-janitors, Sergei Shtylyov, linux-pm, platform-driver-x86,
linux-media, linux-can, Tatyana Nikolova, Shiraz Saleem,
Mustafa Ismail, Chien Tin Tung, linux-rdma, netdev, devel,
alsa-devel, linux-kernel, linux-fbdev, linux-wireless,
Jason Gunthorpe, t
On Mon, 12 Sep 2016, Jarkko Sakkinen wrote:
> On Mon, Sep 12, 2016 at 04:43:58PM +0300, Felipe Balbi wrote:
> >
> > Hi,
> >
> > Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com> writes:
> > > On Mon, Sep 12, 2016 at 10:54:07AM +0200, Julia Lawall wrote:
> > >>
> > >>
> > >> On Sun, 11 Sep 2016, Jarkko Sakkinen wrote:
> > >>
> > >> > On Sun, Sep 11, 2016 at 03:05:42PM +0200, Julia Lawall wrote:
> > >> > > Constify local structures.
> > >> > >
> > >> > > The semantic patch that makes this change is as follows:
> > >> > > (http://coccinelle.lip6.fr/)
> > >> >
> > >> > Just my two cents but:
> > >> >
> > >> > 1. You *can* use a static analysis too to find bugs or other issues.
> > >> > 2. However, you should manually do the commits and proper commit
> > >> > messages to subsystems based on your findings. And I generally think
> > >> > that if one contributes code one should also at least smoke test changes
> > >> > somehow.
> > >> >
> > >> > I don't know if I'm alone with my opinion. I just think that one should
> > >> > also do the analysis part and not blindly create and submit patches.
> > >>
> > >> All of the patches are compile tested. And the individual patches are
> > >
> > > Compile-testing is not testing. If you are not able to test a commit,
> > > you should explain why.
> >
> > Dude, Julia has been doing semantic patching for years already and
> > nobody has raised any concerns so far. There's already an expectation
> > that Coccinelle *works* and Julia's sematic patches are sound.
> >
> > Besides, adding 'const' is something that causes virtually no functional
> > changes to the point that build-testing is really all you need. Any
> > problems caused by adding 'const' to a definition will be seen by build
> > errors or warnings.
> >
> > Really, just stop with the pointless discussion and go read a bit about
> > Coccinelle and what semantic patches are giving you. The work done by
> > Julia and her peers are INRIA have measurable benefits.
> >
> > You're really making a thunderstorm in a glass of water.
>
> Hmm... I've been using coccinelle in cyclic basis for some time now.
> My comment was oversized but I didn't mean it to be impolite or attack
> of any kind for that matter.
No problem :) Thanks for the feedback.
julia
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [06/26] ath: constify local structures
2016-09-11 13:05 ` [PATCH 06/26] ath: " Julia Lawall
@ 2016-09-14 17:02 ` Kalle Valo
0 siblings, 0 replies; 35+ messages in thread
From: Kalle Valo @ 2016-09-14 17:02 UTC (permalink / raw)
To: Julia Lawall
Cc: Luis R. Rodriguez, joe, kernel-janitors, linux-wireless, netdev,
linux-kernel
Julia Lawall <Julia.Lawall@lip6.fr> wrote:
> For structure types defined in the same file or local header files, find
> top-level static structure declarations that have the following
> properties:
> 1. Never reassigned.
> 2. Address never taken
> 3. Not passed to a top-level macro call
> 4. No pointer or array-typed field passed to a function or stored in a
> variable.
> Declare structures having all of these properties as const.
>
> Done using Coccinelle.
> Based on a suggestion by Joe Perches <joe@perches.com>.
>
> Signed-off-by: Julia Lawall <Julia.Lawall@lip6.fr>
Thanks, 3 patches applied to wireless-drivers-next.git:
8136fd58ad60 ath: constify local structures
1dc80798a8ca iwlegacy: constify local structures
d86e64768859 rtlwifi: rtl818x: constify local structures
--
Sent by pwcli
https://patchwork.kernel.org/patch/9325363/
^ permalink raw reply [flat|nested] 35+ messages in thread
end of thread, other threads:[~2016-09-14 17:02 UTC | newest]
Thread overview: 35+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-09-11 13:05 [PATCH 00/26] constify local structures Julia Lawall
2016-09-11 13:05 ` [PATCH 05/26] ARCNET: " Julia Lawall
2016-09-12 12:31 ` Julia Lawall
2016-09-11 13:05 ` [PATCH 06/26] ath: " Julia Lawall
2016-09-14 17:02 ` [06/26] " Kalle Valo
2016-09-11 13:05 ` [PATCH 07/26] net/mlx4_core: " Julia Lawall
2016-09-12 4:59 ` Leon Romanovsky
2016-09-11 13:05 ` [PATCH 08/26] iwlegacy: " Julia Lawall
2016-09-12 11:24 ` Stanislaw Gruszka
2016-09-11 13:05 ` [PATCH 11/26] can: " Julia Lawall
2016-09-12 12:33 ` Julia Lawall
2016-09-11 13:06 ` [PATCH 20/26] stmmac: pci: " Julia Lawall
2016-09-12 12:08 ` Julia Lawall
2016-09-11 13:06 ` [PATCH 23/26] sh_eth: " Julia Lawall
2016-09-11 18:14 ` Sergei Shtylyov
2016-09-12 8:55 ` Julia Lawall
2016-09-12 9:43 ` Julia Lawall
2016-09-11 13:06 ` [PATCH 25/26] pch_gbe: " Julia Lawall
2016-09-12 2:48 ` David Miller
2016-09-12 9:07 ` Julia Lawall
2016-09-12 16:33 ` David Miller
2016-09-12 12:25 ` Julia Lawall
[not found] ` <1473599168-30561-1-git-send-email-Julia.Lawall-L2FTfq7BK8M@public.gmane.org>
2016-09-11 13:06 ` [PATCH 21/26] rtlwifi: rtl818x: " Julia Lawall
2016-09-11 17:21 ` [PATCH 00/26] " Jarkko Sakkinen
2016-09-12 8:54 ` Julia Lawall
2016-09-12 13:16 ` Jarkko Sakkinen
2016-09-12 13:23 ` Julia Lawall
2016-09-12 13:43 ` Felipe Balbi
2016-09-12 13:52 ` Julia Lawall
2016-09-12 18:50 ` Jarkko Sakkinen
2016-09-12 13:57 ` Geert Uytterhoeven
2016-09-12 20:14 ` Jarkko Sakkinen
2016-09-12 21:11 ` Julia Lawall
2016-09-11 17:56 ` Joe Perches
2016-09-11 19:11 ` Julia Lawall
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).