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

["Tomas Härdin" <tjoppen@acc.umu.se>]

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.

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?

> +
> +    /* 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

/Tomas

_______________________________________________
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".