[FFmpeg-devel] [PATCH] Check malloc values in swscale.

[Reimar Döffinger]

On Thu, Aug 13, 2009 at 10:11:45PM -0300, Ramiro Polla wrote:
> Am I being overparanoid cleaning up all previously allocated blocks
> before leaving the function, like in:
>          for (i=0; i<c->vLumBufSize; i++)
> +        {
>              c->alpPixBuf[i]= c->alpPixBuf[i+c->vLumBufSize]= av_mallocz(VOF+1);
> +            if (!c->alpPixBuf[i]) {
> +                for (; i >= 0; i--)
> +                    av_free(c->alpPixBuf[i]);
> +                return NULL;
> +            }
> +        }
> 
> Is ok to just return NULL and leave those previous blocks there?

You should free them, but

> @@ -2905,18 +2939,47 @@ SwsContext *sws_getContext(int srcW, int srcH, enum PixelFormat srcFormat, int d
>  
>      // allocate pixbufs (we use dynamic allocation because otherwise we would need to
>      c->lumPixBuf= av_malloc(c->vLumBufSize*2*sizeof(int16_t*));
> +    if (!c->lumPixBuf)
> +        return NULL;
>      c->chrPixBuf= av_malloc(c->vChrBufSize*2*sizeof(int16_t*));
> +    if (!c->chrPixBuf)
> +        return NULL;
>      if (CONFIG_SWSCALE_ALPHA && isALPHA(c->srcFormat) && isALPHA(c->dstFormat))
> +    {
>          c->alpPixBuf= av_malloc(c->vLumBufSize*2*sizeof(int16_t*));
> +        if (!c->alpPixBuf)
> +            return NULL;
> +    }
>      //Note we need at least one pixel more at the end because of the MMX code (just in case someone wanna replace the 4000/8000)
>      /* align at 16 bytes for AltiVec */
>      for (i=0; i<c->vLumBufSize; i++)
> +    {
>          c->lumPixBuf[i]= c->lumPixBuf[i+c->vLumBufSize]= av_mallocz(VOF+1);
> +        if (!c->lumPixBuf[i]) {
> +            for (; i >= 0; i--)
> +                av_free(c->lumPixBuf[i]);
> +            return NULL;
> +        }
> +    }

It seems a bit pointless if you go only half the way and don't free
lumPixBuf etc.
That kind of thing is why so often goto err_out or so is used.
Also you should use av_freep everywhere (and check if you can make it so
you can just call the normal uninit function instead of adding a lot of
code to free).