[FFmpeg-devel] [PATCH 1/4] lavu: add simple array implementation

[Michael Niedermayer]

On Tue, Mar 04, 2014 at 03:00:02AM +0100, Lukasz Marek wrote:
> On 03.03.2014 11:33, Michael Niedermayer wrote:
> >On Mon, Mar 03, 2014 at 11:00:18AM +0100, Lukasz M wrote:
> >>W dniu poniedziałek, 3 marca 2014 Michael Niedermayer <michaelni at gmx.at>
> >>napisał(a):
> >>
> >>>On Mon, Mar 03, 2014 at 01:59:43AM +0100, Lukasz Marek wrote:
> >>>>
> >>>>>You're adding it to lavu, and it's not marked as internal/experimental.
> >>>If
> >>>>>it stinks or needs 3 API updates or re-implementations, the old cruft
> >>>>>sticks around for years to come.
> >>>>
> >>>>Can you precise how to mark it? I added it as public, but don't
> >>>>understand how can i make it experimental.
> >>>>
> >>>
> >>>>>That's why we now have 2 (and with yours, soon 3) array implementations.
> >>>>>Can we stick to one? At least java uses the same API for all its
> >>>array/list
> >>>>>things.
> >>>>
> >>>>The existing two doesn't allow to free "objects" with allocated
> >>>>pointers inside. If you fix them then you have 4. I really think it
> >>>>is better to have one common for wide range of use cases. I can
> >>>>refactor existing code to use mine if accepted.
> >>>
> >>>if i have an array of "object" with pointers inside them and want
> >>>to free them id do
> >>>
> >>>for(i=0; i<arraysize; i++)
> >>>     my_free(my_array + i);
> >>>
> >>>that needs no API beyond ANSI/ISO C
> >>>what am i missing ?
> >>>
> >>
> >>nothing. existing array impl is not so smart to do that on memory
> >>allocation fail AFAIR.
> >
> >maybe it could be solved with something like
> >int av_dynarray_add_nofree(...)
> >
> >which would not free the array on failure but return an error code
> >and leave deallocation (if needed) to the caller
> 
> changed this way. I used proposed name, if someone have better one
> then go ahead.
> 
> -- 
> Best Regards,
> Lukasz Marek
> 
> If you can't explain it simply, you don't understand it well enough.
> - Albert Einstein

>  mem.c |   22 +++++++++++++++-------
>  mem.h |   19 +++++++++++++++++--
>  2 files changed, 32 insertions(+), 9 deletions(-)
> 34775144f41e00c4b5ee2c5b5fb4c18dd2dae4ae  0001-lavu-mem-add-av_dynarray_add_nofree-function.patch
> From 6cdb1cb4b846478c8d7bc3c44e9e5d474aca52e0 Mon Sep 17 00:00:00 2001
> From: Lukasz Marek <lukasz.m.luki at gmail.com>
> Date: Tue, 25 Feb 2014 01:06:06 +0100
> Subject: [PATCH 1/4] lavu/mem: add av_dynarray_add_nofree function
> 
> av_dynarray_add_nofree function have similar functionality
> as existing av_dynarray_add, but it doesn't deallocate memory
> on fails.
> 
> TODO: minor bump and update doc/APIChanges
> 
> Signed-off-by: Lukasz Marek <lukasz.m.luki at gmail.com>
> ---
>  libavutil/mem.c | 22 +++++++++++++++-------
>  libavutil/mem.h | 19 +++++++++++++++++--
>  2 files changed, 32 insertions(+), 9 deletions(-)
> 
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index 10b0137..1c73f4a 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -58,6 +58,10 @@ void  free(void *ptr);
>  
>  #endif /* MALLOC_PREFIX */
>  
> +#if defined(__MINGW32__)
> +#define EOVERFLOW EFBIG
> +#endif

> +
>  #define ALIGN (HAVE_AVX ? 32 : 16)
>  
>  /* NOTE: if you want to override these functions with your own
> @@ -277,7 +281,7 @@ void *av_memdup(const void *p, size_t size)
>      return ptr;
>  }
>  
> -void av_dynarray_add(void *tab_ptr, int *nb_ptr, void *elem)
> +int av_dynarray_add_nofree(void *tab_ptr, int *nb_ptr, void *elem)
>  {
>      /* see similar ffmpeg.c:grow_array() */
>      int nb, nb_alloc;
> @@ -290,21 +294,25 @@ void av_dynarray_add(void *tab_ptr, int *nb_ptr, void *elem)
>              nb_alloc = 1;
>          } else {
>              if (nb > INT_MAX / (2 * sizeof(intptr_t)))
> -                goto fail;
> +                return AVERROR(EOVERFLOW);

we simply return ENOMEM for these elsewhere if iam not mistaken
and that would avoid the compatibility hack  on mingw
EINVAL might another option

patch LGTM otherwise

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The real ebay dictionary, page 1
"Used only once"    - "Some unspecified defect prevented a second use"
"In good condition" - "Can be repaird by experienced expert"
"As is" - "You wouldnt want it even if you were payed for it, if you knew ..."
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140304/3919dcd7/attachment.asc>