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

[Michael Niedermayer]

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
i dont know if its better to change the implementation or the docs but either
one must be changed ...


> + * @param size Size of the block to be allocated.
> + * @return Pointer to the allocated block, NULL if it cannot allocate
> + * it.
> + * @see av_mallocz()
>   */
>  void *av_malloc(unsigned int size);
>  
>  /**
> - * 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

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Observe your enemies, for they first find out your faults. -- Antisthenes
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070923/7d7ce25a/attachment.pgp>