[MPlayer-cvslog] r27841 - in trunk: libmpdemux/demux_mkv.c libmpdemux/stheader.h mencoder.c mplayer.c spudec.c spudec.h vobsub.c vobsub.h

[Reimar Döffinger]

On Sat, Nov 01, 2008 at 01:21:28AM +0100, Aurelien Jacobs wrote:
> On Fri, 31 Oct 2008 12:06:06 +0100
> Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> wrote:
> > and in that case to free it without setting it to NULL again would
> > be wrong.
> 
> Except if this field won't ever be used again.

Yeah, but you explicitly coded that assumption in, thus making sure it
will break if someone ever tries to use it elsewhere, without even
documenting the fact.

> > I am wondering a bit if it is such a good idea to have the extradata
> > inside the vob, since it is handled different from the other fields,
> > e.g. it is only used internally in vobsub_open, it is not freed by
> > vobsub_close...
> 
> Second attached patch moves extradata out of the vob, but it is
> ugly IMHO.

I do not mind much, but IMO do at least one of these things:
1) move extradata out of the struct
2) document the special behaviour
3) set it to NULL and extradata_len to 0 after freeing it (and
preferably also add code to free it to the close function).

Greetings,
Reimar Döffinger