| Message ID | 1477769829-22230-14-git-send-email-Julia.Lawall@lip6.fr (mailing list archive) |
|---|---|
| State | New, archived |
| Delegated to: | Bjorn Helgaas |
| Headers | show |
On Sat, Oct 29, 2016 at 09:37:07PM +0200, Julia Lawall wrote: > 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> I applied this to pci/aspm to follow the herd, although it looks pretty similar to the ill-fated "Replace numeric parameter like 0444 with macro" series (http://lwn.net/Articles/696229/). Maybe this is different because everybody except me knows what ATTR_RW means? To me, "0644" contained more information than "_RW" does. I do certainly like the removal of the "_show" and "_store" redundancy. > --- > drivers/pci/pcie/aspm.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > index 0ec649d..3b14d9e 100644 > --- a/drivers/pci/pcie/aspm.c > +++ b/drivers/pci/pcie/aspm.c > @@ -886,8 +886,8 @@ static ssize_t clk_ctl_store(struct device *dev, > return n; > } > > -static DEVICE_ATTR(link_state, 0644, link_state_show, link_state_store); > -static DEVICE_ATTR(clk_ctl, 0644, clk_ctl_show, clk_ctl_store); > +static DEVICE_ATTR_RW(link_state); > +static DEVICE_ATTR_RW(clk_ctl); > > static char power_group[] = "power"; > void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev) > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Mon, 14 Nov 2016, Bjorn Helgaas wrote: > On Sat, Oct 29, 2016 at 09:37:07PM +0200, Julia Lawall wrote: > > 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> > > I applied this to pci/aspm to follow the herd, although it looks > pretty similar to the ill-fated "Replace numeric parameter like 0444 > with macro" series (http://lwn.net/Articles/696229/). Maybe this is > different because everybody except me knows what ATTR_RW means? To > me, "0644" contained more information than "_RW" does. > > I do certainly like the removal of the "_show" and "_store" > redundancy. I think that the point is the latter. There were also a couple of cases where the permissions didn't match with the set of provided functions. julia > > > --- > > drivers/pci/pcie/aspm.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c > > index 0ec649d..3b14d9e 100644 > > --- a/drivers/pci/pcie/aspm.c > > +++ b/drivers/pci/pcie/aspm.c > > @@ -886,8 +886,8 @@ static ssize_t clk_ctl_store(struct device *dev, > > return n; > > } > > > > -static DEVICE_ATTR(link_state, 0644, link_state_show, link_state_store); > > -static DEVICE_ATTR(clk_ctl, 0644, clk_ctl_show, clk_ctl_store); > > +static DEVICE_ATTR_RW(link_state); > > +static DEVICE_ATTR_RW(clk_ctl); > > > > static char power_group[] = "power"; > > void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev) > > > -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/drivers/pci/pcie/aspm.c b/drivers/pci/pcie/aspm.c index 0ec649d..3b14d9e 100644 --- a/drivers/pci/pcie/aspm.c +++ b/drivers/pci/pcie/aspm.c @@ -886,8 +886,8 @@ static ssize_t clk_ctl_store(struct device *dev, return n; } -static DEVICE_ATTR(link_state, 0644, link_state_show, link_state_store); -static DEVICE_ATTR(clk_ctl, 0644, clk_ctl_show, clk_ctl_store); +static DEVICE_ATTR_RW(link_state); +static DEVICE_ATTR_RW(clk_ctl); static char power_group[] = "power"; void pcie_aspm_create_sysfs_dev_files(struct pci_dev *pdev)
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> --- drivers/pci/pcie/aspm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- To unsubscribe from this list: send the line "unsubscribe linux-pci" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html