On Mon, 12 Dec 2011 19:12:13 +0100, Luca Barbato <[email protected]> wrote:
> On 12/12/11 18:49, Anton Khirnov wrote:
> >
> >
> > On Mon, 12 Dec 2011 18:24:29 +0100, Luca Barbato<[email protected]>  wrote:
> >> Fix the iformat/oformat typo and make the function behave if
> >> the context is already NULL.
> >> ---
> >>   libavformat/utils.c |    5 ++++-
> >>   1 files changed, 4 insertions(+), 1 deletions(-)
> >>
> >> diff --git a/libavformat/utils.c b/libavformat/utils.c
> >> index 8b749ad..865edbc 100644
> >> --- a/libavformat/utils.c
> >> +++ b/libavformat/utils.c
> >> @@ -3222,6 +3222,9 @@ int av_write_trailer(AVFormatContext *s)
> >>   {
> >>       int ret, i;
> >>
> >> +    if (!s || !s->oformat)
> >> +        return 0;
> >> +
> >
> > Why would this ever happen?
> 
> free() pattern compliance, just to be safe.
> 
> Not sure if we should consider having av_write_trailer more than once
> wrong but if it happens currently we just segfault.

I am not sure this is a very good idea.
Overzealous checks like this might hide programming errors in the
calling code.

There are many good reasons to call free() on a NULL pointer and
semantically it can be seen as freeing a zero-length allocated block.

OTOH I can't see a good reason to ever call av_write_trailer on a NULL
AVFormatContext or with a NULL oformat. This is simply an invalid
operation, so even if we handle it, IMO the proper response would be
AVERROR(EINVAL).

-- 
Anton Khirnov
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel

Reply via email to