Re: [FFmpeg-devel] [PATCH 3/8] avutil/mem: Add av_fast_realloc_array()

[Andreas Rheinhardt <andreas.rheinhardt@outlook.com>]

Tomas Härdin:
> tis 2022-07-05 klockan 22:26 +0200 skrev Andreas Rheinhardt:
>> From: Andreas Rheinhardt <andres.rheinhardt@outlook.com>
>>
>> This is an array-equivalent of av_fast_realloc(). Its advantages
>> compared to using av_fast_realloc() for allocating arrays are as
>> follows:
>>
>> a) It performs its own overflow checks for the multiplication that is
>> implicit in array allocations. (And it only needs to perform these
>> checks (as well as the multiplication itself) in case the array needs
>> to
>> be reallocated.)
>> b) It allows to limit the number of elements to an upper bound given
>> by the caller. This allows to restrict the number of allocated
>> elements
>> to fit into an int and therefore makes this function usable with
>> counters of this type. It can also be used to avoid overflow checks
>> in
>> the caller: E.g. setting it to UINT_MAX - 1 elements makes it safe to
>> increase the desired number of elements in steps of one. And it
>> avoids
>> overallocations in situations where one already has an upper bound.
>> c) av_fast_realloc_array() will always allocate in multiples of array
>> elements; no memory is wasted with partial elements.
>> d) By returning an int, av_fast_realloc_array() can distinguish
>> between
>> ordinary allocation failures (meriting AVERROR(ENOMEM)) and failures
>> because of allocation limits (by returning AVERROR(ERANGE)).
>> e) It is no longer possible for the user to accidentally lose the
>> pointer by using ptr = av_fast_realloc(ptr, ...).
> 
> If you add an option for clearing the newly allocated memory then this
> could work for my av_fast_recalloc() use case in the jpeg2000 decoder.
> Or we could have two functions.
> 

I'd prefer it if the zeroing function were a wrapper around the
non-zeroing function.

> Small bikeshed: since the function takes a pointer to a pointer as
> argument, av_fast_realloc_arrayp() might be a better name. I had in
> mind to similarly rename av_fast_recalloc() to av_fast_recallocp().
> 
> 
>> +
>> +    nb = min_nb + (min_nb + 14) / 16;
> 
> Not +15? Or +0?

"av_fast_realloc_array() instead allocates nb + (nb + 14) / 16. Rounding
up is done in order not to reallocate in steps of one if the current
number is < 16; adding 14 instead of 15 has the effect of only
allocating one element if one element is desired."

> 
>> +
>> +    /* If min_nb is so big that the above calculation overflowed,
>> +     * just allocate as much as we are allowed to. */
>> +    nb = nb < min_nb ? max_nb : FFMIN(nb, max_nb);
> 
> Looks OK, but an explicit check for overflow is easier to verify
> 
>> +
>> +    memcpy(&array, ptr, sizeof(array));
>> +
>> +    array = av_realloc(array, nb * elsize);
>> +    if (!array)
>> +        return AVERROR(ENOMEM);
>> +
>> +    memcpy(ptr, &array, sizeof(array));
> 
> An optional memset() here would be useful for me
> 
> Else it looks OK
> 
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".