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

[James Almer]

On 4/12/2020 6:53 PM, Carl Eugen Hoyos wrote:
> Am So., 12. Apr. 2020 um 23:52 Uhr schrieb James Almer <jamrial at gmail.com>:
>>
>> On 4/12/2020 5:55 PM, Carl Eugen Hoyos wrote:
>>> Am So., 12. Apr. 2020 um 22:48 Uhr schrieb James Almer <jamrial at gmail.com>:
>>>>
>>>> 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?
>>>>
>>>> The previous version you sent apparently considered that scenario.
>>>
>>> But it did not pass fate because the behaviour before the patch
>>> was not to return NULL for alloc(0).
>>
>> Before this patch it would return NULL when size was 0 and alloc(0) when
>> nmemb was 0. Now it will return alloc(0) when either of the two
>> arguments is 0.
>>
>> The check should be (!size || av_size_mult(nmemb, size, &result) < 0) or
>> similar instead, if we want to keep the original behavior.
> 
> How did the original behaviour make any sense?

Not saying it made sense, i'm saying we changed that behavior when the
patch, at least based on the description, only tried to replace the
INT_MAX limit with max_alloc_size.

If making size 0 return malloc(0) was intended, or ultimately preferred,
then I'll not oppose to it.