[FFmpeg-devel] [PATCH] [RFC] libavutil/mem.c: Check return value of posix_memalign

[Måns Rullgård]

Patrik Kullman <patrik at yes.nu> writes:

> On Fri, 2009-02-13 at 18:11 +0000, M?ns Rullg?rd wrote:
>> Patrik Kullman <patrik at yes.nu> writes:
>> 
>> > On Fri, 2009-02-13 at 16:26 +0000, M?ns Rullg?rd wrote:
>> >> Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:
>> >> 
>> >> > On Fri, Feb 13, 2009 at 03:47:23PM +0000, M?ns Rullg?rd wrote:
>> >> >> > libavutil/mem.c: In function ?av_malloc?:
>> >> >> > libavutil/mem.c:66: warning: ignoring return value of ?posix_memalign?,
>> >> >> > declared with attribute warn_unused_result
>> >> >> 
>> >> >> That warning is bogus.  If posix_memalign() fails, memalign() will too
>> >> >> since they are likely to be the same function.  We set the pointer to
>> >> >> NULL before the call, so if it fails, we return NULL as we're supposed
>> >> >> to.
>> >> >
>> >> > Well, if you are pedantic you here assume that posix_memalign will not
>> >> > modify ptr if it fails (or at least not set it != NULL), but that
>> >> > actually does not seem to be guaranteed by POSIX (it doesn't say
>> >> > anything about the value of ptr when an error is returned...).
>> >> 
>> >> That is indeed the case, and thus your suggestion of explicitly
>> >> setting it to NULL makes sense.
>> >
>> > So, this is OK?
>> >
>> > Index: libavutil/mem.c
>> > ===================================================================
>> > --- libavutil/mem.c	(revision 17211)
>> > +++ libavutil/mem.c	(working copy)
>> > @@ -63,7 +63,8 @@
>> >      ptr = (char*)ptr + diff;
>> >      ((char*)ptr)[-1]= diff;
>> >  #elif HAVE_POSIX_MEMALIGN
>> > -    posix_memalign(&ptr,16,size);
>> > +    if (posix_memalign(&ptr,16,size) != 0)
>> 
>> Drop the != 0.
>> 
>> > +        ptr = NULL;
>> >  #elif HAVE_MEMALIGN
>> >      ptr = memalign(16,size);
>> >      /* Why 64?
>> 
>> Otherwise no objections from me.  Maybe we should drop the initial
>> NULL assignment since with this change, it is not necessary.
>
> Ok, dropped the != 0.
>
>
> Index: libavutil/mem.c
> ===================================================================
> --- libavutil/mem.c	(revision 17211)
> +++ libavutil/mem.c	(working copy)
> @@ -63,7 +63,8 @@
>      ptr = (char*)ptr + diff;
>      ((char*)ptr)[-1]= diff;
>  #elif HAVE_POSIX_MEMALIGN
> -    posix_memalign(&ptr,16,size);
> +    if (posix_memalign(&ptr,16,size))
> +        ptr = NULL;
>  #elif HAVE_MEMALIGN
>      ptr = memalign(16,size);
>      /* Why 64?

Now we're just waiting for the maintainer to give his approval (or not).

-- 
M?ns Rullg?rd
mans at mansr.com