[FFmpeg-devel] [PATCH 1/2] opus_parser: fix leaking channel_maps on error

[Michael Niedermayer]

On Fri, Dec 09, 2016 at 11:42:59PM +0100, Andreas Cadhalpun wrote:
> On 09.12.2016 15:23, Michael Niedermayer wrote:
> > On Fri, Dec 09, 2016 at 12:08:10AM +0100, Andreas Cadhalpun wrote:
> >> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> >> ---
> >>  libavcodec/opus_parser.c | 2 +-
> >>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>
> >> diff --git a/libavcodec/opus_parser.c b/libavcodec/opus_parser.c
> >> index c30fd7b..21a73ee 100644
> >> --- a/libavcodec/opus_parser.c
> >> +++ b/libavcodec/opus_parser.c
> >> @@ -116,11 +116,11 @@ static int opus_find_frame_end(AVCodecParserContext *ctx, AVCodecContext *avctx,
> >>  
> >>      if (avctx->extradata && !s->extradata_parsed) {
> >>          ret = ff_opus_parse_extradata(avctx, &s->ctx);
> >> +        av_freep(&s->ctx.channel_maps);
> >>          if (ret < 0) {
> >>              av_log(avctx, AV_LOG_ERROR, "Error parsing Ogg extradata.\n");
> >>              return AVERROR_INVALIDDATA;
> >>          }
> >> -        av_freep(&s->ctx.channel_maps);
> >>          s->extradata_parsed = 1;
> >>      }
> > 
> > isnt it more correct for ff_opus_parse_extradata() to cleanup what
> > it allocated on error ?
> 
> Probably, but opusdec already did it the other way around.
> I'm fine with changing that, though. See attached patch.
> 
> Best regards,
> Andreas
> 

>  opus.c    |    1 +
>  opusdec.c |    1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)
> 3e534ddbb39e05f8c7a5cfeba4b65ed587c3a519  0001-opus_parser-fix-leaking-channel_maps-on-error.patch
> From 2edf5f9a571d483858928dd6bb9e84031ef8a391 Mon Sep 17 00:00:00 2001
> From: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> Date: Fri, 9 Dec 2016 00:00:18 +0100
> Subject: [PATCH 1/2] opus_parser: fix leaking channel_maps on error
> 
> Make ff_opus_parse_extradata free allocated memory on error instead of
> expecting callers to free it in that case.
> 
> Signed-off-by: Andreas Cadhalpun <Andreas.Cadhalpun at googlemail.com>
> ---
>  libavcodec/opus.c    | 1 +
>  libavcodec/opusdec.c | 1 -
>  2 files changed, 1 insertion(+), 1 deletion(-)

LGTM

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Complexity theory is the science of finding the exact solution to an
approximation. Benchmarking OTOH is finding an approximation of the exact
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20161210/a465185a/attachment.sig>