| Message ID | 20230509094942.396150-1-james.clark@arm.com |
|---|---|
| Headers | show |
| Series | devres: Provide krealloc_array | expand |
On Tue, May 09, 2023 at 10:49:38AM +0100, James Clark wrote: > There is no krealloc_array equivalent in devres. Users would have to > do their own multiplication overflow check so provide one. > > Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > Signed-off-by: James Clark <james.clark@arm.com> > --- > Documentation/driver-api/driver-model/devres.rst | 1 + > include/linux/device.h | 11 +++++++++++ > 2 files changed, 12 insertions(+) > > diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst > index 4249eb4239e0..8be086b3f829 100644 > --- a/Documentation/driver-api/driver-model/devres.rst > +++ b/Documentation/driver-api/driver-model/devres.rst > @@ -364,6 +364,7 @@ MEM > devm_kmalloc_array() > devm_kmemdup() > devm_krealloc() > + devm_krealloc_array() > devm_kstrdup() > devm_kstrdup_const() > devm_kvasprintf() > diff --git a/include/linux/device.h b/include/linux/device.h > index 472dd24d4823..58f4f5948edb 100644 > --- a/include/linux/device.h > +++ b/include/linux/device.h > @@ -223,6 +223,17 @@ static inline void *devm_kcalloc(struct device *dev, > { > return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO); > } > +static inline __realloc_size(3, 4) void * __must_check Shouldn't you have a blank line before this one? > +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); > +} I dislike how we have to keep copying the "real" functions (i.e. krealloc_array) into something like this, but I can't think of a better way to do it. thanks, greg k-h
On 13/05/2023 12:04, Greg KH wrote: > On Tue, May 09, 2023 at 10:49:38AM +0100, James Clark wrote: >> There is no krealloc_array equivalent in devres. Users would have to >> do their own multiplication overflow check so provide one. >> >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> >> Signed-off-by: James Clark <james.clark@arm.com> >> --- >> Documentation/driver-api/driver-model/devres.rst | 1 + >> include/linux/device.h | 11 +++++++++++ >> 2 files changed, 12 insertions(+) >> >> diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst >> index 4249eb4239e0..8be086b3f829 100644 >> --- a/Documentation/driver-api/driver-model/devres.rst >> +++ b/Documentation/driver-api/driver-model/devres.rst >> @@ -364,6 +364,7 @@ MEM >> devm_kmalloc_array() >> devm_kmemdup() >> devm_krealloc() >> + devm_krealloc_array() >> devm_kstrdup() >> devm_kstrdup_const() >> devm_kvasprintf() >> diff --git a/include/linux/device.h b/include/linux/device.h >> index 472dd24d4823..58f4f5948edb 100644 >> --- a/include/linux/device.h >> +++ b/include/linux/device.h >> @@ -223,6 +223,17 @@ static inline void *devm_kcalloc(struct device *dev, >> { >> return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO); >> } >> +static inline __realloc_size(3, 4) void * __must_check > > Shouldn't you have a blank line before this one? I was going for consistency with the rest of this section which doesn't have newlines between the functions for some reason. I can add one and resubmit but it might look a bit out of place? > >> +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); >> +} > > I dislike how we have to keep copying the "real" functions (i.e. > krealloc_array) into something like this, but I can't think of a better > way to do it. > Maybe something could be done with some macro magic, but it would probably end up being worse than just copying them and would affect the real ones as well. So yeah I can't think of any easy gains either. Thanks James > thanks, > > greg k-h
On Mon, May 15, 2023 at 08:55:33AM +0100, James Clark wrote: > > > On 13/05/2023 12:04, Greg KH wrote: > > On Tue, May 09, 2023 at 10:49:38AM +0100, James Clark wrote: > >> There is no krealloc_array equivalent in devres. Users would have to > >> do their own multiplication overflow check so provide one. > >> > >> Reviewed-by: Jonathan Cameron <Jonathan.Cameron@huawei.com> > >> Signed-off-by: James Clark <james.clark@arm.com> > >> --- > >> Documentation/driver-api/driver-model/devres.rst | 1 + > >> include/linux/device.h | 11 +++++++++++ > >> 2 files changed, 12 insertions(+) > >> > >> diff --git a/Documentation/driver-api/driver-model/devres.rst b/Documentation/driver-api/driver-model/devres.rst > >> index 4249eb4239e0..8be086b3f829 100644 > >> --- a/Documentation/driver-api/driver-model/devres.rst > >> +++ b/Documentation/driver-api/driver-model/devres.rst > >> @@ -364,6 +364,7 @@ MEM > >> devm_kmalloc_array() > >> devm_kmemdup() > >> devm_krealloc() > >> + devm_krealloc_array() > >> devm_kstrdup() > >> devm_kstrdup_const() > >> devm_kvasprintf() > >> diff --git a/include/linux/device.h b/include/linux/device.h > >> index 472dd24d4823..58f4f5948edb 100644 > >> --- a/include/linux/device.h > >> +++ b/include/linux/device.h > >> @@ -223,6 +223,17 @@ static inline void *devm_kcalloc(struct device *dev, > >> { > >> return devm_kmalloc_array(dev, n, size, flags | __GFP_ZERO); > >> } > >> +static inline __realloc_size(3, 4) void * __must_check > > > > Shouldn't you have a blank line before this one? > > I was going for consistency with the rest of this section which doesn't > have newlines between the functions for some reason. I can add one and > resubmit but it might look a bit out of place? Ah, wasn't aware of that, given the lack of context. So nevermind, it's fine for now. > >> +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); > >> +} > > > > I dislike how we have to keep copying the "real" functions (i.e. > > krealloc_array) into something like this, but I can't think of a better > > way to do it. > > > > Maybe something could be done with some macro magic, but it would > probably end up being worse than just copying them and would affect the > real ones as well. So yeah I can't think of any easy gains either. Ok, that's good. Given a lack of objections from others, I'll just take this through my driver core tree in a few days. thanks, greg k-h
Changes since v3: * Rebase onto v6.4-rc1 Changes since v2: * Remove change in qcom_geni_serial.c in the last commmit and replace it with a comment instead * Whitespace fix Changes since v1: * Style fix ----------------------- Hi, I had a use for a devm realloc_array in a separate change, so I've added one and updated all the obvious existing uses of it that I could find. This is basically a copy paste of the one in slab.h Applies to v6.4-rc1 Thanks James James Clark (4): devres: Provide krealloc_array hwmon: pmbus: Use devm_krealloc_array iio: adc: Use devm_krealloc_array serial: qcom_geni: Comment use of devm_krealloc rather than devm_krealloc_array .../driver-api/driver-model/devres.rst | 1 + drivers/hwmon/pmbus/pmbus_core.c | 6 +++--- drivers/iio/adc/xilinx-ams.c | 9 +++------ drivers/iio/adc/xilinx-xadc-core.c | 17 +++++++---------- drivers/tty/serial/qcom_geni_serial.c | 5 +++++ include/linux/device.h | 11 +++++++++++ 6 files changed, 30 insertions(+), 19 deletions(-)