Skip to contentSkip to conversation

Staging: iio: tsl2x7x_core: Remove extra parentheses from DIV_ROUND_UP

PATCH Superseded Shivani Bhardwaj 2 participants 3 messages Oct 16, 2015

Subsystems (3)

v1 of 2 Superseded Latest is v2 →

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Staging: iio: tsl2x7x_core: Remove extra parentheses from DIV_ROUND_UP

Remove parentheses surrounding the first argument of the macro DIV_ROUND_UP as they are not required.
Semantic patch used:

@@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

---
 drivers/staging/iio/light/tsl2x7x_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/iio/light/tsl2x7x_core.c +4 -4
index 010e607..bf9627e 100644
@@ -418,9 +418,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	if (p->ratio == 0) {
 		lux = 0;
 	} else {
-		ch0lux = DIV_ROUND_UP((ch0 * p->ch0),
+		ch0lux = DIV_ROUND_UP(ch0 * p->ch0,
 			tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
-		ch1lux = DIV_ROUND_UP((ch1 * p->ch1),
+		ch1lux = DIV_ROUND_UP(ch1 * p->ch1,
 			tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
 		lux = ch0lux - ch1lux;
 	}
@@ -1057,7 +1057,7 @@ static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
 	z = y * TSL2X7X_MIN_ITIME;
 
 	filter_delay =
-		DIV_ROUND_UP(((result.integer * 1000) + result.fract), z);
+		DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
 
 	chip->tsl2x7x_settings.persistence &= 0xF0;
 	chip->tsl2x7x_settings.persistence |= (filter_delay & 0x0F);
@@ -1103,7 +1103,7 @@ static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
 	z = y * TSL2X7X_MIN_ITIME;
 
 	filter_delay =
-		DIV_ROUND_UP(((result.integer * 1000) + result.fract), z);
+		DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
 
 	chip->tsl2x7x_settings.persistence &= 0x0F;
 	chip->tsl2x7x_settings.persistence |= ((filter_delay << 4) & 0xF0);
Signature · 1 line
2.1.0
#1 lore raw
Quoted 10 lines · Shivani Bhardwaj

Remove parentheses surrounding the first argument of the macro DIV_ROUND_UP as they are not required.
Semantic patch used:

@@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

This is just a comment; no need to change anything.

In Coccinelle, there is a notion of isomorphism, where it will take the term that you write and convert it to other terms that are "pretty much" the same. One of these isomorphisms is to remove parentheses. Thus, if you run spatch --parse-cocci this_sp.cocci, you will see that it matches both the case where the parentheses are there and the case where they are not there, and transforms both of them according to your + code. In your case, this will amount to saying

- DIV_ROUND_UP(n, d)
+ DIV_ROUND_UP(n, d)

which looks completely useless. But it doesn't actually necessarily do nothing at all. Actually, it will remove DIV_ROUND_UP(n, d), and then use its own strategy for pretty printing DIV_ROUND_UP(n, d) again. Thus the output might have some newline that the original code didn't have. (This might be a good thing :).

To avoid it removing code and putting the same code back, you could write your rule as:

@@
@@

DIV_ROUND_UP(
- (
n
- )
,d)

Now it knows that the () are important to the rule, because they are annotated with - and the rest is not. It will only transform cases that actually have the parentheses in this case.

julia

Quoted 50 lines
---
 drivers/staging/iio/light/tsl2x7x_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/iio/light/tsl2x7x_core.c +4 -5
index 010e607..bf9627e 100644
@@ -418,9 +418,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
 	if (p->ratio == 0) {
 		lux = 0;
 	} else {
-		ch0lux = DIV_ROUND_UP((ch0 * p->ch0),
+		ch0lux = DIV_ROUND_UP(ch0 * p->ch0,
 			tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
-		ch1lux = DIV_ROUND_UP((ch1 * p->ch1),
+		ch1lux = DIV_ROUND_UP(ch1 * p->ch1,
 			tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
 		lux = ch0lux - ch1lux;
 	}
@@ -1057,7 +1057,7 @@ static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
 	z = y * TSL2X7X_MIN_ITIME;
 
 	filter_delay =
-		DIV_ROUND_UP(((result.integer * 1000) + result.fract), z);
+		DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
 
 	chip->tsl2x7x_settings.persistence &= 0xF0;
 	chip->tsl2x7x_settings.persistence |= (filter_delay & 0x0F);
@@ -1103,7 +1103,7 @@ static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
 	z = y * TSL2X7X_MIN_ITIME;
 
 	filter_delay =
-		DIV_ROUND_UP(((result.integer * 1000) + result.fract), z);
+		DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
 
 	chip->tsl2x7x_settings.persistence &= 0x0F;
 	chip->tsl2x7x_settings.persistence |= ((filter_delay << 4) & 0xF0);
--

2.1.0

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151016141011.GA14411%40ubuntu.
For more options, visit https://groups.google.com/d/optout.

#2 lore raw

Shivani

Quoted 44 lines · Julia Lawall
Quoted 10 lines · Shivani Bhardwaj

Remove parentheses surrounding the first argument of the macro DIV_ROUND_UP as they are not required.
Semantic patch used:

@@
expression n,d;
@@

- DIV_ROUND_UP((n),d)
+ DIV_ROUND_UP(n,d)

This is just a comment; no need to change anything.

In Coccinelle, there is a notion of isomorphism, where it will take the term that you write and convert it to other terms that are "pretty much" the same. One of these isomorphisms is to remove parentheses. Thus, if you run spatch --parse-cocci this_sp.cocci, you will see that it matches both the case where the parentheses are there and the case where they are not there, and transforms both of them according to your + code. In your case, this will amount to saying

- DIV_ROUND_UP(n, d)
+ DIV_ROUND_UP(n, d)

which looks completely useless. But it doesn't actually necessarily do nothing at all. Actually, it will remove DIV_ROUND_UP(n, d), and then use its own strategy for pretty printing DIV_ROUND_UP(n, d) again. Thus the output might have some newline that the original code didn't have. (This might be a good thing :).

To avoid it removing code and putting the same code back, you could write your rule as:

@@
@@

DIV_ROUND_UP(
- (
n
- )
,d)

Quoted 7 lines

Now it knows that the () are important to the rule, because they are annotated with - and the rest is not. It will only transform cases that actually have the parentheses in this case.

julia

Thanks, Julia! :) I'm updating the patch.

Quoted 50 lines
Quoted 50 lines
---
 drivers/staging/iio/light/tsl2x7x_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
drivers/staging/iio/light/tsl2x7x_core.c +4 -5
index 010e607..bf9627e 100644
@@ -418,9 +418,9 @@ static int tsl2x7x_get_lux(struct iio_dev *indio_dev)
      if (p->ratio == 0) {
              lux = 0;
      } else {
-             ch0lux = DIV_ROUND_UP((ch0 * p->ch0),
+             ch0lux = DIV_ROUND_UP(ch0 * p->ch0,
                      tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
-             ch1lux = DIV_ROUND_UP((ch1 * p->ch1),
+             ch1lux = DIV_ROUND_UP(ch1 * p->ch1,
                      tsl2X7X_als_gainadj[chip->tsl2x7x_settings.als_gain]);
              lux = ch0lux - ch1lux;
      }
@@ -1057,7 +1057,7 @@ static ssize_t tsl2x7x_als_persistence_store(struct device *dev,
      z = y * TSL2X7X_MIN_ITIME;
 
      filter_delay =
-             DIV_ROUND_UP(((result.integer * 1000) + result.fract), z);
+             DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
 
      chip->tsl2x7x_settings.persistence &= 0xF0;
      chip->tsl2x7x_settings.persistence |= (filter_delay & 0x0F);
@@ -1103,7 +1103,7 @@ static ssize_t tsl2x7x_prox_persistence_store(struct device *dev,
      z = y * TSL2X7X_MIN_ITIME;
 
      filter_delay =
-             DIV_ROUND_UP(((result.integer * 1000) + result.fract), z);
+             DIV_ROUND_UP((result.integer * 1000) + result.fract, z);
 
      chip->tsl2x7x_settings.persistence &= 0x0F;
      chip->tsl2x7x_settings.persistence |= ((filter_delay << 4) & 0xF0);
--

2.1.0

--
You received this message because you are subscribed to the Google Groups "outreachy-kernel" group.
To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected].
To view this discussion on the web visit https://groups.google.com/d/msgid/outreachy-kernel/20151016141011.GA14411%40ubuntu.
For more options, visit https://groups.google.com/d/optout.