* [PATCH RFC] gpio: sim: access sysfs attributes as const @ 2026-10-16 8:13 Bartosz Golaszewski 2026-10-16 8:19 ` sashiko-bot 2026-10-16 9:07 ` Thomas Weißschuh 0 siblings, 2 replies; 4+ messages in thread From: Bartosz Golaszewski @ 2026-10-16 8:13 UTC (permalink / raw) To: Thomas Weißschuh, Linus Walleij, Bartosz Golaszewski Cc: linux-gpio, linux-kernel, Bartosz Golaszewski Use the const variant of constainer_of() to access struct gpio_sim_attribute which wraps the dynamically allocated sysfs attributes for value and pull of simulated GPIO lines in line with the currently preferred approach. Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> --- Hi Thomas! Following your talk, I'm sending a patch for the GPIO simulator module that attempts to constify the sysfs attributes. Please tell me if this is all I need here as the attributes are part of another struct that's allocated dynamically. I'm not sure if there's anything more to do here. --- drivers/gpio/gpio-sim.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c index 0c73bec042670fc5706b4a93700e652cbaf3605e..e0ae586cd08d8adbb3c16918607074fce116654e 100644 --- a/drivers/gpio/gpio-sim.c +++ b/drivers/gpio/gpio-sim.c @@ -59,10 +59,10 @@ struct gpio_sim_attribute { unsigned int offset; }; -static struct gpio_sim_attribute * +static const struct gpio_sim_attribute * to_gpio_sim_attr(struct device_attribute *dev_attr) { - return container_of(dev_attr, struct gpio_sim_attribute, dev_attr); + return container_of_const(dev_attr, struct gpio_sim_attribute, dev_attr); } static int gpio_sim_apply_pull(struct gpio_sim_chip *chip, @@ -272,7 +272,7 @@ static void gpio_sim_dbg_show(struct seq_file *seq, struct gpio_chip *gc) static ssize_t gpio_sim_sysfs_val_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); struct gpio_sim_chip *chip = dev_get_drvdata(dev); int val; @@ -302,7 +302,7 @@ static ssize_t gpio_sim_sysfs_pull_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); struct gpio_sim_chip *chip = dev_get_drvdata(dev); int pull; @@ -316,7 +316,7 @@ static ssize_t gpio_sim_sysfs_pull_store(struct device *dev, struct device_attribute *attr, const char *buf, size_t len) { - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); struct gpio_sim_chip *chip = dev_get_drvdata(dev); int ret, pull; --- base-commit: b810335a0699d1810683796b0e3ffe8827ddfe6c change-id: 20261016-gpio-sim-const-sysfs-attr-350706cc7500 Best regards, -- Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> ^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] gpio: sim: access sysfs attributes as const 2026-10-16 8:13 [PATCH RFC] gpio: sim: access sysfs attributes as const Bartosz Golaszewski @ 2026-10-16 8:19 ` sashiko-bot 2026-10-16 8:54 ` Bartosz Golaszewski 2026-10-16 9:07 ` Thomas Weißschuh 1 sibling, 1 reply; 4+ messages in thread From: sashiko-bot @ 2026-10-16 8:19 UTC (permalink / raw) To: Bartosz Golaszewski; +Cc: linux-gpio Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: - [Low] gpio-sim: incomplete sysfs attribute constification -- commit 055dde35eca9fff8d54ca501f4fca6d98889dfd8 Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> gpio: sim: access sysfs attributes as const Use the const variant of constainer_of() to access struct gpio_sim_attribute which wraps the dynamically allocated sysfs attributes for value and pull of simulated GPIO lines in line with the currently preferred approach. > diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c > index 0c73bec042670..e0ae586cd08d8 100644 > --- a/drivers/gpio/gpio-sim.c > +++ b/drivers/gpio/gpio-sim.c > @@ -59,10 +59,10 @@ struct gpio_sim_attribute { > unsigned int offset; > }; > > -static struct gpio_sim_attribute * > +static const struct gpio_sim_attribute * > to_gpio_sim_attr(struct device_attribute *dev_attr) [Severity: Low] Does this function signature need to be updated to take a const struct device_attribute *dev_attr? Since dev_attr is still non-const here, the container_of_const macro will evaluate its non-const branch, resulting in a non-const pointer that gets implicitly cast to const by the return statement. This makes the generic type selection of container_of_const largely redundant. > { > - return container_of(dev_attr, struct gpio_sim_attribute, dev_attr); > + return container_of_const(dev_attr, struct gpio_sim_attribute, dev_attr); > } [ ... ] > @@ -272,7 +272,7 @@ static void gpio_sim_dbg_show(struct seq_file *seq, struct gpio_chip *gc) > static ssize_t gpio_sim_sysfs_val_show(struct device *dev, > struct device_attribute *attr, char *buf) [Severity: Low] Should these sysfs callback signatures be updated to accept a const struct device_attribute *attr? The callbacks still accept a non-const attr parameter, meaning the refactoring to const attributes is incomplete. For a full migration, the callback signatures should accept const struct device_attribute *attr and be assigned to the appropriate show_const / store_const function pointers in the attribute definition. > { > - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > struct gpio_sim_chip *chip = dev_get_drvdata(dev); > int val; -- Sashiko AI review · https://sashiko.dev/#/patchset/20261016-gpio-sim-const-sysfs-attr-v1-1-5d2f832890d6@oss.qualcomm.com?part=1 ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] gpio: sim: access sysfs attributes as const 2026-10-16 8:19 ` sashiko-bot @ 2026-10-16 8:54 ` Bartosz Golaszewski 0 siblings, 0 replies; 4+ messages in thread From: Bartosz Golaszewski @ 2026-10-16 8:54 UTC (permalink / raw) To: sashiko-reviews, Thomas Weißschuh Cc: sashiko-bot, linux-gpio, Bartosz Golaszewski On Fri, 16 Oct 2026 10:19:13 +0200, sashiko-bot@kernel.org said: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > - [Low] gpio-sim: incomplete sysfs attribute constification > -- > > commit 055dde35eca9fff8d54ca501f4fca6d98889dfd8 > Author: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > > gpio: sim: access sysfs attributes as const > > Use the const variant of constainer_of() to access struct > gpio_sim_attribute which wraps the dynamically allocated sysfs > attributes for value and pull of simulated GPIO lines in line with the > currently preferred approach. > >> diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c >> index 0c73bec042670..e0ae586cd08d8 100644 >> --- a/drivers/gpio/gpio-sim.c >> +++ b/drivers/gpio/gpio-sim.c >> @@ -59,10 +59,10 @@ struct gpio_sim_attribute { >> unsigned int offset; >> }; >> >> -static struct gpio_sim_attribute * >> +static const struct gpio_sim_attribute * >> to_gpio_sim_attr(struct device_attribute *dev_attr) > > [Severity: Low] > Does this function signature need to be updated to take a const > struct device_attribute *dev_attr? > > Since dev_attr is still non-const here, the container_of_const macro will > evaluate its non-const branch, resulting in a non-const pointer that gets > implicitly cast to const by the return statement. This makes the generic type > selection of container_of_const largely redundant. > >> { >> - return container_of(dev_attr, struct gpio_sim_attribute, dev_attr); >> + return container_of_const(dev_attr, struct gpio_sim_attribute, dev_attr); >> } > > [ ... ] > >> @@ -272,7 +272,7 @@ static void gpio_sim_dbg_show(struct seq_file *seq, struct gpio_chip *gc) >> static ssize_t gpio_sim_sysfs_val_show(struct device *dev, >> struct device_attribute *attr, char *buf) > > [Severity: Low] > Should these sysfs callback signatures be updated to accept a const struct > device_attribute *attr? > > The callbacks still accept a non-const attr parameter, meaning the refactoring > to const attributes is incomplete. > > For a full migration, the callback signatures should accept const struct > device_attribute *attr and be assigned to the appropriate show_const / > store_const function pointers in the attribute definition. > Well, I guess that answers the question of whether there's more to do. :) Bartosz ^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH RFC] gpio: sim: access sysfs attributes as const 2026-10-16 8:13 [PATCH RFC] gpio: sim: access sysfs attributes as const Bartosz Golaszewski 2026-10-16 8:19 ` sashiko-bot @ 2026-10-16 9:07 ` Thomas Weißschuh 1 sibling, 0 replies; 4+ messages in thread From: Thomas Weißschuh @ 2026-10-16 9:07 UTC (permalink / raw) To: Bartosz Golaszewski Cc: Linus Walleij, Bartosz Golaszewski, linux-gpio, linux-kernel Hi Bartosz! On Fri, Oct 16, 2026 at 10:13:50AM +0200, Bartosz Golaszewski wrote: > Use the const variant of constainer_of() to access struct > gpio_sim_attribute which wraps the dynamically allocated sysfs > attributes for value and pull of simulated GPIO lines in line with the > currently preferred approach. > > Signed-off-by: Bartosz Golaszewski <bartosz.golaszewski@oss.qualcomm.com> > --- > Following your talk, I'm sending a patch for the GPIO simulator module > that attempts to constify the sysfs attributes. Please tell me if this > is all I need here as the attributes are part of another struct that's > allocated dynamically. I'm not sure if there's anything more to do here. > --- > drivers/gpio/gpio-sim.c | 10 +++++----- > 1 file changed, 5 insertions(+), 5 deletions(-) > > diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c > index 0c73bec042670fc5706b4a93700e652cbaf3605e..e0ae586cd08d8adbb3c16918607074fce116654e 100644 > --- a/drivers/gpio/gpio-sim.c > +++ b/drivers/gpio/gpio-sim.c > @@ -59,10 +59,10 @@ struct gpio_sim_attribute { > unsigned int offset; > }; > > -static struct gpio_sim_attribute * > +static const struct gpio_sim_attribute * > to_gpio_sim_attr(struct device_attribute *dev_attr) > { > - return container_of(dev_attr, struct gpio_sim_attribute, dev_attr); > + return container_of_const(dev_attr, struct gpio_sim_attribute, dev_attr); > } > > static int gpio_sim_apply_pull(struct gpio_sim_chip *chip, > @@ -272,7 +272,7 @@ static void gpio_sim_dbg_show(struct seq_file *seq, struct gpio_chip *gc) > static ssize_t gpio_sim_sysfs_val_show(struct device *dev, > struct device_attribute *attr, char *buf) > { > - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > struct gpio_sim_chip *chip = dev_get_drvdata(dev); > int val; > > @@ -302,7 +302,7 @@ static ssize_t gpio_sim_sysfs_pull_show(struct device *dev, > struct device_attribute *attr, > char *buf) > { > - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > struct gpio_sim_chip *chip = dev_get_drvdata(dev); > int pull; > > @@ -316,7 +316,7 @@ static ssize_t gpio_sim_sysfs_pull_store(struct device *dev, > struct device_attribute *attr, > const char *buf, size_t len) > { > - struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > + const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); > struct gpio_sim_chip *chip = dev_get_drvdata(dev); > int ret, pull; You should also use the _const APIs for the dynamically allocated attributes. While does not improve security here, it makes it clearer that the callbacks are not supposed to change these structures and will allow us to remove the non-const variants later. See below: diff --git a/drivers/gpio/gpio-sim.c b/drivers/gpio/gpio-sim.c index e0ae586cd08d..34b45d0d3575 100644 --- a/drivers/gpio/gpio-sim.c +++ b/drivers/gpio/gpio-sim.c @@ -60,7 +60,7 @@ struct gpio_sim_attribute { }; static const struct gpio_sim_attribute * -to_gpio_sim_attr(struct device_attribute *dev_attr) +to_gpio_sim_attr(const struct device_attribute *dev_attr) { return container_of_const(dev_attr, struct gpio_sim_attribute, dev_attr); } @@ -270,7 +270,7 @@ static void gpio_sim_dbg_show(struct seq_file *seq, struct gpio_chip *gc) } static ssize_t gpio_sim_sysfs_val_show(struct device *dev, - struct device_attribute *attr, char *buf) + const struct device_attribute *attr, char *buf) { const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); struct gpio_sim_chip *chip = dev_get_drvdata(dev); @@ -283,7 +283,7 @@ static ssize_t gpio_sim_sysfs_val_show(struct device *dev, } static ssize_t gpio_sim_sysfs_val_store(struct device *dev, - struct device_attribute *attr, + const struct device_attribute *attr, const char *buf, size_t count) { /* @@ -299,7 +299,7 @@ static const char *const gpio_sim_sysfs_pull_strings[] = { }; static ssize_t gpio_sim_sysfs_pull_show(struct device *dev, - struct device_attribute *attr, + const struct device_attribute *attr, char *buf) { const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); @@ -313,7 +313,7 @@ static ssize_t gpio_sim_sysfs_pull_show(struct device *dev, } static ssize_t gpio_sim_sysfs_pull_store(struct device *dev, - struct device_attribute *attr, + const struct device_attribute *attr, const char *buf, size_t len) { const struct gpio_sim_attribute *line_attr = to_gpio_sim_attr(attr); @@ -361,7 +361,7 @@ static int gpio_sim_setup_sysfs(struct gpio_sim_chip *chip) unsigned int num_lines = chip->gc.ngpio; struct device *dev = chip->gc.parent; struct attribute_group *attr_group; - struct attribute **attrs; + const struct attribute **attrs; int i, ret; chip->attr_groups = devm_kcalloc(dev, sizeof(*chip->attr_groups), @@ -396,15 +396,15 @@ static int gpio_sim_setup_sysfs(struct gpio_sim_chip *chip) val_dev_attr->attr.mode = pull_dev_attr->attr.mode = 0644; - val_dev_attr->show = gpio_sim_sysfs_val_show; - val_dev_attr->store = gpio_sim_sysfs_val_store; - pull_dev_attr->show = gpio_sim_sysfs_pull_show; - pull_dev_attr->store = gpio_sim_sysfs_pull_store; + val_dev_attr->show_const = gpio_sim_sysfs_val_show; + val_dev_attr->store_const = gpio_sim_sysfs_val_store; + pull_dev_attr->show_const = gpio_sim_sysfs_pull_show; + pull_dev_attr->store_const = gpio_sim_sysfs_pull_store; attrs[0] = &val_dev_attr->attr; attrs[1] = &pull_dev_attr->attr; - attr_group->attrs = attrs; + attr_group->attrs_const = attrs; chip->attr_groups[i] = attr_group; } As you are touching this file anyways, you could also prepare it for the constification of configfs attributes. See the example below, the same for the other instances in the file. The necessary infrastructure will be part of the next merge window. @@ -1505,7 +1505,7 @@ static const struct configfs_group_operations gpio_sim_bank_config_group_ops = { static const struct config_item_type gpio_sim_bank_config_group_type = { .ct_item_ops = &gpio_sim_bank_config_item_ops, .ct_group_ops = &gpio_sim_bank_config_group_ops, - .ct_attrs = gpio_sim_bank_config_attrs, + .ct_attrs_const = gpio_sim_bank_config_attrs, .ct_owner = THIS_MODULE, }; @@ -1154,7 +1154,7 @@ gpio_sim_bank_config_num_lines_store(struct config_item *item, CONFIGFS_ATTR(gpio_sim_bank_config_, num_lines); -static struct configfs_attribute *gpio_sim_bank_config_attrs[] = { +static const struct configfs_attribute *const gpio_sim_bank_config_attrs[] = { &gpio_sim_bank_config_attr_chip_name, &gpio_sim_bank_config_attr_label, &gpio_sim_bank_config_attr_num_lines, ^ permalink raw reply related [flat|nested] 4+ messages in thread
end of thread, other threads:[~2026-10-16 9:07 UTC | newest] Thread overview: 4+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-10-16 8:13 [PATCH RFC] gpio: sim: access sysfs attributes as const Bartosz Golaszewski 2026-10-16 8:19 ` sashiko-bot 2026-10-16 8:54 ` Bartosz Golaszewski 2026-10-16 9:07 ` Thomas Weißschuh
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.