[FFmpeg-devel] [PATCH] Use posix_memalign() instead of memalign()

[Ramiro Polla]

Hi,

On Tue, Oct 7, 2008 at 3:25 PM, M?ns Rullg?rd <mans at mansr.com> wrote:
> "Ramiro Polla" <ramiro.polla at gmail.com> writes:
>
>> On Tue, Oct 7, 2008 at 4:24 AM, M?ns Rullg?rd <mans at mansr.com> wrote:
>>> "Ramiro Polla" <ramiro.polla at gmail.com> writes:
>>>
>>>> Hi,
>>>>
>>>> On Sun, Sep 28, 2008 at 10:17 AM, M?ns Rullg?rd <mans at mansr.com> wrote:
>>>>> Diego 'Flameeyes' Petten? <flameeyes at gmail.com> writes:
>>>>>
>>>>>> While on GLIBC the memalign() function is declared in malloc.h, on
>>>>>> OpenSolaris its definiton is in stdlib.h; include the file so that an
>>>>>> implicit declaration can be avoided.
>>>>>> ---
>>>>>>
>>>>>>  libavutil/mem.c |    1 +
>>>>>>  1 files changed, 1 insertions(+), 0 deletions(-)
>>>>>>
>>>>>> diff --git a/libavutil/mem.c b/libavutil/mem.c
>>>>>> index 960074c..da75638 100644
>>>>>> --- a/libavutil/mem.c
>>>>>> +++ b/libavutil/mem.c
>>>>>> @@ -34,6 +34,7 @@
>>>>>>  #ifdef HAVE_MALLOC_H
>>>>>>  #include <malloc.h>
>>>>>>  #endif
>>>>>> +#include <stdlib.h>
>>>>>>
>>>>>>  /* you can redefine av_malloc and av_free in your project to use your
>>>>>>     memory allocator. You do not need to suppress this file because the
>>>>>
>>>>> #including stdlib.h obviously can't do any harm, it being a standard
>>>>> header.
>>>>
>>>>> There is, however, a deeper issue here: memalign() is not
>>>>> defined by any standard, and some systems that have it provide no
>>>>> means to free such allocations.  Is there any reason we can't use the
>>>>> standard posix_memalign() instead?
>>>>
>>>> Attached patch should spark some discussion...
>>>>
>>>
>>> [...]
>>>
>>>> Index: libavutil/mem.c
>>>> ===================================================================
>>>> --- libavutil/mem.c   (revision 15575)
>>>> +++ libavutil/mem.c   (working copy)
>>>> @@ -35,6 +35,10 @@
>>>>  #include <malloc.h>
>>>>  #endif
>>>
>>> #include <malloc.h> wouldn't be needed with posix_memalign(), but see
>>> below.
>>
>> Which systems use malloc.h?
>
> BSD systems predating posix_memalign().  I wouldn't be surprised if
> OpenBSD is still missing it.
>
>>>> +#ifdef HAVE_POSIX_MEMALIGN
>>>> +#include <stdlib.h>
>>>> +#endif
>>>
>>> It is always save to #include <stdlib.h>
>>
>> Now I see that the stdlib.h include comes from common.h.
>>
>> And another thing, I remember there was some discussion regarding
>> whether we should include only the necessary files or whether it was
>> ok to just include common.h everywhere, but I forgot the outcome.
>
> I don't think there ever was a consensus.  IMHO, it is good practise
> to #include standard headers where needed, even if common.h includes
> them as well.  Things that are conditionally included in
> common.h/internal.h are a different matter, since duplicating checks
> all over the code is not pretty.
>
>>>>  /* you can redefine av_malloc and av_free in your project to use your
>>>>     memory allocator. You do not need to suppress this file because the
>>>>     linker will do it automatically */
>>>> @@ -57,8 +61,9 @@
>>>>      diff= ((-(long)ptr - 1)&15) + 1;
>>>>      ptr = (char*)ptr + diff;
>>>>      ((char*)ptr)[-1]= diff;
>>>> -#elif defined (HAVE_MEMALIGN)
>>>> -    ptr = memalign(16,size);
>>>> +#elif defined (HAVE_POSIX_MEMALIGN)
>>>> +    if(posix_memalign(&ptr,16,size))
>>>> +        ptr = NULL;
>>>
>>> It might make sense to use memalign() as a fall-back for old BSD
>>> systems and the like without posix_memalign().
>>
>> Would that be a manually entered list like is done for need_memalign?
>
> No, it would be tested by configure.

New patch attached.

Ramiro Polla
-------------- next part --------------
A non-text attachment was scrubbed...
Name: posix_memalign.diff
Type: text/x-diff
Size: 1559 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20081218/09626509/attachment.diff>