drivers: iio: Replace ternary operator with min macro

PATCH In review simran singhal 5 participants 12 messages Mar 29, 2017

Subsystems (3)

IIO ST LSM6DSx IMU IIO driver HTS221 temperature-humidity IIO driver

lore

Share & tools

lore

simran singhal

author

2017-03-29 12:33 UTC #0 lore raw

cover drivers: iio: Replace ternary operator with min macro

Use macro min() to get the minimum of two values for brevity and readability.

simran singhal (4):
iio: common: st_sensors: Replace ternary operator with min macro
iio: imu: st_lsm6dsx: Replace ternary operator with min macro
iio: imu: st_lsm6dsx: Replace ternary operator with min macro
iio: light: si1145: Replace ternary operator with min macro

 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 drivers/iio/humidity/hts221_core.c             | 2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
 drivers/iio/light/si1145.c                     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Signature · 1 line

2.7.4

simran singhal

author

2017-03-29 12:33 UTC #1 lore raw

PATCH 1/4 iio: common: st_sensors: Replace ternary operator with min macro

Use macro min() to get the minimum of two values for brevity and readability.

Signed-off-by simran singhal <[email protected]>

---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/common/st_sensors/st_sensors_i2c.c +1 -1

index c83df4d..7a68fdd 100644
@@ -39,7 +39,7 @@ static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb,
 	*res_byte = err & 0xff;
 
 st_accel_i2c_read_byte_error:
-	return err < 0 ? err : 0;
+	return min(err, 0);
 }
 
 static int st_sensors_i2c_read_multiple_byte(

Signature · 1 line

2.7.4

Daniel Baluta

2017-03-29 16:16 UTC #2 lore raw

Re: [Outreachy kernel] [PATCH 1/4] iio: common: st_sensors: Replace ternary operator with min macro

Quoted 13 lines · simran singhal

Use macro min() to get the minimum of two values for brevity and readability.
Signed-off-by simran singhal <[email protected]>
---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/common/st_sensors/st_sensors_i2c.c +0 -0
index c83df4d..7a68fdd 100644

drivers/iio/common/st_sensors/st_sensors_i2c.c static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb, · line 43

Quoted 6 lines

drivers/iio/common/st_sensors/st_sensors_i2c.c +1 -1

@@ -39,7 +39,7 @@ static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb,
        *res_byte = err & 0xff;
 
 st_accel_i2c_read_byte_error:
-       return err < 0 ? err : 0;
+       return min(err, 0);
 }

Appreciate the brevity but this certainly doesn't make code easier to read.

In linux kernel err < 0 signifies an error and be replacing comparison < 0 with min() we some hide the meaning of this.

thanks,
Daniel.

SIMRAN SINGHAL

author

2017-03-29 17:24 UTC #3 lore raw

Re: [Outreachy kernel] [PATCH 1/4] iio: common: st_sensors: Replace ternary operator with min macro

Quoted 25 lines · Daniel Baluta

Quoted 13 lines · simran singhal
Use macro min() to get the minimum of two values for brevity and readability.
Signed-off-by simran singhal <[email protected]>
---
 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/common/st_sensors/st_sensors_i2c.c +0 -0
index c83df4d..7a68fdd 100644
drivers/iio/common/st_sensors/st_sensors_i2c.c static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb, · line 43
Quoted 6 lines
drivers/iio/common/st_sensors/st_sensors_i2c.c +1 -1
@@ -39,7 +39,7 @@ static int st_sensors_i2c_read_byte(struct st_sensor_transfer_buffer *tb,
        *res_byte = err & 0xff;
 
 st_accel_i2c_read_byte_error:
-       return err < 0 ? err : 0;
+       return min(err, 0);
 }
Appreciate the brevity but this certainly doesn't make code easier to read.

Quoted 3 lines

In linux kernel err < 0 signifies an error and be replacing comparison < 0 with min() we some hide the meaning of this.

Yes, you are right keeping the previous one will be more meaningful.

Thanks.

Quoted 2 lines

thanks,
Daniel.

simran singhal

author

2017-03-29 12:33 UTC #4 lore raw

PATCH 2/4 iio: imu: st_lsm6dsx: Replace ternary operator with min macro

