[alsa-devel] [PATCH 00/15] use permission-specific DEVICE_ATTR variants
Use DEVICE_ATTR_RO etc. for read only attributes etc. This simplifies the source code, improves readbility, and reduces the chance of inconsistencies.
The complete semantic patch is as follows: (http://coccinelle.lip6.fr/)
// <smpl> @ro@ declarer name DEVICE_ATTR; identifier x,x_show; @@
DEVICE_ATTR(x, (0444|S_IRUGO), x_show, NULL);
@wo@ declarer name DEVICE_ATTR; identifier x,x_store; @@
DEVICE_ATTR(x, (0200|S_IWUSR), NULL, x_store);
@rw@ declarer name DEVICE_ATTR; identifier x,x_show,x_store; @@
DEVICE_ATTR(x, (0644|S_IRUGO|S_IWUSR), x_show, x_store);
@script:ocaml@ x << ro.x; x_show << ro.x_show; @@
if not (x^"_show" = x_show) then Coccilib.include_match false
@script:ocaml@ x << wo.x; x_store << wo.x_store; @@
if not (x^"_store" = x_store) then Coccilib.include_match false
@script:ocaml@ x << rw.x; x_show << rw.x_show; x_store << rw.x_store; @@
if not (x^"_show" = x_show && x^"_store" = x_store) then Coccilib.include_match false
@@ declarer name DEVICE_ATTR_RO; identifier ro.x,ro.x_show; @@
- DEVICE_ATTR(x, (0444|S_IRUGO), x_show, NULL); + DEVICE_ATTR_RO(x);
@@ declarer name DEVICE_ATTR_WO; identifier wo.x,wo.x_store; @@
- DEVICE_ATTR(x, (0200|S_IWUSR), NULL, x_store); + DEVICE_ATTR_WO(x);
@@ declarer name DEVICE_ATTR_RW; identifier rw.x,rw.x_show,rw.x_store; @@
- DEVICE_ATTR(x, (0644|S_IRUGO|S_IWUSR), x_show, x_store); + DEVICE_ATTR_RW(x); // </smpl>
---
arch/mips/txx9/generic/7segled.c | 4 ++-- arch/powerpc/kernel/iommu.c | 3 +-- arch/tile/kernel/sysfs.c | 14 +++++++------- drivers/atm/solos-pci.c | 2 +- drivers/pci/pcie/aspm.c | 4 ++-- drivers/power/supply/wm8350_power.c | 2 +- drivers/ptp/ptp_sysfs.c | 2 +- drivers/thermal/int340x_thermal/int3400_thermal.c | 2 +- drivers/thermal/thermal_hwmon.c | 2 +- drivers/tty/nozomi.c | 4 ++-- drivers/usb/wusbcore/dev-sysfs.c | 6 +++--- drivers/usb/wusbcore/wusbhc.c | 13 +++++-------- drivers/video/fbdev/wm8505fb.c | 2 +- sound/soc/omap/mcbsp.c | 4 ++-- sound/soc/soc-dapm.c | 2 +- 15 files changed, 31 insertions(+), 35 deletions(-)
Use DEVICE_ATTR_RW for read-write attributes. This simplifies the source code, improves readbility, and reduces the chance of inconsistencies.
The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/)
// <smpl> @rw@ declarer name DEVICE_ATTR; identifier x,x_show,x_store; @@
DEVICE_ATTR(x, (0644|S_IRUGO|S_IWUSR), x_show, x_store);
@script:ocaml@ x << rw.x; x_show << rw.x_show; x_store << rw.x_store; @@
if not (x^"_show" = x_show && x^"_store" = x_store) then Coccilib.include_match false
@@ declarer name DEVICE_ATTR_RW; identifier rw.x,rw.x_show,rw.x_store; @@
- DEVICE_ATTR(x, (0644|S_IRUGO|S_IWUSR), x_show, x_store); + DEVICE_ATTR_RW(x); // </smpl>
Signed-off-by: Julia Lawall Julia.Lawall@lip6.fr
--- sound/soc/omap/mcbsp.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/soc/omap/mcbsp.c b/sound/soc/omap/mcbsp.c index 06fec56..0b363d1 100644 --- a/sound/soc/omap/mcbsp.c +++ b/sound/soc/omap/mcbsp.c @@ -858,7 +858,7 @@ static ssize_t dma_op_mode_store(struct device *dev, return size; }
-static DEVICE_ATTR(dma_op_mode, 0644, dma_op_mode_show, dma_op_mode_store); +static DEVICE_ATTR_RW(dma_op_mode);
static const struct attribute *additional_attrs[] = { &dev_attr_max_tx_thres.attr, @@ -927,7 +927,7 @@ static ssize_t st_taps_store(struct device *dev, return size; }
-static DEVICE_ATTR(st_taps, 0644, st_taps_show, st_taps_store); +static DEVICE_ATTR_RW(st_taps);
static const struct attribute *sidetone_attrs[] = { &dev_attr_st_taps.attr,
Hi
On Sat, 29 Oct 2016 21:37:04 +0200 Julia Lawall Julia.Lawall@lip6.fr wrote:
Use DEVICE_ATTR_RW for read-write attributes. This simplifies the source code, improves readbility, and reduces the chance of inconsistencies.
...
- DEVICE_ATTR(x, (0644|S_IRUGO|S_IWUSR), x_show, x_store);
- DEVICE_ATTR_RW(x);
I'm not so sure does this improve readability. 644 is pretty obvious but for DEVICE_ATTR_RW() one has to dive into include/linux/device.h and include/linux/sysfs.h to see for what users it grants the write access.
On Sun, 30 Oct 2016, Jarkko Nikula wrote:
Hi
On Sat, 29 Oct 2016 21:37:04 +0200 Julia Lawall Julia.Lawall@lip6.fr wrote:
Use DEVICE_ATTR_RW for read-write attributes. This simplifies the source code, improves readbility, and reduces the chance of inconsistencies.
...
- DEVICE_ATTR(x, (0644|S_IRUGO|S_IWUSR), x_show, x_store);
- DEVICE_ATTR_RW(x);
I'm not so sure does this improve readability. 644 is pretty obvious but for DEVICE_ATTR_RW() one has to dive into include/linux/device.h and include/linux/sysfs.h to see for what users it grants the write access.
OK, as you like. It does help ensure that the functions that are supposed to be defined are available. There were a couple of occurrences of 0644 with no show or no store function. Among the three declarers, there are currently in total over 800 uses in the kernel, so they are also not so obscure.
julia
Use DEVICE_ATTR_RO for read only attributes. This simplifies the source code, improves readbility, and reduces the chance of inconsistencies.
The semantic patch that makes this change is as follows: (http://coccinelle.lip6.fr/)
// <smpl> @ro@ declarer name DEVICE_ATTR; identifier x,x_show; @@
DEVICE_ATTR(x, (0444|S_IRUGO), x_show, NULL);
@script:ocaml@ x << ro.x; x_show << ro.x_show; @@
if not (x^"_show" = x_show) then Coccilib.include_match false
@@ declarer name DEVICE_ATTR_RO; identifier ro.x,ro.x_show; @@
- DEVICE_ATTR(x, (0444|S_IRUGO), x_show, NULL); + DEVICE_ATTR_RO(x); // </smpl>
Signed-off-by: Julia Lawall Julia.Lawall@lip6.fr
--- sound/soc/soc-dapm.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/sound/soc/soc-dapm.c b/sound/soc/soc-dapm.c index 3bbe32e..164b4d3 100644 --- a/sound/soc/soc-dapm.c +++ b/sound/soc/soc-dapm.c @@ -2297,7 +2297,7 @@ static ssize_t dapm_widget_show(struct device *dev, return count; }
-static DEVICE_ATTR(dapm_widget, 0444, dapm_widget_show, NULL); +static DEVICE_ATTR_RO(dapm_widget);
struct attribute *soc_dapm_dev_attrs[] = { &dev_attr_dapm_widget.attr,
participants (2)
-
Jarkko Nikula -
Julia Lawall