[FFmpeg-devel] [PATCH 1/5] lavu: add av_dynarray_alloc_elem().

[Stefano Sabatini]

On date Sunday 2013-04-14 03:07:54 +0200, Clément Bœsch encoded:
> ---
>  libavutil/mem.c     | 19 +++++++++++++++++++
>  libavutil/mem.h     | 13 +++++++++++++
>  libavutil/version.h |  2 +-
>  3 files changed, 33 insertions(+), 1 deletion(-)

Reminder: missing APIchanges entry

> 
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index 860c011..835c73a 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -250,6 +250,25 @@ void av_dynarray_add(void *tab_ptr, int *nb_ptr, void *elem)
>      *nb_ptr = nb;
>  }
>  
> +void *av_dynarray_alloc_elem(void **tab_ptr, int *nb_ptr, size_t elem_size)
> +{
> +    int nb = *nb_ptr;
> +    uint8_t *tab = *tab_ptr;
> +

> +    if ((nb & (nb - 1)) == 0) {

maybe add a short note about what's this test is about

> +        int nb_alloc = nb == 0 ? 1 : nb * 2;

overflow check?

> +        tab = av_realloc_f(tab, nb_alloc, elem_size);
> +        if (!tab)
> +            return NULL;
> +        *tab_ptr = tab;
> +    }
> +    *nb_ptr = nb + 1;
> +#if CONFIG_MEMORY_POISONING
> +    memset(tab + nb*elem_size, 0x2a, elem_size);

note: 0x2a could be defined as a macro

> +#endif
> +    return tab + nb*elem_size;
> +}

> +
>  static void fill16(uint8_t *dst, int len)
>  {
>      uint32_t v = AV_RN16(dst - 2);
> diff --git a/libavutil/mem.h b/libavutil/mem.h
> index ced9453..64d86ff 100644
> --- a/libavutil/mem.h
> +++ b/libavutil/mem.h
> @@ -198,6 +198,19 @@ void av_freep(void *ptr);
>  void av_dynarray_add(void *tab_ptr, int *nb_ptr, void *elem);
>  

>  /**
> + * Allocate an element of size elem_size to a dynamic array.

Allocate an element ... to a dynamic array sounds ungrammatical and
not very clear to these non-English ears.
Suggestion:

Allocate an element of size elem_size at the end of a dynamic array.

> + *
> + * @param tab_ptr   Pointer to the array.
> + * @param nb_ptr    Pointer to the number of elements in the array.

nit: inconsistent with usual conventions:

@param tab_ptr pointer to dynamic array
@param nb_ptr  pointer to the number of elements in the array, updated
               by this function

same below

Also what happens if the passed pointers are NULL?

Also the names are somewhat poor (what is tab??).

> + * @param elem_size Size of the elements in the array.

confusing: is this the size of the single element, or of the
array. Also is the size in bytes?

> + * @return          Pointer to the elem_size allocated space at the end of the
> + *                  array, or NULL in case of memory error
> + *

> + * @note this function is not compatible with av_dynarray_add

Complete sentence, missing upcase and final dot, also
av_dynarray_add() is more doxygen friendly.

Also in what sense "is not compatible"?

> + */
> +void *av_dynarray_alloc_elem(void **tab_ptr, int *nb_ptr, size_t elem_size);
> +
> +/**
>   * Multiply two size_t values checking for overflow.
>   * @return  0 if success, AVERROR(EINVAL) if overflow.
>   */
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 6531397..e46e97c 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -75,7 +75,7 @@
>   */
>  
>  #define LIBAVUTIL_VERSION_MAJOR  52
> -#define LIBAVUTIL_VERSION_MINOR  26
> +#define LIBAVUTIL_VERSION_MINOR  27
>  #define LIBAVUTIL_VERSION_MICRO 100
>  
>  #define LIBAVUTIL_VERSION_INT   AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> -- 
> 1.8.2.1
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

-- 
FFmpeg = Faithful & Foolish Majestic Power Emblematic Gorilla