[FFmpeg-cvslog] r22119 - in trunk/libavutil: tree.c tree.h

[Vitor Sessak]

Michael Niedermayer wrote:
> On Tue, Mar 02, 2010 at 06:40:03AM +0100, Vitor Sessak wrote:
>> Michael Niedermayer wrote:
>>> On Sun, Feb 28, 2010 at 09:48:42PM +0100, vitor wrote:
>>>> Author: vitor
>>>> Date: Sun Feb 28 21:48:42 2010
>>>> New Revision: 22119
>>>>
>>>> Log:
>>>> Implement av_tree_destroy_free_elem() to destroy a tree and free all the 
>>>> values stored on it.
>>>>
>>>> Modified:
>>>>    trunk/libavutil/tree.c
>>>>    trunk/libavutil/tree.h
>>>>
>>>> Modified: trunk/libavutil/tree.c
>>>> ==============================================================================
>>>> --- trunk/libavutil/tree.c	Sun Feb 28 20:58:26 2010	(r22118)
>>>> +++ trunk/libavutil/tree.c	Sun Feb 28 21:48:42 2010	(r22119)
>>>> @@ -135,6 +135,15 @@ void av_tree_destroy(AVTreeNode *t){
>>>>      }
>>>>  }
>>>>  +void av_tree_destroy_free_elem(AVTreeNode *t){
>>>> +    if(t){
>>>> +        av_tree_destroy_free_elem(t->child[0]);
>>>> +        av_tree_destroy_free_elem(t->child[1]);
>>>> +        av_free(t->elem);
>>>> +        av_free(t);
>>>> +    }
>>>> +}
>>>> +
>>>>  #if 0
>>>>  void av_tree_enumerate(AVTreeNode *t, void *opaque, int (*cmp)(void 
>>>> *opaque, void *elem), int (*enu)(void *opaque, void *elem)){
>>>>      if(t){
>>>>
>>>> Modified: trunk/libavutil/tree.h
>>>> ==============================================================================
>>>> --- trunk/libavutil/tree.h	Sun Feb 28 20:58:26 2010	(r22118)
>>>> +++ trunk/libavutil/tree.h	Sun Feb 28 21:48:42 2010	(r22119)
>>>> @@ -78,5 +78,6 @@ void *av_tree_find(const struct AVTreeNo
>>>>   */
>>>>  void *av_tree_insert(struct AVTreeNode **rootp, void *key, int 
>>>> (*cmp)(void *key, const void *b), struct AVTreeNode **next);
>>>>  void av_tree_destroy(struct AVTreeNode *t);
>>>> +void av_tree_destroy_free_elem(struct AVTreeNode *t);
>>> argh, ive missed this part of your patch
>>> minor api bump & doxy missing and its the wrong way to do it
>>> you dont know at all if avfree() is correct and sufficient to free the
>>> opaque elements.
>>> av_tree_enumerate() is likely the correct way to free
>> Ok, but current version of av_tree_enumerate() is commented out and does 
>> not do what one would expect. What do you think of this patch?
>>
>> -Vitor
> 
>>  libavformat/nut.c    |   11 +++++++++++
>>  libavformat/nut.h    |    1 +
>>  libavformat/nutdec.c |    1 +
>>  libavformat/nutenc.c |    1 +
>>  libavutil/avutil.h   |    2 +-
>>  libavutil/tree.c     |   14 ++++++--------
>>  libavutil/tree.h     |    7 +++++++
>>  7 files changed, 28 insertions(+), 9 deletions(-)
>> 073f43adb2344cede149943eef2ce2cae50d7930  nuttree.diff
>> Index: libavutil/tree.c
>> ===================================================================
>> --- libavutil/tree.c	(revision 22135)
>> +++ libavutil/tree.c	(working copy)
>> @@ -135,16 +135,14 @@
>>      }
>>  }
>>  
>> -#if 0
> 
> 
>> -void av_tree_enumerate(AVTreeNode *t, void *opaque, int (*cmp)(void *opaque, void *elem), int (*enu)(void *opaque, void *elem)){
>> -    if(t){
>> -        int v= cmp ? cmp(opaque, t->elem) : 0;
>> -        if(v>=0) av_tree_enumerate(t->child[0], opaque, cmp, enu);
>> -        if(v==0) enu(opaque, t->elem);
>> -        if(v<=0) av_tree_enumerate(t->child[1], opaque, cmp, enu);
>> +void av_tree_enumerate(AVTreeNode *t, void *opaque,
>> +                       void (*enu)(void *opaque, void **elem)){
>> +    if (t) {
>> +        av_tree_enumerate(t->child[0], opaque, enu);
>> +        enu(opaque, &t->elem);
>> +        av_tree_enumerate(t->child[1], opaque, enu);
>>      }
> 
> no, this is broken
> the only question on the API i had IIRC was if we should allow elements
> to be removed based on the return code of enu()
> ruining the API by droping 2/3 of the features is not an option

Ok, so since there is no documentation for it, can you please explain me 
what av_tree_enumerate() is supposed to do and in what way it is 
different from

{
     void *elem  = av_tree_find(t, opaque, cmp, NULL);
     if (elem)
         enu(opaque, elem);
}

-Vitor