[PATCH v4 1/6] devres: provide devm_krealloc_array()

Jean-Jacques Hiblot posted 6 patches 3 years, 6 months ago
There is a newer version of this series
[PATCH v4 1/6] devres: provide devm_krealloc_array()
Posted by Jean-Jacques Hiblot 3 years, 6 months ago
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.)

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

Signed-off-by: Jean-Jacques Hiblot <jjhiblot@traphandler.com>
---
 include/linux/device.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/linux/device.h b/include/linux/device.h
index 424b55df0272..3b472df6c6cd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -223,6 +223,19 @@ static inline void *devm_kcalloc(struct device *dev,
 {
 	return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
 }
+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;
+
+	return devm_krealloc(dev, p, bytes, flags);
+}
 void devm_kfree(struct device *dev, const void *p);
 char *devm_kstrdup(struct device *dev, const char *s, gfp_t gfp) __malloc;
 const char *devm_kstrdup_const(struct device *dev, const char *s, gfp_t gfp);
-- 
2.25.1
Re: [PATCH v4 1/6] devres: provide devm_krealloc_array()
Posted by Andy Shevchenko 3 years, 6 months ago
+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
Re: [PATCH v4 1/6] devres: provide devm_krealloc_array()
Posted by Jean-Jacques Hiblot 3 years, 5 months ago
On 07/10/2022 18:18, Andy Shevchenko wrote:
>>   {
>>          return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO);
>>   }
> Missed blank line?

There is no blank line after the definitions of devm_kzalloc(), 
devm_kmalloc_array() and devm_kcalloc() defined just above.

>
>> +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()?

except for the errno part, that is what is does. I don't think we should 
use ERR_PTR in this case as the other allocation functions don't use it.

>
>> +       return devm_krealloc(dev, p, bytes, flags);
>> +}
> ...
>
> All comments are minor, feel free to add
> Reviewed-by: Andy Shevchenko <andy.shevchenko@gmail.com>
>