Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1546361 > unrolled thread

[PATCH 00/66] use permission-specific DEVICE_ATTR variants

Started byJulia Lawall <Julia.Lawall@lip6.fr>
First post2016-12-22 13:50 +0100
Last post2016-12-23 14:10 +0100
Articles 8 — 3 participants

Back to article view | Back to linux.kernel

Contents

  [PATCH 00/66] use permission-specific DEVICE_ATTR variants Julia Lawall <Julia.Lawall@lip6.fr> - 2016-12-22 13:50 +0100
    [PATCH 02/66] hwmon: (adm1026) use permission-specific DEVICE_ATTR variants Julia Lawall <Julia.Lawall@lip6.fr> - 2016-12-22 13:50 +0100
    [PATCH 11/66] hwmon: (fschmd) use permission-specific DEVICE_ATTR variants Julia Lawall <Julia.Lawall@lip6.fr> - 2016-12-22 13:50 +0100
    [PATCH 24/66] hwmon: (lm87) use permission-specific DEVICE_ATTR variants Julia Lawall <Julia.Lawall@lip6.fr> - 2016-12-22 13:50 +0100
    [PATCH 06/66] hwmon: (adt7x10) use permission-specific DEVICE_ATTR variants Julia Lawall <Julia.Lawall@lip6.fr> - 2016-12-22 13:50 +0100
    Re: [PATCH 00/66] use permission-specific DEVICE_ATTR variants Guenter Roeck <linux@roeck-us.net> - 2016-12-23 03:40 +0100
      Re: [PATCH 00/66] use permission-specific DEVICE_ATTR variants Julia Lawall <julia.lawall@lip6.fr> - 2016-12-23 07:50 +0100
      Re: [PATCH 00/66] use permission-specific DEVICE_ATTR variants Julia Lawall <julia.lawall@lip6.fr> - 2016-12-23 14:10 +0100

#1546361 — [PATCH 00/66] use permission-specific DEVICE_ATTR variants

FromJulia Lawall <Julia.Lawall@lip6.fr>
Date2016-12-22 13:50 +0100
Subject[PATCH 00/66] use permission-specific DEVICE_ATTR variants
Message-ID<sRaFr-10p-3@gated-at.bofh.it>
Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.  A previous version of this semantic patch transformed
only cases where the show and store functions followed particular naming
conventions.  This semantic patch additionally renames functions as
needed.  It can, however, fail to transform some occurrences where a single
function is used for multiple attributes.  In that case, the first case is
transformed an the others are left as is.

The complete semantic patch is as follows:
(http://coccinelle.lip6.fr/)

// <smpl>
@initialize:ocaml@
@@

let taken = Hashtbl.create 101

// -------------------------------------------------------------------------
// Easy case

@d@
declarer name DEVICE_ATTR;
identifier x;
@@

DEVICE_ATTR(x,...);

@script:ocaml depends on d@
@@

Hashtbl.clear taken

@script:ocaml expected@
x << d.x;
x_show;
x_store;
@@
x_show := make_ident (x^"_show");
x_store := make_ident (x^"_store")

@@
declarer name DEVICE_ATTR_RO;
identifier d.x,expected.x_show;
@@

- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);

@@
declarer name DEVICE_ATTR_WO;
identifier d.x,expected.x_store;
@@

- DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);
+ DEVICE_ATTR_WO(x);

@@
declarer name DEVICE_ATTR_RW;
identifier d.x,expected.x_show,expected.x_store;
@@

- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);

// -------------------------------------------------------------------------
// Other calls

@o@
declarer name DEVICE_ATTR;
identifier d.x,show,store;
@@

DEVICE_ATTR(x,\(0444\|S_IRUGO\|0200\|S_IWUSR\|0644\|S_IRUGO|S_IWUSR\),
            show,store);

@script:ocaml@
x << d.x;
show << o.show;
store << o.store;
@@

if (not(show = "NULL") && Hashtbl.mem taken show) ||
   (not(store = "NULL") && Hashtbl.mem taken store)
then Coccilib.include_match false
else (Hashtbl.add taken show (); Hashtbl.add taken store ())

// rename functions

@show1@
identifier o.show,expected.x_show;
parameter list ps;
@@

static
- show(ps)
+ x_show(ps)
  { ... }

@depends on show1@
identifier o.show,expected.x_show;
expression list es;
@@
- show(es)
+ x_show(es)

@depends on show1@
identifier o.show,expected.x_show;
@@
- show
+ x_show

@store1@
identifier o.store,expected.x_store;
parameter list ps;
@@

