[FFmpeg-devel] [PATCH] Doxygenate libavutil/mem.h

[Rich Felker]

On Sun, Sep 23, 2007 at 09:47:44PM +0200, Michael Niedermayer wrote:
> Hi
> 
> On Sat, Sep 22, 2007 at 03:18:16PM +0200, Stefano Sabatini wrote:
> > On date Saturday 2007-09-22 12:26:57 +0200, Michael Niedermayer encoded:
> > [...]
> > > rejected!
> > > 
> > > ive said it in the past and i say it again, the doxygen comments in teh
> > > headers document the API they do NOT document the implementation behavior
> > > 
> > > this is not a little cosmetic issue, if you say X there it means we must
> > > do X unless we bump the major version number
> > > simply saying we do what implementation Y does is sick, what if 2 versions
> > > of Y do something else? what if Y violates the c spec?
> > > 
> > > please DO NOT document what the current implementation does in the
> > > headers this is INAPPROPRIATE!
> > > document what it should do if its free of bugs and do it in the most
> > > generic way possible
> > > 
> > > for example:
> > > wrong:
> > > @param datestr String representing a date or a duration.
> > > - If a date the syntax is:
> > > [{YYYY-MM-DD|YYYYMMDD}]{T| }{HH[:MM[:SS[.m...]]][Z]|HH[MM[SS[.m...]]][Z]}
> > > 
> > > correct:
> > > @param datestr String representing a date or a duration.
> > > parse_date supports various forms currently supported is:
> > > [{YYYY-MM-DD|YYYYMMDD}]{T| }{HH[:MM[:SS[.m...]]][Z]|HH[MM[SS[.m...]]][Z]}
> > > future versions might add further variants
> > 
> > Hi Michael, 
> > 
> > yes you're completely right, now I see how silly it was.
> > 
> > I amended the previous patch eliminating in the av_realloc
> > documentation all the references to other functions implementations,
> > hope it is now more correct.
> > 
> > Regards.
> > -- 
> > Stefano Sabatini
> > Linux user number 337176 (see http://counter.li.org)
> 
> > Index: libavutil/mem.h
> > ===================================================================
> > --- libavutil/mem.h	(revision 10549)
> > +++ libavutil/mem.h	(working copy)
> > @@ -33,26 +33,47 @@
> >  #endif
> >  
> >  /**
> > - * Memory allocation of size byte with alignment suitable for all
> > + * Allocates a block of \p size bytes with alignment suitable for all
> >   * memory accesses (including vectors if available on the
> 
> > - * CPU). av_malloc(0) must return a non NULL pointer.
> > + * CPU). av_malloc(0) must return a non-NULL pointer.
> 
> i know this isnt new but its not true, av_malloc(0) will return NULL if
> malloc(0) does in some cases

As rightly it should. Implementations which return non-NULL for
zero-size mallocs are fundamentally stupid despite being valid.

> i dont know if its better to change the implementation or the docs but either
> one must be changed ...

Change the docs. There's no reason to force libavutil to mimic stupid
glibc behavior. Either it should force the sane behavior or leave it
unspecified...

> > - * av_realloc semantics (same as glibc): if ptr is NULL and size > 0,
> > - * identical to malloc(size). If size is zero, it is identical to
> > - * free(ptr) and NULL is returned.
> > + * Allocates or reallocates a block of size \p size with alignment
> > + * suitable for all memory accesses (including vectors if available on
> > + * the CPU).
> 
> this is not true, the alignment will be as bad as what realloc() returns
> currently, we could fix the implementaton (IIRC there where some patches)
> but it would slow down av_realloc() considerably (>2x) if the realloc()
> output isnt aligned

:(

Rich