To be able to constify instances of struct bin_attribute it has to be
possible to add them to string attribute_group.
The current type of the bin_attrs member however is not compatible with
that.
Introduce a union that allows registration of both const and non-const
attributes to enable a piecewise transition.
As both union member types are compatible no logic needs to be adapted.
Technically it is now possible register a const struct
bin_attribute and receive it as mutable pointer in the callbacks.
This is a soundness issue.
But this same soundness issue already exists today in
sysfs_create_bin_file().
Also the struct definition and callback implementation are always
closely linked and are meant to be moved to const in lockstep.
Signed-off-by: Thomas Weißschuh <linux@weissschuh.net>
---
include/linux/sysfs.h | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
index d713a6445a6267145a7014f308df3bb25b8c3287..0f2fcd244523f050c5286f19d4fe1846506f9214 100644
--- a/include/linux/sysfs.h
+++ b/include/linux/sysfs.h
@@ -106,7 +106,10 @@ struct attribute_group {
const struct bin_attribute *,
int);
struct attribute **attrs;
- struct bin_attribute **bin_attrs;
+ union {
+ struct bin_attribute **bin_attrs;
+ const struct bin_attribute *const *bin_attrs_new;
+ };
};
#define SYSFS_PREALLOC 010000
--
2.47.0
Hi Greg,
On 2024-11-15 17:42:48+0100, Thomas Weißschuh wrote:
> [..]
> diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> index d713a6445a6267145a7014f308df3bb25b8c3287..0f2fcd244523f050c5286f19d4fe1846506f9214 100644
> --- a/include/linux/sysfs.h
> +++ b/include/linux/sysfs.h
> @@ -106,7 +106,10 @@ struct attribute_group {
> const struct bin_attribute *,
> int);
> struct attribute **attrs;
> - struct bin_attribute **bin_attrs;
> + union {
> + struct bin_attribute **bin_attrs;
> + const struct bin_attribute *const *bin_attrs_new;
> + };
Unfortunately this triggers warnings in two drivers.
These incorrectly have a trailing NULL literal in their struct attribute
definition (full list at the end of the mail):
>> drivers/perf/arm-ni.c:248:63: warning: missing braces around initializer [-Wmissing-braces]
248 | static const struct attribute_group arm_ni_other_attr_group = {
| ^
vim +248 drivers/perf/arm-ni.c
4d5a7680f2b4d0 Robin Murphy 2024-09-04 247
4d5a7680f2b4d0 Robin Murphy 2024-09-04 @248 static const struct attribute_group arm_ni_other_attr_group = {
4d5a7680f2b4d0 Robin Murphy 2024-09-04 249 .attrs = arm_ni_other_attrs,
4d5a7680f2b4d0 Robin Murphy 2024-09-04 250 NULL
4d5a7680f2b4d0 Robin Murphy 2024-09-04 251 };
4d5a7680f2b4d0 Robin Murphy 2024-09-04 252
These trailing NULLs should first be removed.
How do you want to proceed?
Cocci script and results, only the first two results are relevant at
this moment.
virtual patch
@@
identifier ag, pattrs;
@@
struct attribute_group ag = {
.attrs = pattrs,
- NULL
};
diff -u -p a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
--- a/drivers/s390/char/con3215.c
+++ b/drivers/s390/char/con3215.c
@@ -803,7 +803,6 @@ static struct attribute *con3215_drv_att
static struct attribute_group con3215_drv_attr_group = {
.attrs = con3215_drv_attrs,
- NULL,
};
static const struct attribute_group *con3215_drv_attr_groups[] = {
diff -u -p a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
--- a/drivers/perf/arm-ni.c
+++ b/drivers/perf/arm-ni.c
@@ -247,7 +247,6 @@ static struct attribute *arm_ni_other_at
static const struct attribute_group arm_ni_other_attr_group = {
.attrs = arm_ni_other_attrs,
- NULL
};
static const struct attribute_group *arm_ni_attr_groups[] = {
diff -u -p a/kernel/cpu.c b/kernel/cpu.c
--- a/kernel/cpu.c
+++ b/kernel/cpu.c
@@ -2866,7 +2866,6 @@ static struct attribute *cpuhp_cpu_attrs
static const struct attribute_group cpuhp_cpu_attr_group = {
.attrs = cpuhp_cpu_attrs,
.name = "hotplug",
- NULL
};
static ssize_t states_show(struct device *dev,
@@ -2898,7 +2897,6 @@ static struct attribute *cpuhp_cpu_root_
static const struct attribute_group cpuhp_cpu_root_attr_group = {
.attrs = cpuhp_cpu_root_attrs,
.name = "hotplug",
- NULL
};
#ifdef CONFIG_HOTPLUG_SMT
@@ -3020,7 +3018,6 @@ static struct attribute *cpuhp_smt_attrs
static const struct attribute_group cpuhp_smt_attr_group = {
.attrs = cpuhp_smt_attrs,
.name = "smt",
- NULL
};
static int __init cpu_smt_sysfs_init(void)
On Mon, Nov 18, 2024 at 12:36:06PM +0100, Thomas Weißschuh wrote:
> Hi Greg,
>
> On 2024-11-15 17:42:48+0100, Thomas Weißschuh wrote:
>
> > [..]
>
> > diff --git a/include/linux/sysfs.h b/include/linux/sysfs.h
> > index d713a6445a6267145a7014f308df3bb25b8c3287..0f2fcd244523f050c5286f19d4fe1846506f9214 100644
> > --- a/include/linux/sysfs.h
> > +++ b/include/linux/sysfs.h
> > @@ -106,7 +106,10 @@ struct attribute_group {
> > const struct bin_attribute *,
> > int);
> > struct attribute **attrs;
> > - struct bin_attribute **bin_attrs;
> > + union {
> > + struct bin_attribute **bin_attrs;
> > + const struct bin_attribute *const *bin_attrs_new;
> > + };
>
> Unfortunately this triggers warnings in two drivers.
> These incorrectly have a trailing NULL literal in their struct attribute
> definition (full list at the end of the mail):
>
> >> drivers/perf/arm-ni.c:248:63: warning: missing braces around initializer [-Wmissing-braces]
> 248 | static const struct attribute_group arm_ni_other_attr_group = {
> | ^
>
>
> vim +248 drivers/perf/arm-ni.c
>
> 4d5a7680f2b4d0 Robin Murphy 2024-09-04 247
> 4d5a7680f2b4d0 Robin Murphy 2024-09-04 @248 static const struct attribute_group arm_ni_other_attr_group = {
> 4d5a7680f2b4d0 Robin Murphy 2024-09-04 249 .attrs = arm_ni_other_attrs,
> 4d5a7680f2b4d0 Robin Murphy 2024-09-04 250 NULL
> 4d5a7680f2b4d0 Robin Murphy 2024-09-04 251 };
> 4d5a7680f2b4d0 Robin Murphy 2024-09-04 252
>
> These trailing NULLs should first be removed.
> How do you want to proceed?
Odd, it passed 0-day testing.
Just send me a patch to fix up these obvious problems, strange it built
in the first place (it's a mix of named and not named identifiers, I
thought the compiler would complain about that...)
> Cocci script and results, only the first two results are relevant at
> this moment.
>
> virtual patch
>
> @@
> identifier ag, pattrs;
> @@
>
> struct attribute_group ag = {
> .attrs = pattrs,
> - NULL
> };
>
> diff -u -p a/drivers/s390/char/con3215.c b/drivers/s390/char/con3215.c
> --- a/drivers/s390/char/con3215.c
> +++ b/drivers/s390/char/con3215.c
> @@ -803,7 +803,6 @@ static struct attribute *con3215_drv_att
>
> static struct attribute_group con3215_drv_attr_group = {
> .attrs = con3215_drv_attrs,
> - NULL,
> };
>
> static const struct attribute_group *con3215_drv_attr_groups[] = {
> diff -u -p a/drivers/perf/arm-ni.c b/drivers/perf/arm-ni.c
> --- a/drivers/perf/arm-ni.c
> +++ b/drivers/perf/arm-ni.c
> @@ -247,7 +247,6 @@ static struct attribute *arm_ni_other_at
>
> static const struct attribute_group arm_ni_other_attr_group = {
> .attrs = arm_ni_other_attrs,
> - NULL
> };
>
> static const struct attribute_group *arm_ni_attr_groups[] = {
> diff -u -p a/kernel/cpu.c b/kernel/cpu.c
> --- a/kernel/cpu.c
> +++ b/kernel/cpu.c
> @@ -2866,7 +2866,6 @@ static struct attribute *cpuhp_cpu_attrs
> static const struct attribute_group cpuhp_cpu_attr_group = {
> .attrs = cpuhp_cpu_attrs,
> .name = "hotplug",
> - NULL
> };
>
> static ssize_t states_show(struct device *dev,
> @@ -2898,7 +2897,6 @@ static struct attribute *cpuhp_cpu_root_
> static const struct attribute_group cpuhp_cpu_root_attr_group = {
> .attrs = cpuhp_cpu_root_attrs,
> .name = "hotplug",
> - NULL
> };
>
> #ifdef CONFIG_HOTPLUG_SMT
> @@ -3020,7 +3018,6 @@ static struct attribute *cpuhp_smt_attrs
> static const struct attribute_group cpuhp_smt_attr_group = {
> .attrs = cpuhp_smt_attrs,
> .name = "smt",
> - NULL
> };
>
> static int __init cpu_smt_sysfs_init(void)
Looks sane, send me a patch?
thanks,
greg k-h
© 2016 - 2024 Red Hat, Inc.