[MPlayer-cvslog] CVS: main/libmpdemux muxer.c,1.14,1.15

[Rich Felker]

On Sun, Mar 12, 2006 at 09:58:27PM +0100, Michael Niedermayer wrote:
> Hi
> 
> On Sun, Mar 12, 2006 at 07:13:14PM +0100, Nico Sabbi CVS wrote:
> > CVS change done by Nico Sabbi CVS
> > 
> > Update of /cvsroot/mplayer/main/libmpdemux
> > In directory mail:/var2/tmp/cvs-serv10439
> > 
> > Modified Files:
> > 	muxer.c 
> > Log Message:
> > exit if calloc() fails;  free(muxer) before returning NULL if muxer_init() fails (to avoid memleak). Fixes cid 173
> > 
> > Index: muxer.c
> > ===================================================================
> > RCS file: /cvsroot/mplayer/main/libmpdemux/muxer.c,v
> > retrieving revision 1.14
> > retrieving revision 1.15
> > diff -u -r1.14 -r1.15
> > --- muxer.c	26 Jan 2006 19:32:07 -0000	1.14
> > +++ muxer.c	12 Mar 2006 18:13:11 -0000	1.15
> > @@ -21,30 +21,47 @@
> >  muxer_t *muxer_new_muxer(int type,FILE *f){
> >      muxer_t* muxer=malloc(sizeof(muxer_t));
> >      memset(muxer,0,sizeof(muxer_t));
> > +    if(!muxer)
> > +        return NULL;
> 
> memset before the check?!

I thought MPlayer policy was not to check the return value of malloc
anyway unless it could lead to vulns (e.g. first access is not to the
returned pointer itself but some large offset into it..)

> >        case MUXER_TYPE_AVI:
> >        default:
> >  	if(! muxer_init_muxer_avi(muxer))
> > -	  return NULL;
> > +        {
> > +          free(muxer);
> > +          return NULL;
> > +        }
> >      }
> >      return muxer;
> >  }
> 
> a goto fail; everywhere and a fail: if(muxer) free(muxer); return NULL; at
> the end would be IMHO simpler and cleaner

Agree.

Rich