Re: [PATCH 1/2] sysfs: attribute_group: allow registration of const bin_attribute

[Greg Kroah-Hartman <gregkh@linuxfoundation.org>]

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