From patchwork Tue Oct 22 13:16:43 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Andreas Rheinhardt X-Patchwork-Id: 15900 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 250A7448B82 for ; Tue, 22 Oct 2019 16:17:23 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 0A55E68AF1A; Tue, 22 Oct 2019 16:17:23 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id F1DF368AEF2 for ; Tue, 22 Oct 2019 16:17:14 +0300 (EEST) Received: by mail-wr1-f67.google.com with SMTP id t16so12898770wrr.1 for ; Tue, 22 Oct 2019 06:17:14 -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=CZSPQdXois7J3bS8I33AM9Qus4pXAepL3QU0ITVg/9I=; b=d0PQr0OszLNxB1Q4PYD3eeT94IoVUGaySxEX6kIY+gAnoNVvODwHFuK0ObMUrRKEEG v0qVHuOYLkivu+sDLirq+etJH5C4/toZe/t4CkKdtjNbMpPBm3x9Ty+DX0kETbnUJuRe I97aKMQJMKb8lbhX/y4yQHtMQK72KkvYqJ+9gq3gNU2z0ga3BWJwYp+kZaHYAPlVdrS2 SXCUxCsFPbFCVI/mAx29QR48DVvjPpK6KIH/DiyMxLEUDysIikMHevTlMJJPYE2gGWRO H1AuxAkoKTjRs8XKucgWptodCz3jv07QKUSwz2wl3Hc+iEs3yNJfYV7j7sDBZWVVE4Tt 2ghg== 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=CZSPQdXois7J3bS8I33AM9Qus4pXAepL3QU0ITVg/9I=; b=JXTRS97rl40mr9xtWrcO6GqjuGfggklopkdwsdeWzE2gish6yNxmQMMjl7xLe3dKhy QP/l93iXpbn/A8sTO77/m7iTIvIGd+wicPeUWL9Md3ZvI0WOB9k7Fx14QkfEH2mlwQTV KoGAdVbEAFtheFRVM4SRLMz5dObGXoKdtz4lbhtT5x8JAf8+IcTUNdbK6iaPjSbNsGn3 mbN5lkTIhSOck8eR5OlqGk8bzfT+WlwUue5T0ndoYAaotDrovYMsgYinFBGxMn20x3f9 2WAwJrntPCiY6vD1CYdh2tNoVBtUvAShjFy4WxdeEKBrgrY0ctPmfQJiFa8QQvveAR2Q Fslw== X-Gm-Message-State: APjAAAUITBqGVvtx9bz6w1XXM6y4sR3P+VQGey/lEfrRISYL9MJN6r0P nuFuJOpL1L+wfgdcGme7/WitplqX X-Google-Smtp-Source: APXvYqyKL5ZZGk080669PWdjOX3jzYJNRibrfyO4nbw0RSPHgkdGgWO5PJ3ZvJEI/p4ywfa4baaokA== X-Received: by 2002:adf:f00d:: with SMTP id j13mr3279786wro.253.1571750234221; Tue, 22 Oct 2019 06:17:14 -0700 (PDT) Received: from sblaptop.fritz.box (ipbcc08937.dynamic.kabel-deutschland.de. [188.192.137.55]) by smtp.gmail.com with ESMTPSA id o70sm25074520wme.29.2019.10.22.06.17.13 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 22 Oct 2019 06:17:13 -0700 (PDT) From: Andreas Rheinhardt To: ffmpeg-devel@ffmpeg.org Date: Tue, 22 Oct 2019 15:16:43 +0200 Message-Id: <20191022131645.8394-3-andreas.rheinhardt@gmail.com> X-Mailer: git-send-email 2.20.1 In-Reply-To: <20191022131645.8394-1-andreas.rheinhardt@gmail.com> References: <20191022131645.8394-1-andreas.rheinhardt@gmail.com> MIME-Version: 1.0 Subject: [FFmpeg-devel] [PATCH 3/5] avformat/mpeg: Avoid allocation and fix memleak I 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 --- I guess that this is the memleak that 0b8956b2 intended to fix!? 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; }