LinuxLists.cc - [PATCH 0/3] w1: Constify w1_family

2020-10-04 19:33:34

Subject: [PATCH 0/3] w1: Constify w1_family_ops

None of the current instances of struct w1_family_ops in the kernel is
modified. Constify these to let the compiler put them in read-only memory.

The first patch changes the fops field in w1_family struct to a pointer to
const and makes a local variable a pointer to const to avoid a compiler
warning. This patch is a prerequisite for the second and third patches
which constifies the static structs in drivers in w1 and power. These
changes was done with coccinelle (details in the commit messages).

With these changes applied, all instances of struct w1_family_ops in the
kernel are const.

Build-tested on x86 allmodconfig.

Rikard Falkeborn (3):
w1: Constify struct w1_family_ops
w1: Constify static w1_family_ops structs
power: supply: Constify static w1_family_ops structs

drivers/power/supply/bq27xxx_battery_hdq.c | 2 +-
drivers/power/supply/ds2760_battery.c | 2 +-
drivers/power/supply/max1721x_battery.c | 2 +-
drivers/w1/slaves/w1_ds2405.c | 2 +-
drivers/w1/slaves/w1_ds2406.c | 2 +-
drivers/w1/slaves/w1_ds2408.c | 2 +-
drivers/w1/slaves/w1_ds2413.c | 2 +-
drivers/w1/slaves/w1_ds2423.c | 2 +-
drivers/w1/slaves/w1_ds2430.c | 2 +-
drivers/w1/slaves/w1_ds2431.c | 2 +-
drivers/w1/slaves/w1_ds2433.c | 2 +-
drivers/w1/slaves/w1_ds2438.c | 2 +-
drivers/w1/slaves/w1_ds250x.c | 2 +-
drivers/w1/slaves/w1_ds2780.c | 2 +-
drivers/w1/slaves/w1_ds2781.c | 2 +-
drivers/w1/slaves/w1_ds2805.c | 2 +-
drivers/w1/slaves/w1_ds28e04.c | 2 +-
drivers/w1/slaves/w1_ds28e17.c | 2 +-
drivers/w1/slaves/w1_therm.c | 6 +++---
drivers/w1/w1.c | 4 ++--
include/linux/w1.h | 2 +-
21 files changed, 24 insertions(+), 24 deletions(-)

--
2.28.0

2020-10-04 19:36:44

by Rikard Falkeborn

[permalink] [raw]

Subject: [PATCH 3/3] power: supply: Constify static w1_family_ops structs

2020-10-04 19:36:45

by Rikard Falkeborn

[permalink] [raw]

Subject: [PATCH 2/3] w1: Constify static w1_family_ops structs

The only usage of these structs is to assign their address to the fops
field in the w1_family struct, which is a const pointer. Make them const
to allow the compiler to put them in read-only memory.

This was done with the following Coccinelle semantic patch
(http://coccinelle.lip6.fr/):

// <smpl>
@r1 disable optional_qualifier @
identifier i;
position p;
@@
static struct w1_family_ops i@p = {...};

@ok1@
identifier r1.i;
position p;
identifier s;
@@
static struct w1_family s = {
.fops=&i@p,
};

@bad1@
position p!={r1.p,ok1.p};
identifier r1.i;
@@
i@p

@depends on !bad1 disable optional_qualifier@
identifier r1.i;
@@
static
+const
struct w1_family_ops i={};
// </smpl>

Signed-off-by: Rikard Falkeborn <[email protected]>
---
drivers/w1/slaves/w1_ds2405.c | 2 +-
drivers/w1/slaves/w1_ds2406.c | 2 +-
drivers/w1/slaves/w1_ds2408.c | 2 +-
drivers/w1/slaves/w1_ds2413.c | 2 +-
drivers/w1/slaves/w1_ds2423.c | 2 +-
drivers/w1/slaves/w1_ds2430.c | 2 +-
drivers/w1/slaves/w1_ds2431.c | 2 +-
drivers/w1/slaves/w1_ds2433.c | 2 +-
drivers/w1/slaves/w1_ds2438.c | 2 +-
drivers/w1/slaves/w1_ds250x.c | 2 +-
drivers/w1/slaves/w1_ds2780.c | 2 +-
drivers/w1/slaves/w1_ds2781.c | 2 +-
drivers/w1/slaves/w1_ds2805.c | 2 +-
drivers/w1/slaves/w1_ds28e04.c | 2 +-
drivers/w1/slaves/w1_ds28e17.c | 2 +-
drivers/w1/slaves/w1_therm.c | 6 +++---
drivers/w1/w1.c | 2 +-
17 files changed, 19 insertions(+), 19 deletions(-)

diff --git a/drivers/w1/slaves/w1_ds2405.c b/drivers/w1/slaves/w1_ds2405.c
index 86cd97309d87..1d9a1183e83f 100644
--- a/drivers/w1/slaves/w1_ds2405.c
+++ b/drivers/w1/slaves/w1_ds2405.c
@@ -206,7 +206,7 @@ static struct attribute *w1_ds2405_attrs[] = {

ATTRIBUTE_GROUPS(w1_ds2405);

-static struct w1_family_ops w1_ds2405_fops = {
+static const struct w1_family_ops w1_ds2405_fops = {
.groups = w1_ds2405_groups
};

diff --git a/drivers/w1/slaves/w1_ds2406.c b/drivers/w1/slaves/w1_ds2406.c
index 762e5e4e2b48..6c269af73c80 100644
--- a/drivers/w1/slaves/w1_ds2406.c
+++ b/drivers/w1/slaves/w1_ds2406.c
@@ -138,7 +138,7 @@ static void w1_f12_remove_slave(struct w1_slave *sl)
&(w1_f12_sysfs_bin_files[i]));
}

-static struct w1_family_ops w1_f12_fops = {
+static const struct w1_family_ops w1_f12_fops = {
.add_slave = w1_f12_add_slave,
.remove_slave = w1_f12_remove_slave,
};
diff --git a/drivers/w1/slaves/w1_ds2408.c b/drivers/w1/slaves/w1_ds2408.c
index 83f8d94bb814..ad102c577122 100644
--- a/drivers/w1/slaves/w1_ds2408.c
+++ b/drivers/w1/slaves/w1_ds2408.c
@@ -336,7 +336,7 @@ static const struct attribute_group *w1_f29_groups[] = {
NULL,
};

-static struct w1_family_ops w1_f29_fops = {
+static const struct w1_family_ops w1_f29_fops = {
.add_slave = w1_f29_disable_test_mode,
.groups = w1_f29_groups,
};
diff --git a/drivers/w1/slaves/w1_ds2413.c b/drivers/w1/slaves/w1_ds2413.c
index f1fb18afbcea..c8cfac555b48 100644
--- a/drivers/w1/slaves/w1_ds2413.c
+++ b/drivers/w1/slaves/w1_ds2413.c
@@ -143,7 +143,7 @@ static const struct attribute_group *w1_f3a_groups[] = {
NULL,
};

-static struct w1_family_ops w1_f3a_fops = {
+static const struct w1_family_ops w1_f3a_fops = {
.groups = w1_f3a_groups,
};

diff --git a/drivers/w1/slaves/w1_ds2423.c b/drivers/w1/slaves/w1_ds2423.c
index f4367282dcc1..b6bd18d5b3f6 100644
--- a/drivers/w1/slaves/w1_ds2423.c
+++ b/drivers/w1/slaves/w1_ds2423.c
@@ -117,7 +117,7 @@ static struct attribute *w1_f1d_attrs[] = {
};
ATTRIBUTE_GROUPS(w1_f1d);

-static struct w1_family_ops w1_f1d_fops = {
+static const struct w1_family_ops w1_f1d_fops = {
.groups = w1_f1d_groups,
};

diff --git a/drivers/w1/slaves/w1_ds2430.c b/drivers/w1/slaves/w1_ds2430.c
index 75bb8a88620b..0ea7d779d17a 100644
--- a/drivers/w1/slaves/w1_ds2430.c
+++ b/drivers/w1/slaves/w1_ds2430.c
@@ -279,7 +279,7 @@ static const struct attribute_group *w1_f14_groups[] = {
NULL,
};

-static struct w1_family_ops w1_f14_fops = {
+static const struct w1_family_ops w1_f14_fops = {
.groups = w1_f14_groups,
};

diff --git a/drivers/w1/slaves/w1_ds2431.c b/drivers/w1/slaves/w1_ds2431.c
index e5bd7e2354d7..6856b1c29e17 100644
--- a/drivers/w1/slaves/w1_ds2431.c
+++ b/drivers/w1/slaves/w1_ds2431.c
@@ -278,7 +278,7 @@ static const struct attribute_group *w1_f2d_groups[] = {
NULL,
};

-static struct w1_family_ops w1_f2d_fops = {
+static const struct w1_family_ops w1_f2d_fops = {
.groups = w1_f2d_groups,
};

diff --git a/drivers/w1/slaves/w1_ds2433.c b/drivers/w1/slaves/w1_ds2433.c
index 1f805c86517a..0f72df15a024 100644
--- a/drivers/w1/slaves/w1_ds2433.c
+++ b/drivers/w1/slaves/w1_ds2433.c
@@ -288,7 +288,7 @@ static void w1_f23_remove_slave(struct w1_slave *sl)
#endif /* CONFIG_W1_SLAVE_DS2433_CRC */
}

-static struct w1_family_ops w1_f23_fops = {
+static const struct w1_family_ops w1_f23_fops = {
.add_slave = w1_f23_add_slave,
.remove_slave = w1_f23_remove_slave,
.groups = w1_f23_groups,
diff --git a/drivers/w1/slaves/w1_ds2438.c b/drivers/w1/slaves/w1_ds2438.c
index d199e5a25cc0..5cfb0ae23e91 100644
--- a/drivers/w1/slaves/w1_ds2438.c
+++ b/drivers/w1/slaves/w1_ds2438.c
@@ -412,7 +412,7 @@ static const struct attribute_group *w1_ds2438_groups[] = {
NULL,
};

-static struct w1_family_ops w1_ds2438_fops = {
+static const struct w1_family_ops w1_ds2438_fops = {
.groups = w1_ds2438_groups,
};

diff --git a/drivers/w1/slaves/w1_ds250x.c b/drivers/w1/slaves/w1_ds250x.c
index e507117444d8..7592c7050d1d 100644
--- a/drivers/w1/slaves/w1_ds250x.c
+++ b/drivers/w1/slaves/w1_ds250x.c
@@ -215,7 +215,7 @@ static int w1_eprom_add_slave(struct w1_slave *sl)
return PTR_ERR_OR_ZERO(nvmem);
}

-static struct w1_family_ops w1_eprom_fops = {
+static const struct w1_family_ops w1_eprom_fops = {
.add_slave = w1_eprom_add_slave,
};

diff --git a/drivers/w1/slaves/w1_ds2780.c b/drivers/w1/slaves/w1_ds2780.c
index c689b1b987b8..c281fe5ed688 100644
--- a/drivers/w1/slaves/w1_ds2780.c
+++ b/drivers/w1/slaves/w1_ds2780.c
@@ -141,7 +141,7 @@ static void w1_ds2780_remove_slave(struct w1_slave *sl)
platform_device_unregister(pdev);
}

-static struct w1_family_ops w1_ds2780_fops = {
+static const struct w1_family_ops w1_ds2780_fops = {
.add_slave = w1_ds2780_add_slave,
.remove_slave = w1_ds2780_remove_slave,
.groups = w1_ds2780_groups,
diff --git a/drivers/w1/slaves/w1_ds2781.c b/drivers/w1/slaves/w1_ds2781.c
index 84d6ceec5da5..f0d393ae070b 100644
--- a/drivers/w1/slaves/w1_ds2781.c
+++ b/drivers/w1/slaves/w1_ds2781.c
@@ -138,7 +138,7 @@ static void w1_ds2781_remove_slave(struct w1_slave *sl)
platform_device_unregister(pdev);
}

-static struct w1_family_ops w1_ds2781_fops = {
+static const struct w1_family_ops w1_ds2781_fops = {
.add_slave = w1_ds2781_add_slave,
.remove_slave = w1_ds2781_remove_slave,
.groups = w1_ds2781_groups,
diff --git a/drivers/w1/slaves/w1_ds2805.c b/drivers/w1/slaves/w1_ds2805.c
index ccb753a474b1..206186db727d 100644
--- a/drivers/w1/slaves/w1_ds2805.c
+++ b/drivers/w1/slaves/w1_ds2805.c
@@ -281,7 +281,7 @@ static void w1_f0d_remove_slave(struct w1_slave *sl)
sysfs_remove_bin_file(&sl->dev.kobj, &w1_f0d_bin_attr);
}

-static struct w1_family_ops w1_f0d_fops = {
+static const struct w1_family_ops w1_f0d_fops = {
.add_slave = w1_f0d_add_slave,
.remove_slave = w1_f0d_remove_slave,
};
diff --git a/drivers/w1/slaves/w1_ds28e04.c b/drivers/w1/slaves/w1_ds28e04.c
index 8a640f159078..e4f336111edc 100644
--- a/drivers/w1/slaves/w1_ds28e04.c
+++ b/drivers/w1/slaves/w1_ds28e04.c
@@ -410,7 +410,7 @@ static void w1_f1C_remove_slave(struct w1_slave *sl)
sl->family_data = NULL;
}

-static struct w1_family_ops w1_f1C_fops = {
+static const struct w1_family_ops w1_f1C_fops = {
.add_slave = w1_f1C_add_slave,
.remove_slave = w1_f1C_remove_slave,
.groups = w1_f1C_groups,
diff --git a/drivers/w1/slaves/w1_ds28e17.c b/drivers/w1/slaves/w1_ds28e17.c
index 046ddda83df9..6b00db7169ab 100644
--- a/drivers/w1/slaves/w1_ds28e17.c
+++ b/drivers/w1/slaves/w1_ds28e17.c
@@ -741,7 +741,7 @@ static void w1_f19_remove_slave(struct w1_slave *sl)

/* Declarations within the w1 subsystem. */
-static struct w1_family_ops w1_f19_fops = {
+static const struct w1_family_ops w1_f19_fops = {
.add_slave = w1_f19_add_slave,
.remove_slave = w1_f19_remove_slave,
.groups = w1_f19_groups,
diff --git a/drivers/w1/slaves/w1_therm.c b/drivers/w1/slaves/w1_therm.c
index c1b4eda16719..54f84f2d5064 100644
--- a/drivers/w1/slaves/w1_therm.c
+++ b/drivers/w1/slaves/w1_therm.c
@@ -409,21 +409,21 @@ static const struct hwmon_chip_info w1_chip_info = {

/* Family operations */

-static struct w1_family_ops w1_therm_fops = {
+static const struct w1_family_ops w1_therm_fops = {
.add_slave = w1_therm_add_slave,
.remove_slave = w1_therm_remove_slave,
.groups = w1_therm_groups,
.chip_info = W1_CHIPINFO,
};

-static struct w1_family_ops w1_ds18s20_fops = {
+static const struct w1_family_ops w1_ds18s20_fops = {
.add_slave = w1_therm_add_slave,
.remove_slave = w1_therm_remove_slave,
.groups = w1_ds18s20_groups,
.chip_info = W1_CHIPINFO,
};

-static struct w1_family_ops w1_ds28ea00_fops = {
+static const struct w1_family_ops w1_ds28ea00_fops = {
.add_slave = w1_therm_add_slave,
.remove_slave = w1_therm_remove_slave,
.groups = w1_ds28ea00_groups,
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 6bd64bcb6316..15a2ee32f116 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -160,7 +160,7 @@ static const struct attribute_group *w1_slave_default_groups[] = {
NULL,
};

-static struct w1_family_ops w1_default_fops = {
+static const struct w1_family_ops w1_default_fops = {
.groups = w1_slave_default_groups,
};

--
2.28.0

2020-10-04 22:04:25

by Sebastian Reichel

[permalink] [raw]

Subject: Re: [PATCH 3/3] power: supply: Constify static w1_family_ops structs

Hi,

On Sun, Oct 04, 2020 at 09:32:02PM +0200, Rikard Falkeborn wrote:
> The only usage of these structs is to assign their address to the fops
> field in the w1_family struct, which is a const pointer. Make them const
> to allow the compiler to put them in read-only memory.
>
> This was done with the following Coccinelle semantic patch
> (http://coccinelle.lip6.fr/):
>
> // <smpl>
> @r1 disable optional_qualifier @
> identifier i;
> position p;
> @@
> static struct w1_family_ops i@p = {...};
>
> @ok1@
> identifier r1.i;
> position p;
> identifier s;
> @@
> static struct w1_family s = {
> .fops=&i@p,
> };
>
> @bad1@
> position p!={r1.p,ok1.p};
> identifier r1.i;
> @@
> i@p
>
> @depends on !bad1 disable optional_qualifier@
> identifier r1.i;
> @@
> static
> +const
> struct w1_family_ops i={};
> // </smpl>
>
> Signed-off-by: Rikard Falkeborn <[email protected]>
> ---

I suggest that this simply goes through the w1 tree together
with the other patches:

Acked-by: Sebastian Reichel <[email protected]>

-- Sebastian

> drivers/power/supply/bq27xxx_battery_hdq.c | 2 +-
> drivers/power/supply/ds2760_battery.c | 2 +-
> drivers/power/supply/max1721x_battery.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/power/supply/bq27xxx_battery_hdq.c b/drivers/power/supply/bq27xxx_battery_hdq.c
> index 12b10dad77d3..922759ab2e04 100644
> --- a/drivers/power/supply/bq27xxx_battery_hdq.c
> +++ b/drivers/power/supply/bq27xxx_battery_hdq.c
> @@ -97,7 +97,7 @@ static void bq27xxx_battery_hdq_remove_slave(struct w1_slave *sl)
> bq27xxx_battery_teardown(di);
> }
>
> -static struct w1_family_ops bq27xxx_battery_hdq_fops = {
> +static const struct w1_family_ops bq27xxx_battery_hdq_fops = {
> .add_slave = bq27xxx_battery_hdq_add_slave,
> .remove_slave = bq27xxx_battery_hdq_remove_slave,
> };
> diff --git a/drivers/power/supply/ds2760_battery.c b/drivers/power/supply/ds2760_battery.c
> index 11bed88a89fa..695bb6747400 100644
> --- a/drivers/power/supply/ds2760_battery.c
> +++ b/drivers/power/supply/ds2760_battery.c
> @@ -795,7 +795,7 @@ static const struct of_device_id w1_ds2760_of_ids[] = {
> };
> #endif
>
> -static struct w1_family_ops w1_ds2760_fops = {
> +static const struct w1_family_ops w1_ds2760_fops = {
> .add_slave = w1_ds2760_add_slave,
> .remove_slave = w1_ds2760_remove_slave,
> .groups = w1_ds2760_groups,
> diff --git a/drivers/power/supply/max1721x_battery.c b/drivers/power/supply/max1721x_battery.c
> index 9ca895b0dabb..1b1a36f8e929 100644
> --- a/drivers/power/supply/max1721x_battery.c
> +++ b/drivers/power/supply/max1721x_battery.c
> @@ -431,7 +431,7 @@ static int devm_w1_max1721x_add_device(struct w1_slave *sl)
> return 0;
> }
>
> -static struct w1_family_ops w1_max1721x_fops = {
> +static const struct w1_family_ops w1_max1721x_fops = {
> .add_slave = devm_w1_max1721x_add_device,
> };
>
> --
> 2.28.0
>

Attachments:

(No filename) (2.96 kB)
signature.asc (849.00 B)
Download all attachments