Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
References: <20221007145641.3307075-1-jjhiblot@traphandler.com> <20221007145641.3307075-2-jjhiblot@traphandler.com>
In-Reply-To: <20221007145641.3307075-2-jjhiblot@traphandler.com>
From:   Andy Shevchenko <andy.shevchenko@gmail.com>
Date:   Fri, 7 Oct 2022 19:18:44 +0300
Message-ID: <CAHp75VcX2w9mdzbKtfRpc-4zrz8tp33Q5uHyBfds8MJfyeVYvQ@mail.gmail.com>
Subject: Re: [PATCH v4 1/6] devres: provide devm_krealloc_array()
To:     Jean-Jacques Hiblot <jjhiblot@traphandler.com>,
        Bartosz Golaszewski <brgl@bgdev.pl>
Cc:     lee.jones@linaro.org, pavel@ucw.cz, robh+dt@kernel.org,
        sven.schwermer@disruptive-technologies.com,
        krzysztof.kozlowski+dt@linaro.org, johan+linaro@kernel.org,
        marijn.suijten@somainline.org, bjorn.andersson@linaro.org,
        jacek.anaszewski@gmail.com, linux-leds@vger.kernel.org,
        devicetree@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

+Cc: Bart who learnt realloc() specifics hard way

Thanks for doing this!

On Fri, Oct 7, 2022 at 6:13 PM Jean-Jacques Hiblot
<jjhiblot@traphandler.com> wrote:
>
> Implement the managed variant of krealloc_array().
> This internally uses devm_krealloc() and as such is usable with all memory
> allocated by devm_kmalloc() (or devres functions using it implicitly like
> devm_kmemdup(), devm_kstrdup() etc.)

Missed grammatical period at the end.

> Managed realloc'ed chunks can be manually released with devm_kfree().

...

>  {
>         return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
>  }

Missed blank line?

> +static inline void *devm_krealloc_array(struct device *dev,
> +                                       void *p,
> +                                       size_t new_n,
> +                                       size_t new_size,
> +                                       gfp_t flags)
> +{
> +       size_t bytes;
> +
> +       if (unlikely(check_mul_overflow(new_n, new_size, &bytes)))
> +               return NULL;

I'm not sure it is what we want, but I have read the man realloc and found this:

      ... reallocarray() fails safely in the case where the multiplication
      would overflow.  If such an overflow occurs, reallocarray() returns NULL,
      sets  errno  to  ENOMEM,  and leaves the original block of memory
      unchanged.

So, perhaps you can add that this behaviour mimics reallocarray()?

> +       return devm_krealloc(dev, p, bytes, flags);
> +}

...

All comments are minor, feel free to add
Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>

-- 
With Best Regards,
Andy Shevchenko