static
- store(ps)
+ x_store(ps)
  { ... }

@depends on store1@
identifier o.store,expected.x_store;
expression list es;
@@
- store(es)
+ x_store(es)

@depends on store1@
identifier o.store,expected.x_store;
@@
- store
+ x_store

// try again

@@
declarer name DEVICE_ATTR_RO;
identifier d.x,expected.x_show;
@@

- DEVICE_ATTR(x, \(0444\|S_IRUGO\), x_show, NULL);
+ DEVICE_ATTR_RO(x);

@@
declarer name DEVICE_ATTR_WO;
identifier d.x,expected.x_store;
@@

- DEVICE_ATTR(x, \(0200\|S_IWUSR\), NULL, x_store);
+ DEVICE_ATTR_WO(x);

@@
declarer name DEVICE_ATTR_RW;
identifier d.x,expected.x_show,expected.x_store;
@@

- DEVICE_ATTR(x, \(0644\|S_IRUGO|S_IWUSR\), x_show, x_store);
+ DEVICE_ATTR_RW(x);
// </smpl>

---

 drivers/hwmon/adm1021.c      |   14 ++--
 drivers/hwmon/adm1025.c      |   16 ++---
 drivers/hwmon/adm1026.c      |  128 ++++++++++++++++++++++---------------------
 drivers/hwmon/adm1031.c      |   15 ++---
 drivers/hwmon/adm9240.c      |   28 ++++-----
 drivers/hwmon/adt7470.c      |   48 +++++++---------
 drivers/hwmon/adt7475.c      |   28 ++++-----
 drivers/hwmon/adt7x10.c      |    7 +-
 drivers/hwmon/asb100.c       |   36 ++++++------
 drivers/hwmon/atxp1.c        |   35 +++++------
 drivers/hwmon/dme1737.c      |   18 +++---
 drivers/hwmon/ds1621.c       |   16 ++---
 drivers/hwmon/emc2103.c      |   36 ++++++------
 drivers/hwmon/f71805f.c      |   16 ++---
 drivers/hwmon/f71882fg.c     |    6 +-
 drivers/hwmon/fam15h_power.c |   34 +++++------
 drivers/hwmon/fschmd.c       |    6 +-
 drivers/hwmon/g760a.c        |   22 +++----
 drivers/hwmon/g762.c         |   86 ++++++++++++++--------------
 drivers/hwmon/gl520sm.c      |   48 ++++++++--------
 drivers/hwmon/gpio-fan.c     |   54 +++++++++---------
 drivers/hwmon/hwmon.c        |    4 -
 drivers/hwmon/i5500_temp.c   |    6 +-
 drivers/hwmon/i5k_amb.c      |    4 -
 drivers/hwmon/it87.c         |   20 +++---
 drivers/hwmon/jz4740-hwmon.c |    6 +-
 drivers/hwmon/k10temp.c      |   12 ++--
 drivers/hwmon/k8temp.c       |    4 -
 drivers/hwmon/lm63.c         |   48 +++++++---------
 drivers/hwmon/lm70.c         |    6 +-
 drivers/hwmon/lm78.c         |   38 ++++++------
 drivers/hwmon/lm80.c         |    4 -
 drivers/hwmon/lm83.c         |    4 -
 drivers/hwmon/lm85.c         |   22 +++----
 drivers/hwmon/lm87.c         |   43 +++++++-------
 drivers/hwmon/lm90.c         |    8 +-
 drivers/hwmon/lm92.c         |   10 +--
 drivers/hwmon/lm93.c         |   39 +++++--------
 drivers/hwmon/lm95234.c      |   12 ++--
 drivers/hwmon/max1111.c      |    4 -
 drivers/hwmon/max1619.c      |    4 -
 drivers/hwmon/max197.c       |    6 +-
 drivers/hwmon/max6650.c      |   44 ++++++++------
 drivers/hwmon/mc13783-adc.c  |    6 +-
 drivers/hwmon/mcp3021.c      |    6 +-
 drivers/hwmon/nct6683.c      |   17 ++---
 drivers/hwmon/nct6775.c      |    4 -
 drivers/hwmon/nsa320-hwmon.c |   12 ++--
 drivers/hwmon/pc87360.c      |   26 ++++----
 drivers/hwmon/pc87427.c      |    4 -
 drivers/hwmon/pcf8591.c      |   24 +++-----
 drivers/hwmon/sch5627.c      |    4 -
 drivers/hwmon/sht15.c        |    4 -
 drivers/hwmon/sis5595.c      |   36 ++++++------
 drivers/hwmon/smsc47m1.c     |   10 +--
 drivers/hwmon/smsc47m192.c   |   14 ++--
 drivers/hwmon/tmp401.c       |   11 +--
 drivers/hwmon/via-cputemp.c  |    6 +-
 drivers/hwmon/via686a.c      |    8 +-
 drivers/hwmon/vt8231.c       |   59 ++++++++++---------
 drivers/hwmon/w83627ehf.c    |    8 +-
 drivers/hwmon/w83627hf.c     |   53 +++++++++--------
 drivers/hwmon/w83781d.c      |   34 +++++------
 drivers/hwmon/w83791d.c      |   23 +++----
 drivers/hwmon/w83792d.c      |   15 ++---
 drivers/hwmon/w83793.c       |    6 +-
 66 files changed, 709 insertions(+), 726 deletions(-)

