Re: [PATCH v7 5/7] iio: accel: kionix-kx022a: Refactor driver and add chip_info structure

From: Andy Shevchenko
Date: Fri Aug 11 2023 - 06:10:10 EST


On Fri, Aug 11, 2023 at 12:01:23PM +0200, Mehdi Djait wrote:
> On Fri, Aug 11, 2023 at 11:46 AM Andy Shevchenko
> <andriy.shevchenko@xxxxxxxxxxxxxxx> wrote:
> > On Wed, Aug 09, 2023 at 09:11:36PM +0200, Mehdi Djait wrote:

...

> > > + val = min_t(unsigned int, data->chip_info->fifo_length, val);
> >
> > min_t() is a beast. Please, use min() if no special requirement for
> > min_t() here, otherwise explain why.
>
> No actual reason, you suggested min_t or min for a previous version

After the minmax series (see discussion in LKML) I am pretty sure min_t()
shouldn't be used at all or with very minimum cases where people know what
they are doing.

...

> > > + data->fifo_buffer = kmalloc(data->chip_info->fifo_length *
> > > + KX022A_FIFO_SAMPLES_SIZE_BYTES, GFP_KERNEL);
> >
> > kmalloc_array()
>
> Should I send another version for this ?

Sure.

> The usage of kmalloc is quite straightforward and easy to understand here.

Yes, and prone to errors. Multiplication theoretically may overflow and
your code doesn't aware of that, kmalloc_array() does it.

--
With Best Regards,
Andy Shevchenko