From patchwork Fri Dec 9 22:42:59 2016 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Cadhalpun X-Patchwork-Id: 1736 Delivered-To: ffmpegpatchwork@gmail.com Received: by 10.103.65.86 with SMTP id o83csp565377vsa; Fri, 9 Dec 2016 14:43:12 -0800 (PST) X-Received: by 10.28.217.195 with SMTP id q186mr400620wmg.17.1481323392808; Fri, 09 Dec 2016 14:43:12 -0800 (PST) Return-Path: Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org. [79.124.17.100]) by mx.google.com with ESMTP id f7si35736563wjg.224.2016.12.09.14.43.12; Fri, 09 Dec 2016 14:43:12 -0800 (PST) Received-SPF: pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) client-ip=79.124.17.100; Authentication-Results: mx.google.com; dkim=neutral (body hash did not verify) header.i=@googlemail.com; spf=pass (google.com: domain of ffmpeg-devel-bounces@ffmpeg.org designates 79.124.17.100 as permitted sender) smtp.mailfrom=ffmpeg-devel-bounces@ffmpeg.org; dmarc=fail (p=QUARANTINE dis=NONE) header.from=googlemail.com Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id EA172689DF3; Sat, 10 Dec 2016 00:43:05 +0200 (EET) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 37998689E11 for ; Sat, 10 Dec 2016 00:42:59 +0200 (EET) Received: by mail-wm0-f66.google.com with SMTP id u144so29724wmu.0 for ; Fri, 09 Dec 2016 14:43:03 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=googlemail.com; s=20120113; h=from:subject:to:references:message-id:date:user-agent:mime-version :in-reply-to; bh=xUFH5MR4atc0+MtqpCrDLMobJEqr5XrHqPx72yONNHs=; b=Tbud50FuPK1CLWiZq20br5oPjvX4w7N0vxHFqEVk74nOI2u9tNIVKCfuFP9TSuk68X wQrkDvHTGzwVCALhg54mi+y01obsAAqyZiTSkymNXMHUfjy1dXmw3gcw9S3j1Ldhh8gI TfUBaqykSaNNk7V6lqGwA9rYERUjj8XFLg7NGbXquDdVdjgxfUINN8jFogjSmpihkrLC v1JXzbw/lIUNNSym2VGiLDFwDOSVfTaXPCfJQFGKaxqRp944G91YjNbY0mDhWlF5wUvQ hgKQGba6SU5mVVv/s6uJSvt9bQOnV7wxtF2wIiQaikRpyqIu8E2d7Va9K4OaX7q72yFo sDvQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:from:subject:to:references:message-id:date :user-agent:mime-version:in-reply-to; bh=xUFH5MR4atc0+MtqpCrDLMobJEqr5XrHqPx72yONNHs=; b=nNfccSGJ3ar1CVM1t9fzW7yWk8ofmghRDeq2JcDghNptEhO7JiI1Dv8Vc7Ok/4Jfiy PnmUjXp3lU5H1Cm9ce6gUKjizn5JT/LYUbsPhPpeL01ymkBojt3B3QMvP4wTFxDjnK6P b7VDmtVSB6VX1IoFHuFuI6DgKoHgHNNgPYD5rnq0w5Teil05dolnH0moZlO0+sKM23Ol hmXCxB41gw3XpNzms/ws4RayJif6enu5jqDfuNtWw2oh7+I2+EYU+W6ZXJxy9bhaSocm ySgziNAWhRJ3If11A876NUnTPz/DG+vwlwfUhS1LuuizcEvXMGGbm10C7gaUe2wCTopN y2iQ== X-Gm-Message-State: AKaTC03vg0zuHLYxsFPkouD1LV1zW68SgEmwHX5mkig/AzbPHkDxqmOLTz8AC59GKtgyGQ== X-Received: by 10.28.54.3 with SMTP id d3mr378390wma.34.1481323382161; Fri, 09 Dec 2016 14:43:02 -0800 (PST) Received: from [192.168.2.21] (p5B0958CE.dip0.t-ipconnect.de. [91.9.88.206]) by smtp.googlemail.com with ESMTPSA id k11sm22721590wmb.18.2016.12.09.14.43.01 for (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 09 Dec 2016 14:43:01 -0800 (PST) From: Andreas Cadhalpun X-Google-Original-From: Andreas Cadhalpun To: ffmpeg-devel@ffmpeg.org References: <20161209142329.GG4749@nb4> Message-ID: Date: Fri, 9 Dec 2016 23:42:59 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Icedove/45.4.0 MIME-Version: 1.0 In-Reply-To: <20161209142329.GG4749@nb4> Subject: Re: [FFmpeg-devel] [PATCH 1/2] opus_parser: fix leaking channel_maps on error X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" 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 >> --- >> 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 From 2edf5f9a571d483858928dd6bb9e84031ef8a391 Mon Sep 17 00:00:00 2001 From: Andreas Cadhalpun 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 --- libavcodec/opus.c | 1 + libavcodec/opusdec.c | 1 - 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/libavcodec/opus.c b/libavcodec/opus.c index 08d94b6..1eeb92c 100644 --- a/libavcodec/opus.c +++ b/libavcodec/opus.c @@ -403,6 +403,7 @@ av_cold int ff_opus_parse_extradata(AVCodecContext *avctx, } else if (idx >= streams + stereo_streams) { av_log(avctx, AV_LOG_ERROR, "Invalid channel map for output channel %d: %d\n", i, idx); + av_freep(&s->channel_maps); return AVERROR_INVALIDDATA; } diff --git a/libavcodec/opusdec.c b/libavcodec/opusdec.c index ec793c6..329f784 100644 --- a/libavcodec/opusdec.c +++ b/libavcodec/opusdec.c @@ -646,7 +646,6 @@ static av_cold int opus_decode_init(AVCodecContext *avctx) /* find out the channel configuration */ ret = ff_opus_parse_extradata(avctx, c); if (ret < 0) { - av_freep(&c->channel_maps); av_freep(&c->fdsp); return ret; } -- 2.10.2