2017-03-29 12:33:19

by Simran Singhal

[permalink] [raw]
Subject: [PATCH 0/4] 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(-)

--
2.7.4


2017-03-29 12:33:34

by Simran Singhal

[permalink] [raw]
Subject: [PATCH 4/4] iio: light: si1145: 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/light/si1145.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
index 096034c..e7ad6fb 100644
--- a/drivers/iio/light/si1145.c
+++ b/drivers/iio/light/si1145.c
@@ -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,
--
2.7.4

2017-03-29 12:33:23

by Simran Singhal

[permalink] [raw]
Subject: [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(-)

diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
index c83df4d..7a68fdd 100644
--- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
+++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
@@ -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(
--
2.7.4

2017-03-29 12:33:40

by Simran Singhal

[permalink] [raw]
Subject: [PATCH 3/4] iio: imu: st_lsm6dsx: 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/imu/st_lsm6dsx/st_lsm6dsx_buffer.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
index e959825..e11653d 100644
--- a/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
+++ b/drivers/iio/imu/st_lsm6dsx/st_lsm6dsx_buffer.c
@@ -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);
}

/**
--
2.7.4

2017-03-29 12:33:26

by Simran Singhal

[permalink] [raw]
Subject: [PATCH 2/4] iio: imu: st_lsm6dsx: 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/humidity/hts221_core.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/iio/humidity/hts221_core.c b/drivers/iio/humidity/hts221_core.c
index 3f3ef4a1..f98240a 100644
--- a/drivers/iio/humidity/hts221_core.c
+++ b/drivers/iio/humidity/hts221_core.c
@@ -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)
--
2.7.4

2017-03-29 12:41:02

by Julia Lawall

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



On Wed, 29 Mar 2017, simran singhal wrote:

> 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(-)
>
> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
> index 096034c..e7ad6fb 100644
> --- a/drivers/iio/light/si1145.c
> +++ b/drivers/iio/light/si1145.c
> @@ -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

> }
>
> 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.
>

2017-03-29 12:53:19

by Lars-Peter Clausen

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

On 03/29/2017 02:40 PM, Julia Lawall wrote:
>
>
> On Wed, 29 Mar 2017, simran singhal wrote:
>
>> 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(-)
>>
>> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
>> index 096034c..e7ad6fb 100644
>> --- a/drivers/iio/light/si1145.c
>> +++ b/drivers/iio/light/si1145.c
>> @@ -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.

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.

2017-03-29 14:52:20

by Simran Singhal

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

On Wed, Mar 29, 2017 at 6:23 PM, Lars-Peter Clausen <[email protected]> wrote:
> On 03/29/2017 02:40 PM, Julia Lawall wrote:
>>
>>
>> On Wed, 29 Mar 2017, simran singhal wrote:
>>
>>> 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(-)
>>>
>>> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
>>> index 096034c..e7ad6fb 100644
>>> --- a/drivers/iio/light/si1145.c
>>> +++ b/drivers/iio/light/si1145.c
>>> @@ -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.
>
> 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.
>

2017-03-29 14:53:32

by Simran Singhal

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

On Wed, Mar 29, 2017 at 8:22 PM, SIMRAN SINGHAL
<[email protected]> wrote:
> On Wed, Mar 29, 2017 at 6:23 PM, Lars-Peter Clausen <[email protected]> wrote:
>> On 03/29/2017 02:40 PM, Julia Lawall wrote:
>>>
>>>
>>> On Wed, 29 Mar 2017, simran singhal wrote:
>>>
>>>> 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(-)
>>>>
>>>> diff --git a/drivers/iio/light/si1145.c b/drivers/iio/light/si1145.c
>>>> index 096034c..e7ad6fb 100644
>>>> --- a/drivers/iio/light/si1145.c
>>>> +++ b/drivers/iio/light/si1145.c
>>>> @@ -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.
>>
>> 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.

2017-03-29 16:16:17

by Daniel Baluta

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

On Wed, Mar 29, 2017 at 3:33 PM, simran singhal
<[email protected]> wrote:
> 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(-)
>
> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> index c83df4d..7a68fdd 100644
> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
> @@ -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.

2017-03-29 17:24:22

by Simran Singhal

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

On Wed, Mar 29, 2017 at 9:46 PM, Daniel Baluta <[email protected]> wrote:
> On Wed, Mar 29, 2017 at 3:33 PM, simran singhal
> <[email protected]> wrote:
>> 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(-)
>>
>> diff --git a/drivers/iio/common/st_sensors/st_sensors_i2c.c b/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> index c83df4d..7a68fdd 100644
>> --- a/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> +++ b/drivers/iio/common/st_sensors/st_sensors_i2c.c
>> @@ -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.
>
Yes, you are right keeping the previous one will be more
meaningful.

Thanks.

> thanks,
> Daniel.

2017-03-29 18:57:29

by Alison Schofield

[permalink] [raw]
Subject: Re: [Outreachy kernel] [PATCH 0/4] drivers: iio: Replace ternary operator with min macro

On Wed, Mar 29, 2017 at 06:03:07PM +0530, simran singhal wrote:
> 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

>
> 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(-)
>
> --
> 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.