[toc] | [next] | [standalone]

#1546362 — [PATCH 02/66] hwmon: (adm1026) use permission-specific DEVICE_ATTR variants

FromJulia Lawall <Julia.Lawall@lip6.fr>
Date2016-12-22 13:50 +0100
Subject[PATCH 02/66] hwmon: (adm1026) use permission-specific DEVICE_ATTR variants
Message-ID<sRaYO-17u-39@gated-at.bofh.it>
In reply to#1546361
Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The semantic patch for the RO case, in the case where the show function
already has the expected name, 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>

---
 drivers/hwmon/adm1026.c |  128 ++++++++++++++++++++++++------------------------
 1 file changed, 66 insertions(+), 62 deletions(-)

diff --git a/drivers/hwmon/adm1026.c b/drivers/hwmon/adm1026.c
index b2a5d9e..e43f09a 100644
--- a/drivers/hwmon/adm1026.c
+++ b/drivers/hwmon/adm1026.c
@@ -1034,15 +1034,15 @@ static SENSOR_DEVICE_ATTR(temp##offset##_crit, S_IRUGO | S_IWUSR,	\
 temp_crit_reg(2);
 temp_crit_reg(3);
 
-static ssize_t show_analog_out_reg(struct device *dev,
-				   struct device_attribute *attr, char *buf)
+static ssize_t analog_out_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%d\n", DAC_FROM_REG(data->analog_out));
 }
-static ssize_t set_analog_out_reg(struct device *dev,
-				  struct device_attribute *attr,
-				  const char *buf, size_t count)
+static ssize_t analog_out_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1060,11 +1060,10 @@ static ssize_t set_analog_out_reg(struct device *dev,
 	return count;
 }
 
-static DEVICE_ATTR(analog_out, S_IRUGO | S_IWUSR, show_analog_out_reg,
-	set_analog_out_reg);
+static DEVICE_ATTR_RW(analog_out);
 
-static ssize_t show_vid_reg(struct device *dev, struct device_attribute *attr,
-			    char *buf)
+static ssize_t cpu0_vid_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	int vid = (data->gpio >> 11) & 0x1f;
@@ -1073,17 +1072,17 @@ static ssize_t show_vid_reg(struct device *dev, struct device_attribute *attr,
 	return sprintf(buf, "%d\n", vid_from_reg(vid, data->vrm));
 }
 
-static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid_reg, NULL);
+static DEVICE_ATTR_RO(cpu0_vid);
 
-static ssize_t show_vrm_reg(struct device *dev, struct device_attribute *attr,
-			    char *buf)
+static ssize_t vrm_show(struct device *dev, struct device_attribute *attr,
+			char *buf)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	return sprintf(buf, "%d\n", data->vrm);
 }
 
-static ssize_t store_vrm_reg(struct device *dev, struct device_attribute *attr,
-			     const char *buf, size_t count)
+static ssize_t vrm_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	unsigned long val;
@@ -1100,16 +1099,16 @@ static ssize_t store_vrm_reg(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm_reg, store_vrm_reg);
+static DEVICE_ATTR_RW(vrm);
 
-static ssize_t show_alarms_reg(struct device *dev,
-			       struct device_attribute *attr, char *buf)
+static ssize_t alarms_show(struct device *dev, struct device_attribute *attr,
+			   char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%ld\n", data->alarms);
 }
 
-static DEVICE_ATTR(alarms, S_IRUGO, show_alarms_reg, NULL);
+static DEVICE_ATTR_RO(alarms);
 
 static ssize_t show_alarm(struct device *dev, struct device_attribute *attr,
 			  char *buf)
