From patchwork Fri Oct 25 01:27:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 15939 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id 55EEC44698D for ; Fri, 25 Oct 2019 04:28:01 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 3129E68B0B4; Fri, 25 Oct 2019 04:28:01 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wm1-f66.google.com (mail-wm1-f66.google.com [209.85.128.66]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id CD2FE68AC62 for ; Fri, 25 Oct 2019 04:27:54 +0300 (EEST) Received: by mail-wm1-f66.google.com with SMTP id r141so297962wme.4 for ; Thu, 24 Oct 2019 18:27:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=k98w3UOciIrR4s/yG8evkEoQIPJ5yPpcQHLtzJ8nUUU=; b=uHzsrKk4ErO8eillRVEfOubpjhSAMgbin0zgZcPONAfJpQBAemDQMxbwgkY1BzLjQy FWqRzayueE4z3PISm/FbClprfe+LVI7vveIxk4X8HexSIBPwSzeqCVn6uMnsBdpT29LI yxp8wIBTvXqJqlHMQnCHa+jBGJfn/Obrf0tRenztK8UVUz4mm+SCG0NXi6uVv6N0K/m+ ncDe7nPb8yZwAqUjfj5dpJaUxwy4GEWK50bZNO4SBsJt1z6nMPHXIHcCQz5phAL0SoqG kShqCaDkC4RXxBhq1yzTVL9+uEC/gWWRWNvuA14wfiqoA9HAylNPyCEyzCex30cNiNK7 9G4A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=k98w3UOciIrR4s/yG8evkEoQIPJ5yPpcQHLtzJ8nUUU=; b=QCP436Hn88E9puzkITZcAuGj+ejpE/b6q+jiHUaBhBzuIczF9FFuXonFG56Fkywdzt y4/OxKe0Os33HJ+tyxR7Aahwxj40eGHpA7Xp4JksRdbGcQDB/+7WUjstHAr43vHDmVTP 52Bylqamc2zYC0qG2WqYjN/cT0Xs9Ol5nV9NqwtHGYpxlnhrZrk+kRLC20lh/8CeGCOA he4V3SkRrFEHBpIZTn/NS4eiu7bG9wkF6DdrYEbtFiOUJW+oIyzFK4SzNDJ8kFJz88mY 7ZcjInUgGcQuU4+rYWxOINyELwkOSKNvVeKvk0n8r8dll/e1blZ78UQ5V+w3+6syrDVV N54A== X-Gm-Message-State: APjAAAUqZ4akunLXbjgsAD2NHagu9aGcDUtDleqA0REnqnqwDhxAm5H4 7No74EmSyCqDe93OmKszjllF1cd0 X-Google-Smtp-Source: APXvYqzfdGkrHTl2Pu1x4eNAHrQMpWGtKZuVSWcd9U0s1Ifli0f26hF8ZFwU9+MlQOj0qZnc+14fgA== X-Received: by 2002:a05:600c:21d3:: with SMTP id x19mr757303wmj.121.1571966874000; Thu, 24 Oct 2019 18:27:54 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08937.dynamic.kabel-deutschland.de. [188.192.137.55]) by smtp.gmail.com with ESMTPSA id o4sm614841wre.91.2019.10.24.18.27.52 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 24 Oct 2019 18:27:53 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Fri, 25 Oct 2019 03:27:38 +0200 Message-Id: <20191025012739.29734-1-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191024174202.GB27353@michaelspb> References: <20191024174202.GB27353@michaelspb> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 1/2] avformat/mpeg: Don't copy or leak string in AVBPrint 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 Cc: Andreas Rheinhardt Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" vobsub_read_header() uses an AVBPrint to write a string and up until now, it collected the string stored in the AVBPrint via av_bprint_finalize(), which might involve an allocation and copy of the string. But this is unnecessary, as the lifetime of the returned string does not exceed the lifetime of the AVBPrint. So use the string in the AVBPrint directly. This also makes it possible to easily fix a memleak: In certain error situations, the string stored in the AVBPrint would not be freed (if it was dynamically allocated). This has been fixed, too. Signed-off-by: Andreas Rheinhardt --- Initializing the AVBPrint earlier has been made with an eye towards the next patch where the AVFormatContext will be freed via avformat_close_input() when copying the white-/blacklists fails. This is done even before the call to avformat_open_input(). I hope this is ok. libavformat/mpeg.c | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c index 46c59163fd..e6bc6700de 100644 --- a/libavformat/mpeg.c +++ b/libavformat/mpeg.c @@ -720,7 +720,6 @@ static int vobsub_read_header(AVFormatContext *s) int i, ret = 0, header_parsed = 0, langidx = 0; MpegDemuxContext *vobsub = s->priv_data; size_t fname_len; - char *header_str = NULL; AVBPrint header; int64_t delay = 0; AVStream *st = NULL; @@ -733,8 +732,7 @@ static int vobsub_read_header(AVFormatContext *s) char *ext; vobsub->sub_name = av_strdup(s->url); if (!vobsub->sub_name) { - ret = AVERROR(ENOMEM); - goto end; + return AVERROR(ENOMEM); } fname_len = strlen(vobsub->sub_name); @@ -742,24 +740,23 @@ static int vobsub_read_header(AVFormatContext *s) if (fname_len < 4 || *(ext - 1) != '.') { av_log(s, AV_LOG_ERROR, "The input index filename is too short " "to guess the associated .SUB file\n"); - ret = AVERROR_INVALIDDATA; - goto end; + return AVERROR_INVALIDDATA; } memcpy(ext, !strncmp(ext, "IDX", 3) ? "SUB" : "sub", 3); av_log(s, AV_LOG_VERBOSE, "IDX/SUB: %s -> %s\n", s->url, vobsub->sub_name); } if (!(iformat = av_find_input_format("mpeg"))) { - ret = AVERROR_DEMUXER_NOT_FOUND; - goto end; + return AVERROR_DEMUXER_NOT_FOUND; } vobsub->sub_ctx = avformat_alloc_context(); if (!vobsub->sub_ctx) { - ret = AVERROR(ENOMEM); - goto end; + return AVERROR(ENOMEM); } + av_bprint_init(&header, 0, INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE); + if ((ret = ff_copy_whiteblacklists(vobsub->sub_ctx, s)) < 0) goto end; @@ -769,7 +766,6 @@ static int vobsub_read_header(AVFormatContext *s) goto end; } - av_bprint_init(&header, 0, INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE); while (!avio_feof(s->pb)) { char line[MAX_LINE_SIZE]; int len = ff_get_line(s->pb, line, sizeof(line)); @@ -890,22 +886,20 @@ static int vobsub_read_header(AVFormatContext *s) } if (!av_bprint_is_complete(&header)) { - av_bprint_finalize(&header, NULL); ret = AVERROR(ENOMEM); goto end; } - av_bprint_finalize(&header, &header_str); for (i = 0; i < s->nb_streams; i++) { AVCodecParameters *par = s->streams[i]->codecpar; ret = ff_alloc_extradata(par, header.len); if (ret < 0) { goto end; } - memcpy(par->extradata, header_str, header.len); + memcpy(par->extradata, header.str, header.len); } end: - av_free(header_str); + av_bprint_finalize(&header, NULL); return ret; }