driver core: constify groups arrays in several structs

[PATCH RFC 02/10] rtc: prepare for struct device member groups becoming a constant array

Posted by Heiner Kallweit 1 month, 2 weeks ago

This prepares for making struct device member groups a constant array.
The assignment groups = rtc->dev.groups would result in a "discarding
const qualifier" warning with this change.
No functional change intended.

Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
 drivers/rtc/sysfs.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/rtc/sysfs.c b/drivers/rtc/sysfs.c
index 4ab05e105a7..ae5e1252b4c 100644
--- a/drivers/rtc/sysfs.c
+++ b/drivers/rtc/sysfs.c
@@ -308,7 +308,7 @@ const struct attribute_group **rtc_get_dev_attribute_groups(void)
 int rtc_add_groups(struct rtc_device *rtc, const struct attribute_group **grps)
 {
 	size_t old_cnt = 0, add_cnt = 0, new_cnt;
-	const struct attribute_group **groups, **old;
+	const struct attribute_group **groups, *const *old;
 
 	if (grps) {
 		for (groups = grps; *groups; groups++)
@@ -320,9 +320,9 @@ int rtc_add_groups(struct rtc_device *rtc, const struct attribute_group **grps)
 		return -EINVAL;
 	}
 
-	groups = rtc->dev.groups;
-	if (groups)
-		for (; *groups; groups++)
+	old = rtc->dev.groups;
+	if (old)
+		while (*old++)
 			old_cnt++;
 
 	new_cnt = old_cnt + add_cnt + 1;
-- 
2.53.0

Re: [PATCH RFC 02/10] rtc: prepare for struct device member groups becoming a constant array

Posted by yanjun.zhu 1 month, 1 week ago

On 2/17/26 2:26 PM, Heiner Kallweit wrote:
> This prepares for making struct device member groups a constant array.
> The assignment groups = rtc->dev.groups would result in a "discarding
> const qualifier" warning with this change.
> No functional change intended.
> 
> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> ---
>   drivers/rtc/sysfs.c | 8 ++++----
>   1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/rtc/sysfs.c b/drivers/rtc/sysfs.c
> index 4ab05e105a7..ae5e1252b4c 100644
> --- a/drivers/rtc/sysfs.c
> +++ b/drivers/rtc/sysfs.c
> @@ -308,7 +308,7 @@ const struct attribute_group **rtc_get_dev_attribute_groups(void)
>   int rtc_add_groups(struct rtc_device *rtc, const struct attribute_group **grps)
>   {
>   	size_t old_cnt = 0, add_cnt = 0, new_cnt;
> -	const struct attribute_group **groups, **old;
> +	const struct attribute_group **groups, *const *old;
>   
>   	if (grps) {
>   		for (groups = grps; *groups; groups++)
> @@ -320,9 +320,9 @@ int rtc_add_groups(struct rtc_device *rtc, const struct attribute_group **grps)
>   		return -EINVAL;
>   	}
>   
> -	groups = rtc->dev.groups;
> -	if (groups)
> -		for (; *groups; groups++)
> +	old = rtc->dev.groups;
> +	if (old)
> +		while (*old++)
>   			old_cnt++;

The change from for (; *groups; groups++) to while (*old++) is not 
functionally equivalent. In the while version, the post-increment old++ 
executes even when *old is NULL. This leaves the pointer old pointing 
one element past the NULL terminator. While old_cnt remains correct, 
this is a side-effect-heavy idiom that differs from standard kernel 
patterns and could be fragile if old is used later in the function.

Best Regards,
Zhu Yanjun

>   
>   	new_cnt = old_cnt + add_cnt + 1;

Re: [PATCH RFC 02/10] rtc: prepare for struct device member groups becoming a constant array

Posted by Alexandre Belloni 1 month, 1 week ago

On 18/02/2026 16:53:00-0800, yanjun.zhu wrote:
> On 2/17/26 2:26 PM, Heiner Kallweit wrote:
> > This prepares for making struct device member groups a constant array.
> > The assignment groups = rtc->dev.groups would result in a "discarding
> > const qualifier" warning with this change.
> > No functional change intended.
> > 
> > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
> > ---
> >   drivers/rtc/sysfs.c | 8 ++++----
> >   1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/drivers/rtc/sysfs.c b/drivers/rtc/sysfs.c
> > index 4ab05e105a7..ae5e1252b4c 100644
> > --- a/drivers/rtc/sysfs.c
> > +++ b/drivers/rtc/sysfs.c
> > @@ -308,7 +308,7 @@ const struct attribute_group **rtc_get_dev_attribute_groups(void)
> >   int rtc_add_groups(struct rtc_device *rtc, const struct attribute_group **grps)
> >   {
> >   	size_t old_cnt = 0, add_cnt = 0, new_cnt;
> > -	const struct attribute_group **groups, **old;
> > +	const struct attribute_group **groups, *const *old;
> >   	if (grps) {
> >   		for (groups = grps; *groups; groups++)
> > @@ -320,9 +320,9 @@ int rtc_add_groups(struct rtc_device *rtc, const struct attribute_group **grps)
> >   		return -EINVAL;
> >   	}
> > -	groups = rtc->dev.groups;
> > -	if (groups)
> > -		for (; *groups; groups++)
> > +	old = rtc->dev.groups;
> > +	if (old)
> > +		while (*old++)
> >   			old_cnt++;
> 
> The change from for (; *groups; groups++) to while (*old++) is not
> functionally equivalent. In the while version, the post-increment old++
> executes even when *old is NULL. This leaves the pointer old pointing one
> element past the NULL terminator. While old_cnt remains correct, this is a
> side-effect-heavy idiom that differs from standard kernel patterns and could
> be fragile if old is used later in the function.
> 

Thanks for pointing this out, I agree we should keep the original for
loop.

With that change,
Acked-by: Alexandre Belloni <alexandre.belloni@bootlin.com>


> Best Regards,
> Zhu Yanjun
> 
> >   	new_cnt = old_cnt + add_cnt + 1;
> 

-- 
Alexandre Belloni, co-owner and COO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com