[FFmpeg-devel] [PATCH] Fix potential av_find_opt() crash if context is NULL

[Stefano Sabatini]

On date Thursday 2008-10-30 23:07:29 +0100, Michael Niedermayer encoded:
> On Thu, Oct 30, 2008 at 09:05:32PM +0100, Stefano Sabatini wrote:
[...]
> > OK, but then we should check for its nullness every time we access to
> > it in cmdline.c, since that's generic code and from there we can't
> > know if sws_opts has been initialized or not, same consideration for
> > avformat_opts and avcodec_opts.
> > 
> > *Or*, we can change the semantics of av_find_opt() (micro bump) to
> > make it accept a NULL, for which solution I have a slight preference.
> 
> i disagree, its all fine as it is, NULL is not allowed and that should be
> clear from the docs,
> the avfilter patch is broken its the onle thing that need fixing.

I'll try to explain myself better...

av*_opts and swscale_opts are declared and defined (as NULL) in
cmdutils.c, applications linking to it have to define them to
something different from NULL or they will crash when calling
opt_default().

Patching libavfilter-soc would mean in this case something like the
following in cmdutils.c:

#ifndef CONFIG_AVFILTER
    if(!o)
        o = av_set_string2(sws_opts, opt, arg, 1);
#endif

which I think it's just plain wrong, since cmdutils.o should be
application agnostic, and there is no reason for which we shouldn't
look in sws_opts simply because CONFIG_AVFILTER is set.

So I propose these alternatives:

1) we can check for av*_opts and sws_opts nullness like this:
    if(!o && sws_opts)
        o = av_set_string2(sws_opts, opt, arg, 1);

or 

2) (mabe simpler) change as proposed the semantics of
av_set_string2()/av_find_opt() making them accept NULL values, I have
no idea if this is the best semantics to implement but looks quite
acceptable and it's simpler to implement.

Regards.
-- 
FFmpeg = Freak Fast Meaningful Portable Exciting Gospel