[FFmpeg-devel] [PATCH]lavu/mem: Make alloc array functions more similar to av_malloc

[Anton Khirnov]

Quoting James Almer (2020-04-12 22:47:55)
> On 4/11/2020 8:53 PM, Carl Eugen Hoyos wrote:
> > Am So., 12. Apr. 2020 um 00:44 Uhr schrieb Carl Eugen Hoyos
> > <ceffmpeg at gmail.com>:
> >>
> >> Am So., 5. Apr. 2020 um 14:03 Uhr schrieb Michael Niedermayer
> >> <michael at niedermayer.cc>:
> >>>
> >>> On Sat, Apr 04, 2020 at 12:46:36AM +0200, Carl Eugen Hoyos wrote:
> >>>> Hi!
> >>>>
> >>>> Attached patch makes the alloc array functions more similar to
> >>>> av_malloc, depending on max_alloc_size instead of INT_MAX.
> >>>>
> >>>> Allows a work-around for ticket #7140
> >>>>
> >>>> Please comment, Carl Eugen
> >>>
> >>>>  mem.c |    8 ++++----
> >>>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>> 507531ed6f0932834d005bc1dd7d18e762f158b2  0001-lavu-mem-Make-alloc-array-functions-more-similar-to-.patch
> >>>> From 7ae240a9f7885130251031aba5d0764b11947fec Mon Sep 17 00:00:00 2001
> >>>> From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
> >>>> Date: Sat, 4 Apr 2020 00:37:03 +0200
> >>>> Subject: [PATCH] lavu/mem: Make alloc array functions more similar to
> >>>>  av_malloc().
> >>>>
> >>>> Do not limit the array allocation functions to allocations of INT_MAX,
> >>>> instead depend on max_alloc_size like av_malloc().
> >>>>
> >>>> Allows a workaround for ticket #7140.
> >>>> ---
> >>>>  libavutil/mem.c | 8 ++++----
> >>>>  1 file changed, 4 insertions(+), 4 deletions(-)
> >>>
> >>> av_size_mult() may be faster
> >>
> >> New patch attached.
> > 
> > And an actually working variant.
> > 
> > Please comment, Carl Eugen
> 
> > From 643c501d6698d7d17e47a9f907165649f1446fa6 Mon Sep 17 00:00:00 2001
> > From: Carl Eugen Hoyos <ceffmpeg at gmail.com>
> > Date: Sun, 12 Apr 2020 00:36:30 +0200
> > Subject: [PATCH] lavu/mem: Make other alloc functions more similar to av_malloc().
> > 
> > Do not limit the array allocation functions and av_calloc() to allocations
> > of INT_MAX, instead depend on max_alloc_size like av_malloc().
> > 
> > Allows a workaround for ticket #7140.
> > ---
> >  libavutil/mem.c | 20 ++++++++++++--------
> >  1 file changed, 12 insertions(+), 8 deletions(-)
> > 
> > diff --git a/libavutil/mem.c b/libavutil/mem.c
> > index 88fe09b179..e044374c62 100644
> > --- a/libavutil/mem.c
> > +++ b/libavutil/mem.c
> > @@ -183,23 +183,26 @@ int av_reallocp(void *ptr, size_t size)
> >  
> >  void *av_malloc_array(size_t nmemb, size_t size)
> >  {
> > -    if (!size || nmemb >= INT_MAX / size)
> > +    size_t result;
> > +    if (av_size_mult(nmemb, size, &result) < 0)
> >          return NULL;
> > -    return av_malloc(nmemb * size);
> > +    return av_malloc(result);
> 
> If I'm reading this right, when size is 0, instead of NULL this will now
> return av_malloc(0), which looks like it may end up being a pointer to a
> 1 byte big buffer. Is that intended?

IIUC some platforms had problems with malloc(0)
Plus I don't think any valid code should ever malloc zero bytes, so my
suggestion would be for all malloc wrappers to:
- av_assert2(size > 0)
- return NULL otherwise

-- 
Anton Khirnov