diff mbox

[FFmpeg-devel,3/5] avformat/mpeg: Avoid allocation and fix memleak I

Message ID 20191022131645.8394-3-andreas.rheinhardt@gmail.com
State New
Headers show

Commit Message

Andreas Rheinhardt Oct. 22, 2019, 1:16 p.m. UTC 
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 <andreas.rheinhardt@gmail.com>
---
I guess that this is the memleak that 0b8956b2 intended to fix!?

 libavformat/mpeg.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)
diff mbox

Patch

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;
 }