In reply to simran singhal

Use macro min() to get the minimum of two values for brevity and readability.

Signed-off-by simran singhal <[email protected]>

---
 drivers/iio/humidity/hts221_core.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/humidity/hts221_core.c +1 -1

index 3f3ef4a1..f98240a 100644
@@ -206,7 +206,7 @@ int hts221_config_drdy(struct hts221_hw *hw, bool enable)
 	err = hts221_write_with_mask(hw, HTS221_REG_CNTRL3_ADDR,
 				     HTS221_DRDY_MASK, val);
 
-	return err < 0 ? err : 0;
+	return min(err, 0);
 }
 
 static int hts221_update_odr(struct hts221_hw *hw, u8 odr)

Signature · 1 line

2.7.4

simran singhal

author

2017-03-29 12:33 UTC #5 lore raw

PATCH 3/4 iio: imu: st_lsm6dsx: Replace ternary operator with min macro

In reply to simran singhal

Use macro min() to get the minimum of two values for brevity and readability.

Signed-off-by simran singhal <[email protected]>

---
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c +1 -1

index e959825..e11653d 100644
@@ -203,7 +203,7 @@ int st_lsm6dsx_update_watermark(struct st_lsm6dsx_sensor *sensor, u16 watermark)
 out:
 	mutex_unlock(&hw->lock);
 
-	return err < 0 ? err : 0;
+	return min(err, 0);
 }
 
 /**

Signature · 1 line

2.7.4

simran singhal

author

2017-03-29 12:33 UTC #6 lore raw

PATCH 4/4 iio: light: si1145: Replace ternary operator with min macro

In reply to simran singhal

Use macro min() to get the minimum of two values for brevity and readability.

Signed-off-by simran singhal <[email protected]>

---
 drivers/iio/light/si1145.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

drivers/iio/light/si1145.c +1 -1

index 096034c..e7ad6fb 100644
@@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask)
 	data->scan_mask = scan_mask;
 	ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
 
-	return ret < 0 ? ret : 0;
+	return min(ret, 0);
 }
 
 static int si1145_measure(struct iio_dev *indio_dev,

Signature · 1 line

2.7.4

Julia Lawall

2017-03-29 12:40 UTC #7 lore raw

Re: [Outreachy kernel] [PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

Quoted 13 lines · simran singhal

Use macro min() to get the minimum of two values for brevity and readability.
Signed-off-by simran singhal <[email protected]>
---
 drivers/iio/light/si1145.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/light/si1145.c +0 -0
index 096034c..e7ad6fb 100644

drivers/iio/light/si1145.c static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask) · line 560

Quoted 5 lines

drivers/iio/light/si1145.c +1 -1

@@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask)
 	data->scan_mask = scan_mask;
 	ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
 
-	return ret < 0 ? ret : 0;
+	return min(ret, 0);

A similar change involving max was already rejected. ret < 0 is a standard way of detecting an error, so perhaps leaving that explicitly present will be preferred.

julia

Quoted 13 lines

drivers/iio/light/si1145.c +0 -1
 }
 
 static int si1145_measure(struct iio_dev *indio_dev,
--
2.7.4
--
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/1490790791-5694-5-git-send-email-singhalsimran0%40gmail.com.
For more options, visit https://groups.google.com/d/optout.

Lars-Peter Clausen

2017-03-29 12:53 UTC #8 lore raw

Re: [Outreachy kernel] [PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

Quoted 20 lines · Julia Lawall

Quoted 16 lines · simran singhal
Use macro min() to get the minimum of two values for brevity and readability.
Signed-off-by simran singhal <[email protected]>
---
 drivers/iio/light/si1145.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/light/si1145.c +0 -0
index 096034c..e7ad6fb 100644
@@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask)
 	data->scan_mask = scan_mask;
 	ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
 

Quoted 6 lines

drivers/iio/light/si1145.c static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask) · line 560
Quoted 2 lines
drivers/iio/light/si1145.c +1 -1
-	return ret < 0 ? ret : 0;
+	return min(ret, 0);
A similar change involving max was already rejected. ret < 0 is a standard way of detecting an error, so perhaps leaving that explicitly present will be preferred.

I think a more sensible thing to do here is to check whether ret/err can actually take any positive values and if not, replace the whole thing with just 'return ret;' or 'return some_fn()'. I'd expect that his can be done in most of the cases.

SIMRAN SINGHAL

author

2017-03-29 14:52 UTC #9 lore raw

Re: [Outreachy kernel] [PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

Quoted 33 lines · Lars-Peter Clausen

Quoted 20 lines · Julia Lawall
Quoted 16 lines · simran singhal
Use macro min() to get the minimum of two values for brevity and readability.
Signed-off-by simran singhal <[email protected]>
---
 drivers/iio/light/si1145.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/light/si1145.c +0 -0
index 096034c..e7ad6fb 100644
@@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask)
     data->scan_mask = scan_mask;
     ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
 
Quoted 6 lines
drivers/iio/light/si1145.c static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask) · line 560
Quoted 2 lines
drivers/iio/light/si1145.c +1 -1
-    return ret < 0 ? ret : 0;
+    return min(ret, 0);
A similar change involving max was already rejected. ret < 0 is a standard way of detecting an error, so perhaps leaving that explicitly present will be preferred.
I think a more sensible thing to do here is to check whether ret/err can actually take any positive values and if not, replace the whole thing with just 'return ret;' or 'return some_fn()'. I'd expect that his can be done in most of the cases.

SIMRAN SINGHAL

author

2017-03-29 14:53 UTC #10 lore raw

Re: [Outreachy kernel] [PATCH 4/4] iio: light: si1145: Replace ternary operator with min macro

Quoted 29 lines · SIMRAN SINGHAL

Quoted 28 lines · Lars-Peter Clausen
Quoted 26 lines · Julia Lawall
Quoted 13 lines · simran singhal
Use macro min() to get the minimum of two values for brevity and readability.
Signed-off-by simran singhal <[email protected]>
---
 drivers/iio/light/si1145.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/iio/light/si1145.c +0 -0
index 096034c..e7ad6fb 100644
drivers/iio/light/si1145.c static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask) · line 560
Quoted 5 lines
drivers/iio/light/si1145.c +1 -1
@@ -557,7 +557,7 @@ static int si1145_set_chlist(struct iio_dev *indio_dev, unsigned long scan_mask)
     data->scan_mask = scan_mask;
     ret = si1145_param_set(data, SI1145_PARAM_CHLIST, reg);
 
-    return ret < 0 ? ret : 0;
+    return min(ret, 0);
A similar change involving max was already rejected. ret < 0 is a standard way of detecting an error, so perhaps leaving that explicitly present will be preferred.

Quoted 5 lines

Quoted 5 lines
I think a more sensible thing to do here is to check whether ret/err can actually take any positive values and if not, replace the whole thing with just 'return ret;' or 'return some_fn()'. I'd expect that his can be done in most of the cases.

Lars, I will check it and resend it.

Alison Schofield

2017-03-29 18:56 UTC #11 lore raw

In reply to simran singhal

Quoted 7 lines · simran singhal

Use macro min() to get the minimum of two values for brevity and readability.
simran singhal (4):
iio: common: st_sensors: Replace ternary operator with min macro
iio: imu: st_lsm6dsx: Replace ternary operator with min macro
iio: imu: st_lsm6dsx: Replace ternary operator with min macro
iio: light: si1145: Replace ternary operator with min macro

Hi Simran,

I'm guessing these were found by Coccinelle. Please reference that in the changelog.

FYI: 'Mileage will vary' on cleanup patches sent upstream. Their acceptance varies by subsystem and the change. Consider sending one such patch and await feedback. Another option is to send one and be direct. Ask below the line --- if such cleanup patches are welcome. Sometimes they create too much churn and are only taken as part of a larger update to the particular driver.

Just my thoughts for future patches as Outreachy window closes :( alisons

Quoted 16 lines

 drivers/iio/common/st_sensors/st_sensors_i2c.c | 2 +-
 drivers/iio/humidity/hts221_core.c             | 2 +-
 drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
 drivers/iio/light/si1145.c                     | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

Signature · 8 lines

2.7.4

-- 
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/1490790791-5694-1-git-send-email-singhalsimran0%40gmail.com.
For more options, visit https://groups.google.com/d/optout.