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