Re: [PATCH v1 3/8] iio: core: Switch to krealloc_array()

[Nuno Sá]

On Thu, 2023-07-20 at 23:53 +0300, Andy Shevchenko wrote:
> Let the krealloc_array() copy the original data and
> check for a multiplication overflow.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> ---
>  drivers/iio/industrialio-core.c | 20 +++++++-------------
>  1 file changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/drivers/iio/industrialio-core.c b/drivers/iio/industrialio-core.c
> index 90e59223b178..be154879983e 100644
> --- a/drivers/iio/industrialio-core.c
> +++ b/drivers/iio/industrialio-core.c
> @@ -1465,7 +1465,7 @@ int iio_device_register_sysfs_group(struct iio_dev
> *indio_dev,
>         const struct attribute_group **new, **old = iio_dev_opaque->groups;
>         unsigned int cnt = iio_dev_opaque->groupcounter;
>  
> -       new = krealloc(old, sizeof(*new) * (cnt + 2), GFP_KERNEL);
> +       new = krealloc_array(old, cnt + 2, sizeof(*new), GFP_KERNEL);
>         if (!new)
>                 return -ENOMEM;
>  
> @@ -1483,13 +1483,13 @@ static int iio_device_register_sysfs(struct iio_dev
> *indio_dev)
>  {
>         struct iio_dev_opaque *iio_dev_opaque = to_iio_dev_opaque(indio_dev);
>         int i, ret = 0, attrcount, attrn, attrcount_orig = 0;
> +       struct attribute **attrs, **attr, *clk = NULL;
>         struct iio_dev_attr *p;
> -       struct attribute **attr, *clk = NULL;
>  
>         /* First count elements in any existing group */
> -       if (indio_dev->info->attrs) {
> -               attr = indio_dev->info->attrs->attrs;
> -               while (*attr++ != NULL)
> +       attrs = indio_dev->info->attrs ? indio_dev->info->attrs->attrs : NULL;
> +       if (attrs) {
> +               for (attr = attrs; *attr; attr++)
>                         attrcount_orig++;

not really related with the change... maybe just mention it in the commit?

>         }
>         attrcount = attrcount_orig;
> @@ -1521,20 +1521,14 @@ static int iio_device_register_sysfs(struct iio_dev
> *indio_dev)
>         if (clk)
>                 attrcount++;
>  
> +       /* Copy across original attributes, and point to original binary
> attributes */
>         iio_dev_opaque->chan_attr_group.attrs =
> -               kcalloc(attrcount + 1,
> -                       sizeof(iio_dev_opaque->chan_attr_group.attrs[0]),
> -                       GFP_KERNEL);
> +               krealloc_array(attrs, attrcount + 1, sizeof(*attrs),
> GFP_KERNEL);
>         if (iio_dev_opaque->chan_attr_group.attrs == NULL) {

since you're here and you also already did some style cleanups above, maybe
change it to 'if (!iio_dev_opaque->chan_attr_group.attrs)'?

- Nuno Sá