Re: [PATCH] iio: pac1921: remove unnecessary explicit casts

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Quoting Jonathan Cameron (2024-09-28 17:28:43)
> On Mon, 16 Sep 2024 14:00:05 +0200
> Matteo Martelli <matteomartelli3@xxxxxxxxx> wrote:
> 
> > Many explicit casts were introduced to address Wconversion and
> > Wsign-compare warnings. Remove them to improve readability.
> > 
> > Fixes: 371f778b83cd ("iio: adc: add support for pac1921")
> 
> No fixes tag on this one. Its not a bug, just a readability improvement.
> 
> > Signed-off-by: Matteo Martelli <matteomartelli3@xxxxxxxxx>
> There are a few cases in here where I think the cast is about ensuring
> we don't overflow in the maths rather than for warning suppression.
> 
> We all love 32 bit architectures after all ;)
> 
> Jonathan
> 
> > ---
> > Link: https://lore.kernel.org/linux-iio/1fa4ab12-0939-477d-bc92-306fd32e4fd9@stanley.mountain/
> > ---
> >  drivers/iio/adc/pac1921.c | 43 +++++++++++++++++++++----------------------
> >  1 file changed, 21 insertions(+), 22 deletions(-)
> > 
> > diff --git a/drivers/iio/adc/pac1921.c b/drivers/iio/adc/pac1921.c
> > index 4c2a1c07bc39..de69a1619a9e 100644
> > --- a/drivers/iio/adc/pac1921.c
> > +++ b/drivers/iio/adc/pac1921.c
> > @@ -240,8 +240,8 @@ static inline void pac1921_calc_scale(int dividend, int divisor, int *val,
> >  {
> >       s64 tmp;
> >  
> > -     tmp = div_s64(dividend * (s64)NANO, divisor);
> > -     *val = (int)div_s64_rem(tmp, NANO, val2);
> > +     tmp = div_s64(dividend * NANO, divisor);
> 
> For this one, NANO is an unsigned long and dividend just an int.
> Either the s64 cast is needed because dividend * NANO might go out of
> unsigned long range which might (I think) be 32 bit on a 32bit machine
> or it doesn't.  If it does, then you need the cast, if not you don't
> need to use div_s64 as it's not that large.

Oops! While removing the casts I was only thinking about the sign change
for this case, which would not be an issue being the dividend always
positive. However you are indeed right, this would overflow in 32-bit
architectures. Thanks for catching it, I am going to reintroduce this
cast and the other similar one in the next patch version.

...

Thanks,
Matteo Martelli





[Index of Archives]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Input]     [Linux Kernel]     [Linux SCSI]     [X.org]

  Powered by Linux