lkml.org 
[lkml]   [2022]   [Nov]   [7]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH] staging: iio: meter: use min() for comparison and assignment
On Mon, Nov 07, 2022 at 09:40:00AM +0530, Deepak R Varma wrote:
> Simplify code by using recommended min helper macro for logical
> evaluation and value assignment. This issue is identified by
> coccicheck using the minmax.cocci file.
>
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> drivers/staging/iio/meter/ade7854-i2c.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/staging/iio/meter/ade7854-i2c.c b/drivers/staging/iio/meter/ade7854-i2c.c
> index a9a06e8dda51..a6ce7b24cc8f 100644
> --- a/drivers/staging/iio/meter/ade7854-i2c.c
> +++ b/drivers/staging/iio/meter/ade7854-i2c.c
> @@ -61,7 +61,7 @@ static int ade7854_i2c_write_reg(struct device *dev,
> unlock:
> mutex_unlock(&st->buf_lock);
>
> - return ret < 0 ? ret : 0;
> + return min(ret, 0);

The original code is better.

If it's a failure return the error code. If it's not return zero.

You can only compare apples to apples. min() makes sense if you're
talking about two lengths. But here if ret is negative that's an error
code. If it's positive that's the number of bytes. If the error
code is less than the number of bytes then return that? What??? It
makes no sense.

In terms of run time, this patch is fine but in terms of reading the
code using min() makes it less readable.

regards,
dan carpenter

\
 
 \ /
  Last update: 2022-11-07 14:09    [W:0.220 / U:0.020 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site