Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
2018-02-13 8:51 GMT+01:00, Gang Fan(范刚) : > Here is the patch. Ping. If this gets applied, please mention ticket #7019. Thank you, Carl Eugen ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
Here is the patch. On Tue, Feb 13, 2018 at 3:30 PM, Gang Fan(范刚) wrote: > Thanks for the advice, let me have another try. > > On Tue, Feb 13, 2018 at 4:59 AM, wm4 wrote: > >> On Mon, 12 Feb 2018 20:56:25 +0800 >> Gang Fan(范刚) wrote: >> >> > Thanks to Hendrik >> > Here is the new patch: >> > >> > From 642a413080f20f9515321e42056248e86e003997 Mon Sep 17 00:00:00 2001 >> > From: Fan Gang >> > Date: Mon, 12 Feb 2018 20:55:06 +0800 >> > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc >> fails. >> > >> > --- >> > libavcodec/ass_split.c | 6 ++ >> > 1 file changed, 2 insertions(+), 4 deletions(-) >> > >> > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c >> > index 872528b..eebe239 100644 >> > --- a/libavcodec/ass_split.c >> > +++ b/libavcodec/ass_split.c >> > @@ -249,7 +249,7 @@ static const char *ass_split_section(ASSSplitCon >> text >> > *ctx, const char *buf) >> > const ASSSection *section = &ass_sections[ctx->current_section]; >> > int *number = &ctx->field_number[ctx->current_section]; >> > int *order = ctx->field_order[ctx->current_section]; >> > -int *tmp, i, len; >> > +int i, len; >> > >> > while (buf && *buf) { >> > if (buf[0] == '[') { >> > @@ -280,9 +280,7 @@ static const char *ass_split_section(ASSSplitCon >> text >> > *ctx, const char *buf) >> > while (!is_eol(*buf)) { >> > buf = skip_space(buf); >> > len = strcspn(buf, ", \r\n"); >> > -if (!(tmp = av_realloc_array(order, (*number + 1), >> > sizeof(*order >> > -return NULL; >> > -order = tmp; >> > +av_reallocp_array(&order, (*number + 1), >> > sizeof(*order)); >> > order[*number] = -1; >> > for (i=0; section->fields[i].name; i++) >> > if (!strncmp(buf, section->fields[i].name, >> len)) { >> >> The patch formatting is broken (line breaks). It will be cumbersome to >> apply it, which most likely will mean nobody is going to try. >> >> Never copy&paste a patch into the text field of your email client. >> Instead you should do one of these things: >> >> - just attach the patch as text attachment >> - use git send-email >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > > > -- > > > *With kind regards* > -- *With kind regards* From 5c66208ebdf94a854fdae823e58b164f2f8056b1 Mon Sep 17 00:00:00 2001 From: Fan Gang Date: Tue, 13 Feb 2018 15:38:59 +0800 Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc fails. --- libavcodec/ass_split.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c index 872528b..67da7c6 100644 --- a/libavcodec/ass_split.c +++ b/libavcodec/ass_split.c @@ -249,7 +249,7 @@ static const char *ass_split_section(ASSSplitContext *ctx, const char *buf) const ASSSection *section = &ass_sections[ctx->current_section]; int *number = &ctx->field_number[ctx->current_section]; int *order = ctx->field_order[ctx->current_section]; -int *tmp, i, len; +int i, len; while (buf && *buf) { if (buf[0] == '[') { @@ -280,9 +280,9 @@ static const char *ass_split_section(ASSSplitContext *ctx, const char *buf) while (!is_eol(*buf)) { buf = skip_space(buf); len = strcspn(buf, ", \r\n"); -if (!(tmp = av_realloc_array(order, (*number + 1), sizeof(*order +if (av_reallocp_array(&order, (*number + 1), sizeof(*order)) != 0) return NULL; -order = tmp; + order[*number] = -1; for (i=0; section->fields[i].name; i++) if (!strncmp(buf, section->fields[i].name, len)) { -- 1.9.1 ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
Thanks for the advice, let me have another try. On Tue, Feb 13, 2018 at 4:59 AM, wm4 wrote: > On Mon, 12 Feb 2018 20:56:25 +0800 > Gang Fan(范刚) wrote: > > > Thanks to Hendrik > > Here is the new patch: > > > > From 642a413080f20f9515321e42056248e86e003997 Mon Sep 17 00:00:00 2001 > > From: Fan Gang > > Date: Mon, 12 Feb 2018 20:55:06 +0800 > > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc > fails. > > > > --- > > libavcodec/ass_split.c | 6 ++ > > 1 file changed, 2 insertions(+), 4 deletions(-) > > > > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c > > index 872528b..eebe239 100644 > > --- a/libavcodec/ass_split.c > > +++ b/libavcodec/ass_split.c > > @@ -249,7 +249,7 @@ static const char *ass_split_section(ASSSplitContext > > *ctx, const char *buf) > > const ASSSection *section = &ass_sections[ctx->current_section]; > > int *number = &ctx->field_number[ctx->current_section]; > > int *order = ctx->field_order[ctx->current_section]; > > -int *tmp, i, len; > > +int i, len; > > > > while (buf && *buf) { > > if (buf[0] == '[') { > > @@ -280,9 +280,7 @@ static const char *ass_split_section(ASSSplitContext > > *ctx, const char *buf) > > while (!is_eol(*buf)) { > > buf = skip_space(buf); > > len = strcspn(buf, ", \r\n"); > > -if (!(tmp = av_realloc_array(order, (*number + 1), > > sizeof(*order > > -return NULL; > > -order = tmp; > > +av_reallocp_array(&order, (*number + 1), > > sizeof(*order)); > > order[*number] = -1; > > for (i=0; section->fields[i].name; i++) > > if (!strncmp(buf, section->fields[i].name, > len)) { > > The patch formatting is broken (line breaks). It will be cumbersome to > apply it, which most likely will mean nobody is going to try. > > Never copy&paste a patch into the text field of your email client. > Instead you should do one of these things: > > - just attach the patch as text attachment > - use git send-email > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > -- *With kind regards* ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
On Mon, 12 Feb 2018 20:56:25 +0800 Gang Fan(范刚) wrote: > Thanks to Hendrik > Here is the new patch: > > From 642a413080f20f9515321e42056248e86e003997 Mon Sep 17 00:00:00 2001 > From: Fan Gang > Date: Mon, 12 Feb 2018 20:55:06 +0800 > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc fails. > > --- > libavcodec/ass_split.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c > index 872528b..eebe239 100644 > --- a/libavcodec/ass_split.c > +++ b/libavcodec/ass_split.c > @@ -249,7 +249,7 @@ static const char *ass_split_section(ASSSplitContext > *ctx, const char *buf) > const ASSSection *section = &ass_sections[ctx->current_section]; > int *number = &ctx->field_number[ctx->current_section]; > int *order = ctx->field_order[ctx->current_section]; > -int *tmp, i, len; > +int i, len; > > while (buf && *buf) { > if (buf[0] == '[') { > @@ -280,9 +280,7 @@ static const char *ass_split_section(ASSSplitContext > *ctx, const char *buf) > while (!is_eol(*buf)) { > buf = skip_space(buf); > len = strcspn(buf, ", \r\n"); > -if (!(tmp = av_realloc_array(order, (*number + 1), > sizeof(*order > -return NULL; > -order = tmp; > +av_reallocp_array(&order, (*number + 1), > sizeof(*order)); > order[*number] = -1; > for (i=0; section->fields[i].name; i++) > if (!strncmp(buf, section->fields[i].name, len)) { The patch formatting is broken (line breaks). It will be cumbersome to apply it, which most likely will mean nobody is going to try. Never copy&paste a patch into the text field of your email client. Instead you should do one of these things: - just attach the patch as text attachment - use git send-email ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
On Mon, Feb 12, 2018 at 1:56 PM, Gang Fan(范刚) wrote: > Thanks to Hendrik > Here is the new patch: > > From 642a413080f20f9515321e42056248e86e003997 Mon Sep 17 00:00:00 2001 > From: Fan Gang > Date: Mon, 12 Feb 2018 20:55:06 +0800 > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc fails. > > --- > libavcodec/ass_split.c | 6 ++ > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c > index 872528b..eebe239 100644 > --- a/libavcodec/ass_split.c > +++ b/libavcodec/ass_split.c > @@ -249,7 +249,7 @@ static const char *ass_split_section(ASSSplitContext > *ctx, const char *buf) > const ASSSection *section = &ass_sections[ctx->current_section]; > int *number = &ctx->field_number[ctx->current_section]; > int *order = ctx->field_order[ctx->current_section]; > -int *tmp, i, len; > +int i, len; > > while (buf && *buf) { > if (buf[0] == '[') { > @@ -280,9 +280,7 @@ static const char *ass_split_section(ASSSplitContext > *ctx, const char *buf) > while (!is_eol(*buf)) { > buf = skip_space(buf); > len = strcspn(buf, ", \r\n"); > -if (!(tmp = av_realloc_array(order, (*number + 1), > sizeof(*order > -return NULL; > -order = tmp; > +av_reallocp_array(&order, (*number + 1), > sizeof(*order)); > order[*number] = -1; > for (i=0; section->fields[i].name; i++) > if (!strncmp(buf, section->fields[i].name, len)) { > -- > 1.9.1 > > Allocation can still fail, so you shouldn't remove the check - just need to change it, since it returns 0 for success, negative for failure. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
Thanks to Hendrik Here is the new patch: From 642a413080f20f9515321e42056248e86e003997 Mon Sep 17 00:00:00 2001 From: Fan Gang Date: Mon, 12 Feb 2018 20:55:06 +0800 Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc fails. --- libavcodec/ass_split.c | 6 ++ 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c index 872528b..eebe239 100644 --- a/libavcodec/ass_split.c +++ b/libavcodec/ass_split.c @@ -249,7 +249,7 @@ static const char *ass_split_section(ASSSplitContext *ctx, const char *buf) const ASSSection *section = &ass_sections[ctx->current_section]; int *number = &ctx->field_number[ctx->current_section]; int *order = ctx->field_order[ctx->current_section]; -int *tmp, i, len; +int i, len; while (buf && *buf) { if (buf[0] == '[') { @@ -280,9 +280,7 @@ static const char *ass_split_section(ASSSplitContext *ctx, const char *buf) while (!is_eol(*buf)) { buf = skip_space(buf); len = strcspn(buf, ", \r\n"); -if (!(tmp = av_realloc_array(order, (*number + 1), sizeof(*order -return NULL; -order = tmp; +av_reallocp_array(&order, (*number + 1), sizeof(*order)); order[*number] = -1; for (i=0; section->fields[i].name; i++) if (!strncmp(buf, section->fields[i].name, len)) { -- 1.9.1 On Mon, Feb 12, 2018 at 8:32 PM, Gang Fan(范刚) wrote: > OK, should I email the new patch to the same thread or a new thread? > > Thanks > Gang > > On Mon, Feb 12, 2018 at 7:49 PM, Hendrik Leppkes > wrote: > >> On Mon, Feb 12, 2018 at 11:55 AM, Gang Fan(范刚) >> wrote: >> > There is a potential memory leak bug in file ass_split.c, here is the >> > description. >> > >> > A piece of memory is allocated on line 283. When executing the loop >> twice >> > and if the av_realloc_array returns null the function returns without >> > freeing the memory pointed by order. >> > >> > Suggested fix: >> > free(order) before return NULL; on line 284 >> > >> > Reference Ticket: https://trac.ffmpeg.org/ticket/7019#comment:1 >> > >> > Thanks >> > Gang >> > Sbrella >> > >> > >> > From 6850fc3a6562b4f5fb92e72eed125e057ad975ae Mon Sep 17 00:00:00 2001 >> > From: Fan Gang >> > Date: Mon, 12 Feb 2018 18:46:20 +0800 >> > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc >> fails. >> > >> > --- >> > libavcodec/ass_split.c | 4 +++- >> > 1 file changed, 3 insertions(+), 1 deletion(-) >> > >> > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c >> > index 872528b..c7eb07d 100644 >> > --- a/libavcodec/ass_split.c >> > +++ b/libavcodec/ass_split.c >> > @@ -280,8 +280,10 @@ static const char *ass_split_section(ASSSplitCon >> text >> > *ctx, const char *buf) >> > while (!is_eol(*buf)) { >> > buf = skip_space(buf); >> > len = strcspn(buf, ", \r\n"); >> > -if (!(tmp = av_realloc_array(order, (*number + 1), >> > sizeof(*order >> > +if (!(tmp = av_realloc_array(order, (*number + 1), >> > sizeof(*order{ >> > +free(order); >> > return NULL; >> > +} >> > order = tmp; >> > order[*number] = -1; >> > for (i=0; section->fields[i].name; i++) >> > -- >> >> You would need to use av_free instead of free. However, a better >> option would be just using av_reallocp_array, which automatically >> frees the original pointer on failure. >> >> - Hendrik >> ___ >> ffmpeg-devel mailing list >> ffmpeg-devel@ffmpeg.org >> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >> > > > > -- > > > *With kind regards* > -- *With kind regards* ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
OK, should I email the new patch to the same thread or a new thread? Thanks Gang On Mon, Feb 12, 2018 at 7:49 PM, Hendrik Leppkes wrote: > On Mon, Feb 12, 2018 at 11:55 AM, Gang Fan(范刚) > wrote: > > There is a potential memory leak bug in file ass_split.c, here is the > > description. > > > > A piece of memory is allocated on line 283. When executing the loop twice > > and if the av_realloc_array returns null the function returns without > > freeing the memory pointed by order. > > > > Suggested fix: > > free(order) before return NULL; on line 284 > > > > Reference Ticket: https://trac.ffmpeg.org/ticket/7019#comment:1 > > > > Thanks > > Gang > > Sbrella > > > > > > From 6850fc3a6562b4f5fb92e72eed125e057ad975ae Mon Sep 17 00:00:00 2001 > > From: Fan Gang > > Date: Mon, 12 Feb 2018 18:46:20 +0800 > > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc > fails. > > > > --- > > libavcodec/ass_split.c | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > > > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c > > index 872528b..c7eb07d 100644 > > --- a/libavcodec/ass_split.c > > +++ b/libavcodec/ass_split.c > > @@ -280,8 +280,10 @@ static const char *ass_split_section( > ASSSplitContext > > *ctx, const char *buf) > > while (!is_eol(*buf)) { > > buf = skip_space(buf); > > len = strcspn(buf, ", \r\n"); > > -if (!(tmp = av_realloc_array(order, (*number + 1), > > sizeof(*order > > +if (!(tmp = av_realloc_array(order, (*number + 1), > > sizeof(*order{ > > +free(order); > > return NULL; > > +} > > order = tmp; > > order[*number] = -1; > > for (i=0; section->fields[i].name; i++) > > -- > > You would need to use av_free instead of free. However, a better > option would be just using av_reallocp_array, which automatically > frees the original pointer on failure. > > - Hendrik > ___ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel > -- *With kind regards* ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] avcodec/ass: Fix a memory leak defect.
On Mon, Feb 12, 2018 at 11:55 AM, Gang Fan(范刚) wrote: > There is a potential memory leak bug in file ass_split.c, here is the > description. > > A piece of memory is allocated on line 283. When executing the loop twice > and if the av_realloc_array returns null the function returns without > freeing the memory pointed by order. > > Suggested fix: > free(order) before return NULL; on line 284 > > Reference Ticket: https://trac.ffmpeg.org/ticket/7019#comment:1 > > Thanks > Gang > Sbrella > > > From 6850fc3a6562b4f5fb92e72eed125e057ad975ae Mon Sep 17 00:00:00 2001 > From: Fan Gang > Date: Mon, 12 Feb 2018 18:46:20 +0800 > Subject: [PATCH] avcodec/ass: Fix a memory leak defect when realloc fails. > > --- > libavcodec/ass_split.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/libavcodec/ass_split.c b/libavcodec/ass_split.c > index 872528b..c7eb07d 100644 > --- a/libavcodec/ass_split.c > +++ b/libavcodec/ass_split.c > @@ -280,8 +280,10 @@ static const char *ass_split_section(ASSSplitContext > *ctx, const char *buf) > while (!is_eol(*buf)) { > buf = skip_space(buf); > len = strcspn(buf, ", \r\n"); > -if (!(tmp = av_realloc_array(order, (*number + 1), > sizeof(*order > +if (!(tmp = av_realloc_array(order, (*number + 1), > sizeof(*order{ > +free(order); > return NULL; > +} > order = tmp; > order[*number] = -1; > for (i=0; section->fields[i].name; i++) > -- You would need to use av_free instead of free. However, a better option would be just using av_reallocp_array, which automatically frees the original pointer on failure. - Hendrik ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel