2022-10-07 15:00:35

by Jean-Jacques Hiblot

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

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 <[email protected]>
---
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


2022-10-07 16:57:37

by Andy Shevchenko

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] devres: provide devm_krealloc_array()

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

Thanks for doing this!

On Fri, Oct 7, 2022 at 6:13 PM Jean-Jacques Hiblot
<[email protected]> 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 <[email protected]>

--
With Best Regards,
Andy Shevchenko

2022-10-14 09:43:25

by Jean-Jacques Hiblot

[permalink] [raw]
Subject: Re: [PATCH v4 1/6] devres: provide devm_krealloc_array()


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 <[email protected]>
>