@@ -1148,14 +1147,15 @@ static ssize_t show_alarm(struct device *dev, struct device_attribute *attr,
 static SENSOR_DEVICE_ATTR(in10_alarm, S_IRUGO, show_alarm, NULL, 25);
 static SENSOR_DEVICE_ATTR(in8_alarm, S_IRUGO, show_alarm, NULL, 26);
 
-static ssize_t show_alarm_mask(struct device *dev,
+static ssize_t alarm_mask_show(struct device *dev,
 			       struct device_attribute *attr, char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%ld\n", data->alarm_mask);
 }
-static ssize_t set_alarm_mask(struct device *dev, struct device_attribute *attr,
-			      const char *buf, size_t count)
+static ssize_t alarm_mask_store(struct device *dev,
+				struct device_attribute *attr,
+				const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1186,18 +1186,17 @@ static ssize_t set_alarm_mask(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(alarm_mask, S_IRUGO | S_IWUSR, show_alarm_mask,
-	set_alarm_mask);
+static DEVICE_ATTR_RW(alarm_mask);
 
 
-static ssize_t show_gpio(struct device *dev, struct device_attribute *attr,
+static ssize_t gpio_show(struct device *dev, struct device_attribute *attr,
 			 char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%ld\n", data->gpio);
 }
-static ssize_t set_gpio(struct device *dev, struct device_attribute *attr,
-			const char *buf, size_t count)
+static ssize_t gpio_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1221,16 +1220,18 @@ static ssize_t set_gpio(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(gpio, S_IRUGO | S_IWUSR, show_gpio, set_gpio);
+static DEVICE_ATTR_RW(gpio);
 
-static ssize_t show_gpio_mask(struct device *dev, struct device_attribute *attr,
+static ssize_t gpio_mask_show(struct device *dev,
+			      struct device_attribute *attr,
 			      char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%ld\n", data->gpio_mask);
 }
-static ssize_t set_gpio_mask(struct device *dev, struct device_attribute *attr,
-			     const char *buf, size_t count)
+static ssize_t gpio_mask_store(struct device *dev,
+			       struct device_attribute *attr, const char *buf,
+			       size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1254,17 +1255,17 @@ static ssize_t set_gpio_mask(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static DEVICE_ATTR(gpio_mask, S_IRUGO | S_IWUSR, show_gpio_mask, set_gpio_mask);
+static DEVICE_ATTR_RW(gpio_mask);
 
-static ssize_t show_pwm_reg(struct device *dev, struct device_attribute *attr,
-			    char *buf)
+static ssize_t pwm1_show(struct device *dev, struct device_attribute *attr,
+			 char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%d\n", PWM_FROM_REG(data->pwm1.pwm));
 }
 
-static ssize_t set_pwm_reg(struct device *dev, struct device_attribute *attr,
-			   const char *buf, size_t count)
+static ssize_t pwm1_store(struct device *dev, struct device_attribute *attr,
+			  const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1285,16 +1286,17 @@ static ssize_t set_pwm_reg(struct device *dev, struct device_attribute *attr,
 	return count;
 }
 
-static ssize_t show_auto_pwm_min(struct device *dev,
-				 struct device_attribute *attr, char *buf)
+static ssize_t temp1_auto_point1_pwm_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%d\n", data->pwm1.auto_pwm_min);
 }
 
-static ssize_t set_auto_pwm_min(struct device *dev,
-				struct device_attribute *attr, const char *buf,
-				size_t count)
+static ssize_t temp1_auto_point1_pwm_store(struct device *dev,
+					   struct device_attribute *attr,
+					   const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1316,21 +1318,23 @@ static ssize_t set_auto_pwm_min(struct device *dev,
 	return count;
 }
 
-static ssize_t show_auto_pwm_max(struct device *dev,
-				 struct device_attribute *attr, char *buf)
+static ssize_t temp1_auto_point2_pwm_show(struct device *dev,
+					  struct device_attribute *attr,
+					  char *buf)
 {
 	return sprintf(buf, "%d\n", ADM1026_PWM_MAX);
 }
 
-static ssize_t show_pwm_enable(struct device *dev,
-			       struct device_attribute *attr, char *buf)
+static ssize_t pwm1_enable_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
 {
 	struct adm1026_data *data = adm1026_update_device(dev);
 	return sprintf(buf, "%d\n", data->pwm1.enable);
 }
 
-static ssize_t set_pwm_enable(struct device *dev, struct device_attribute *attr,
-			      const char *buf, size_t count)
+static ssize_t pwm1_enable_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
 {
 	struct adm1026_data *data = dev_get_drvdata(dev);
 	struct i2c_client *client = data->client;
@@ -1366,25 +1370,25 @@ static ssize_t set_pwm_enable(struct device *dev, struct device_attribute *attr,
 }
 
 /* enable PWM fan control */
-static DEVICE_ATTR(pwm1, S_IRUGO | S_IWUSR, show_pwm_reg, set_pwm_reg);
-static DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, show_pwm_reg, set_pwm_reg);
-static DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, show_pwm_reg, set_pwm_reg);
-static DEVICE_ATTR(pwm1_enable, S_IRUGO | S_IWUSR, show_pwm_enable,
-	set_pwm_enable);
-static DEVICE_ATTR(pwm2_enable, S_IRUGO | S_IWUSR, show_pwm_enable,
-	set_pwm_enable);
-static DEVICE_ATTR(pwm3_enable, S_IRUGO | S_IWUSR, show_pwm_enable,
-	set_pwm_enable);
-static DEVICE_ATTR(temp1_auto_point1_pwm, S_IRUGO | S_IWUSR,
-	show_auto_pwm_min, set_auto_pwm_min);
+static DEVICE_ATTR_RW(pwm1);
+static DEVICE_ATTR(pwm2, S_IRUGO | S_IWUSR, pwm1_show, pwm1_store);
+static DEVICE_ATTR(pwm3, S_IRUGO | S_IWUSR, pwm1_show, pwm1_store);
+static DEVICE_ATTR_RW(pwm1_enable);
+static DEVICE_ATTR(pwm2_enable, S_IRUGO | S_IWUSR, pwm1_enable_show,
+		   pwm1_enable_store);
+static DEVICE_ATTR(pwm3_enable, S_IRUGO | S_IWUSR, pwm1_enable_show,
+		   pwm1_enable_store);
+static DEVICE_ATTR_RW(temp1_auto_point1_pwm);
 static DEVICE_ATTR(temp2_auto_point1_pwm, S_IRUGO | S_IWUSR,
-	show_auto_pwm_min, set_auto_pwm_min);
+	temp1_auto_point1_pwm_show, temp1_auto_point1_pwm_store);
 static DEVICE_ATTR(temp3_auto_point1_pwm, S_IRUGO | S_IWUSR,
-	show_auto_pwm_min, set_auto_pwm_min);
+	temp1_auto_point1_pwm_show, temp1_auto_point1_pwm_store);
 
-static DEVICE_ATTR(temp1_auto_point2_pwm, S_IRUGO, show_auto_pwm_max, NULL);
-static DEVICE_ATTR(temp2_auto_point2_pwm, S_IRUGO, show_auto_pwm_max, NULL);
-static DEVICE_ATTR(temp3_auto_point2_pwm, S_IRUGO, show_auto_pwm_max, NULL);
+static DEVICE_ATTR_RO(temp1_auto_point2_pwm);
+static DEVICE_ATTR(temp2_auto_point2_pwm, S_IRUGO, temp1_auto_point2_pwm_show,
+		   NULL);
+static DEVICE_ATTR(temp3_auto_point2_pwm, S_IRUGO, temp1_auto_point2_pwm_show,
+		   NULL);
 
 static struct attribute *adm1026_attributes[] = {
 	&sensor_dev_attr_in0_input.dev_attr.attr,

[toc] | [prev] | [next] | [standalone]

#1546363 — [PATCH 11/66] hwmon: (fschmd) use permission-specific DEVICE_ATTR variants

FromJulia Lawall <Julia.Lawall@lip6.fr>
Date2016-12-22 13:50 +0100
Subject[PATCH 11/66] hwmon: (fschmd) use permission-specific DEVICE_ATTR variants
Message-ID<sRaYO-17u-47@gated-at.bofh.it>
In reply to#1546361
Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The semantic patch for the RO case, in the case where the show function
already has the expected name, 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>

---
 drivers/hwmon/fschmd.c |    6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/drivers/hwmon/fschmd.c b/drivers/hwmon/fschmd.c
index d58abdc..5e78229 100644
--- a/drivers/hwmon/fschmd.c
+++ b/drivers/hwmon/fschmd.c
@@ -561,7 +561,7 @@ static ssize_t store_pwm_auto_point1_pwm(struct device *dev,
  * The FSC hwmon family has the ability to force an attached alert led to flash
  * from software, we export this as an alert_led sysfs attr
  */
-static ssize_t show_alert_led(struct device *dev,
+static ssize_t alert_led_show(struct device *dev,
 	struct device_attribute *devattr, char *buf)
 {
 	struct fschmd_data *data = fschmd_update_device(dev);
@@ -572,7 +572,7 @@ static ssize_t show_alert_led(struct device *dev,
 		return sprintf(buf, "0\n");
 }
 
-static ssize_t store_alert_led(struct device *dev,
+static ssize_t alert_led_store(struct device *dev,
 	struct device_attribute *devattr, const char *buf, size_t count)
 {
 	u8 reg;
@@ -602,7 +602,7 @@ static ssize_t store_alert_led(struct device *dev,
 	return count;
 }
 
-static DEVICE_ATTR(alert_led, 0644, show_alert_led, store_alert_led);
+static DEVICE_ATTR_RW(alert_led);
 
 static struct sensor_device_attribute fschmd_attr[] = {
 	SENSOR_ATTR(in0_input, 0444, show_in_value, NULL, 0),

[toc] | [prev] | [next] | [standalone]

#1546364 — [PATCH 24/66] hwmon: (lm87) use permission-specific DEVICE_ATTR variants

FromJulia Lawall <Julia.Lawall@lip6.fr>
Date2016-12-22 13:50 +0100
Subject[PATCH 24/66] hwmon: (lm87) use permission-specific DEVICE_ATTR variants
Message-ID<sRaYO-17u-41@gated-at.bofh.it>
In reply to#1546361
Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The semantic patch for the RO case, in the case where the show function
already has the expected name, 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>

---
 drivers/hwmon/lm87.c |   43 ++++++++++++++++++++++---------------------
 1 file changed, 22 insertions(+), 21 deletions(-)

diff --git a/drivers/hwmon/lm87.c b/drivers/hwmon/lm87.c
index 13cca36..e06faf9 100644
--- a/drivers/hwmon/lm87.c
+++ b/drivers/hwmon/lm87.c
@@ -445,23 +445,23 @@ static SENSOR_DEVICE_ATTR(temp##offset##_min, S_IRUGO | S_IWUSR, \
 set_temp(2);
 set_temp(3);
 
-static ssize_t show_temp_crit_int(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t temp1_crit_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
 {
 	struct lm87_data *data = lm87_update_device(dev);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_crit_int));
 }
 
-static ssize_t show_temp_crit_ext(struct device *dev,
-				  struct device_attribute *attr, char *buf)
+static ssize_t temp2_crit_show(struct device *dev,
+			       struct device_attribute *attr, char *buf)
 {
 	struct lm87_data *data = lm87_update_device(dev);
 	return sprintf(buf, "%d\n", TEMP_FROM_REG(data->temp_crit_ext));
 }
 
-static DEVICE_ATTR(temp1_crit, S_IRUGO, show_temp_crit_int, NULL);
-static DEVICE_ATTR(temp2_crit, S_IRUGO, show_temp_crit_ext, NULL);
-static DEVICE_ATTR(temp3_crit, S_IRUGO, show_temp_crit_ext, NULL);
+static DEVICE_ATTR_RO(temp1_crit);
+static DEVICE_ATTR_RO(temp2_crit);
+static DEVICE_ATTR(temp3_crit, S_IRUGO, temp2_crit_show, NULL);
 
 static ssize_t show_fan_input(struct device *dev,
 			      struct device_attribute *attr, char *buf)
@@ -586,30 +586,30 @@ static SENSOR_DEVICE_ATTR(fan##offset##_div, S_IRUGO | S_IWUSR, \
 set_fan(1);
 set_fan(2);
 
-static ssize_t show_alarms(struct device *dev, struct device_attribute *attr,
+static ssize_t alarms_show(struct device *dev, struct device_attribute *attr,
 			   char *buf)
 {
 	struct lm87_data *data = lm87_update_device(dev);
 	return sprintf(buf, "%d\n", data->alarms);
 }
-static DEVICE_ATTR(alarms, S_IRUGO, show_alarms, NULL);
+static DEVICE_ATTR_RO(alarms);
 
-static ssize_t show_vid(struct device *dev, struct device_attribute *attr,
-			char *buf)
+static ssize_t cpu0_vid_show(struct device *dev,
+			     struct device_attribute *attr, char *buf)
 {
 	struct lm87_data *data = lm87_update_device(dev);
 	return sprintf(buf, "%d\n", vid_from_reg(data->vid, data->vrm));
 }
-static DEVICE_ATTR(cpu0_vid, S_IRUGO, show_vid, NULL);
+static DEVICE_ATTR_RO(cpu0_vid);
 
-static ssize_t show_vrm(struct device *dev, struct device_attribute *attr,
+static ssize_t vrm_show(struct device *dev, struct device_attribute *attr,
 			char *buf)
 {
 	struct lm87_data *data = dev_get_drvdata(dev);
 	return sprintf(buf, "%d\n", data->vrm);
 }
-static ssize_t set_vrm(struct device *dev, struct device_attribute *attr,
-		       const char *buf, size_t count)
+static ssize_t vrm_store(struct device *dev, struct device_attribute *attr,
+			 const char *buf, size_t count)
 {
 	struct lm87_data *data = dev_get_drvdata(dev);
 	unsigned long val;
@@ -625,16 +625,17 @@ static ssize_t set_vrm(struct device *dev, struct device_attribute *attr,
 	data->vrm = val;
 	return count;
 }
-static DEVICE_ATTR(vrm, S_IRUGO | S_IWUSR, show_vrm, set_vrm);
+static DEVICE_ATTR_RW(vrm);
 
-static ssize_t show_aout(struct device *dev, struct device_attribute *attr,
-			 char *buf)
+static ssize_t aout_output_show(struct device *dev,
+				struct device_attribute *attr, char *buf)
 {
 	struct lm87_data *data = lm87_update_device(dev);
 	return sprintf(buf, "%d\n", AOUT_FROM_REG(data->aout));
 }
-static ssize_t set_aout(struct device *dev, struct device_attribute *attr,
-			const char *buf, size_t count)
+static ssize_t aout_output_store(struct device *dev,
+				 struct device_attribute *attr,
+				 const char *buf, size_t count)
 {
 	struct i2c_client *client = dev_get_drvdata(dev);
 	struct lm87_data *data = i2c_get_clientdata(client);
@@ -651,7 +652,7 @@ static ssize_t set_aout(struct device *dev, struct device_attribute *attr,
 	mutex_unlock(&data->update_lock);
 	return count;
 }
-static DEVICE_ATTR(aout_output, S_IRUGO | S_IWUSR, show_aout, set_aout);
+static DEVICE_ATTR_RW(aout_output);
 
 static ssize_t show_alarm(struct device *dev, struct device_attribute *attr,
 			  char *buf)

[toc] | [prev] | [next] | [standalone]

#1546365 — [PATCH 06/66] hwmon: (adt7x10) use permission-specific DEVICE_ATTR variants

FromJulia Lawall <Julia.Lawall@lip6.fr>
Date2016-12-22 13:50 +0100
Subject[PATCH 06/66] hwmon: (adt7x10) use permission-specific DEVICE_ATTR variants
Message-ID<sRaYP-17u-55@gated-at.bofh.it>
In reply to#1546361
Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
source code, improves readbility, and reduces the chance of
inconsistencies.

The semantic patch for the RO case, in the case where the show function
already has the expected name, 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>

---
 drivers/hwmon/adt7x10.c |    7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/drivers/hwmon/adt7x10.c b/drivers/hwmon/adt7x10.c
index 98141f4..0f538f8 100644
--- a/drivers/hwmon/adt7x10.c
+++ b/drivers/hwmon/adt7x10.c
@@ -331,9 +331,8 @@ static ssize_t adt7x10_show_alarm(struct device *dev,
 	return sprintf(buf, "%d\n", !!(ret & attr->index));
 }
 
-static ssize_t adt7x10_show_name(struct device *dev,
-				 struct device_attribute *da,
-				 char *buf)
+static ssize_t name_show(struct device *dev, struct device_attribute *da,
+			 char *buf)
 {
 	struct adt7x10_data *data = dev_get_drvdata(dev);
 
@@ -359,7 +358,7 @@ static SENSOR_DEVICE_ATTR(temp1_max_alarm, S_IRUGO, adt7x10_show_alarm,
 			  NULL, ADT7X10_STAT_T_HIGH);
 static SENSOR_DEVICE_ATTR(temp1_crit_alarm, S_IRUGO, adt7x10_show_alarm,
 			  NULL, ADT7X10_STAT_T_CRIT);
-static DEVICE_ATTR(name, S_IRUGO, adt7x10_show_name, NULL);
+static DEVICE_ATTR_RO(name);
 
 static struct attribute *adt7x10_attributes[] = {
 	&sensor_dev_attr_temp1_input.dev_attr.attr,

[toc] | [prev] | [next] | [standalone]

#1546722

FromGuenter Roeck <linux@roeck-us.net>
Date2016-12-23 03:40 +0100
Message-ID<sRnW1-Pj-9@gated-at.bofh.it>
In reply to#1546361
On 12/22/2016 04:04 AM, Julia Lawall wrote:
> Use DEVICE_ATTR_RO etc. for read only attributes etc.  This simplifies the
> source code, improves readbility, and reduces the chance of
> inconsistencies.  A previous version of this semantic patch transformed
> only cases where the show and store functions followed particular naming
> conventions.  This semantic patch additionally renames functions as
> needed.  It can, however, fail to transform some occurrences where a single
> function is used for multiple attributes.  In that case, the first case is
> transformed an the others are left as is.
>
> The complete semantic patch is as follows:
> (http://coccinelle.lip6.fr/)
>

Hi Julia,

I noticed that drivers/hwmon/gl518sm.c was not converted. Running your script
on it does not do anything. Any idea what might cause that ?

I noticed that I get a warning if I specify a debug file:

File "devattr-new.cocci", line 36, characters 5-6:
Warning 26: unused variable x.

but that seems to have no impact and is always seen. I think it is caused
by the following.

...

@script:ocaml@
x << d.x;	<====
show << o.show;
store << o.store;
@@

if (not(show = "NULL") && Hashtbl.mem taken show) ||
    (not(store = "NULL") && Hashtbl.mem taken store)
then Coccilib.include_match false
else (Hashtbl.add taken show (); Hashtbl.add taken store ())

...

How does one specify indentation preferences ? Also, how do I generate
an actual patch (instead of getting the output on screen) ?
Sorry for the maybe dumb questions ;-).

Thanks,
Guenter

[toc] | [prev] | [next] | [standalone]

#1546759

FromJulia Lawall <julia.lawall@lip6.fr>
Date2016-12-23 07:50 +0100
Message-ID<sRrPY-3nH-5@gated-at.bofh.it>
In reply to#1546722
> Hi Julia,
>
> I noticed that drivers/hwmon/gl518sm.c was not converted. Running your script
> on it does not do anything. Any idea what might cause that ?

I'll check.

> I noticed that I get a warning if I specify a debug file:
>
> File "devattr-new.cocci", line 36, characters 5-6:
> Warning 26: unused variable x.
>
> but that seems to have no impact and is always seen. I think it is caused
> by the following.

Yes, it has no impact.  You can change x << d.x; to _x << d.x;

>
> ...
>
> @script:ocaml@
> x << d.x;	<====
> show << o.show;
> store << o.store;
> @@
>
> if (not(show = "NULL") && Hashtbl.mem taken show) ||
>    (not(store = "NULL") && Hashtbl.mem taken store)
> then Coccilib.include_match false
> else (Hashtbl.add taken show (); Hashtbl.add taken store ())
>
> ...
>
> How does one specify indentation preferences ?

Actually, you are pretty much stuck with what Coccinelle provides you.
What do you want to adjust?  Currently, it shouldn't have any impact on
indentation if the new name is the same as the old one.  That function
could be generalized to allow length changes of up to a few characters, for
example, but I'm not sure that the result would always be satisfactory.
When I made the patch, I had to adjust about 5 cases by hand, where it was
sending all of the parameters past 80 characters, because the name of the
function was long.

> Also, how do I generate
> an actual patch (instead of getting the output on screen) ?
> Sorry for the maybe dumb questions ;-).

The patch is on standard output.  All other messages are on standard error.
You can also use the argument --in-place to change your code directly.

If you want to only work on the hwmon directory, but to get a patch that is
relative to the root directory, you can do:

--dir linux/drivers/hwmon --patch linux

That is, the argument to --patch is the directory that you want the patch
to be relative to.

julia

> Thanks,
> Guenter
>
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

[toc] | [prev] | [next] | [standalone]

#1546860

FromJulia Lawall <julia.lawall@lip6.fr>
Date2016-12-23 14:10 +0100
Message-ID<sRxLH-7js-9@gated-at.bofh.it>
In reply to#1546722
> Hi Julia,
>
> I noticed that drivers/hwmon/gl518sm.c was not converted. Running your script
> on it does not do anything. Any idea what might cause that ?

I believe that it is due to the following:

#define show(type, suffix, value)                                       \
static ssize_t show_##suffix(struct device *dev,                        \
                             struct device_attribute *attr, char *buf)  \
{                                                                       \
        struct gl518_data *data = gl518_update_device(dev);             \
        return sprintf(buf, "%d\n", type##_FROM_REG(data->value));      \
}


Coccinelle doesn't understand the relationship between this macro and the
show functions.

julia

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web