Re: [FFmpeg-devel] [PATCH 2/3] avformat/mpeg: Don't copy or leak string in AVBPrint

2019-12-05 Thread Andreas Rheinhardt
Michael Niedermayer:
> On Wed, Dec 04, 2019 at 01:37:13PM +0100, Andreas Rheinhardt wrote:
>> 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 
>> ---
>> Supersedes https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/252074.html
>> Resending because of merge conflict.
> 
> will apply
> 
> thx
> 
Thanks. During checking whether my patches work I have also found a
use of uninitialized values in vobsub_read_packet() [1]; and earlier I
have found a way to eliminate the secondary packet when reading VobSub
[2]. Could you look over them, too?

- Andreas

[1]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/251961.html
[2]: https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/251146.html

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/mpeg: Don't copy or leak string in AVBPrint

2019-12-05 Thread Michael Niedermayer
On Wed, Dec 04, 2019 at 01:37:13PM +0100, Andreas Rheinhardt wrote:
> 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 
> ---
> Supersedes https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/252074.html
> Resending because of merge conflict.

will apply

thx

[...]
-- 
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus


signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] avformat/mpeg: Don't copy or leak string in AVBPrint

2019-12-04 Thread Andreas Rheinhardt
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 
---
Supersedes https://ffmpeg.org/pipermail/ffmpeg-devel/2019-October/252074.html
Resending because of merge conflict.

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

diff --git a/libavformat/mpeg.c b/libavformat/mpeg.c
index 80983f8a81..e4fe16c7d2 100644
--- a/libavformat/mpeg.c
+++ b/libavformat/mpeg.c
@@ -718,7 +718,6 @@ static int vobsub_read_header(AVFormatContext *s)
 int i, ret = 0, header_parsed = 0, langidx = 0;
 VobSubDemuxContext *vobsub = s->priv_data;
 size_t fname_len;
-char *header_str = NULL;
 AVBPrint header;
 int64_t delay = 0;
 AVStream *st = NULL;
@@ -731,8 +730,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);
@@ -740,24 +738,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(, 0, INT_MAX - AV_INPUT_BUFFER_PADDING_SIZE);
+
 if ((ret = ff_copy_whiteblacklists(vobsub->sub_ctx, s)) < 0)
 goto end;
 
@@ -767,7 +764,6 @@ static int vobsub_read_header(AVFormatContext *s)
 goto end;
 }
 
-av_bprint_init(, 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));
@@ -888,22 +884,20 @@ static int vobsub_read_header(AVFormatContext *s)
 }
 
 if (!av_bprint_is_complete()) {
-av_bprint_finalize(, NULL);
 ret = AVERROR(ENOMEM);
 goto end;
 }
-av_bprint_finalize(, _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(, NULL);
 return ret;
 }
 
-- 
2.20.1

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".