* [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: fix memory leak in ffmpeg_mux_init.c
@ 2024-04-12 16:44 LuMingYin
2024-04-12 19:11 ` James Almer
0 siblings, 1 reply; 4+ messages in thread
From: LuMingYin @ 2024-04-12 16:44 UTC (permalink / raw)
To: ffmpeg-devel; +Cc: lumingyindetect
Signed-off-by: LuMingYin <lumingyindetect@163.com>
---
fftools/ffmpeg_mux_init.c | 7 ++++---
libavutil/mem.c | 5 ++++-
2 files changed, 8 insertions(+), 4 deletions(-)
diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
index 6d8bd5bcdf..e7e2281bd0 100644
--- a/fftools/ffmpeg_mux_init.c
+++ b/fftools/ffmpeg_mux_init.c
@@ -2851,9 +2851,10 @@ static int parse_forced_key_frames(void *log, KeyframeForceCtx *kf,
if (nb_ch > INT_MAX - size ||
!(pts = av_realloc_f(pts, size += nb_ch - 1,
- sizeof(*pts))))
- return AVERROR(ENOMEM);
-
+ sizeof(*pts)))) {
+ ret = AVERROR(ENOMEM);
+ goto fail;
+ }
if (p[8]) {
ret = av_parse_time(&t, p + 8, 1);
if (ret < 0) {
diff --git a/libavutil/mem.c b/libavutil/mem.c
index b205d3fb25..7f34765fe7 100644
--- a/libavutil/mem.c
+++ b/libavutil/mem.c
@@ -177,11 +177,14 @@ void *av_realloc_f(void *ptr, size_t nelem, size_t elsize)
if (size_mult(elsize, nelem, &size)) {
av_free(ptr);
+ ptr = NULL;
return NULL;
}
r = av_realloc(ptr, size);
- if (!r)
+ if (!r) {
av_free(ptr);
+ ptr = NULL;
+ }
return r;
}
--
2.25.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-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: fix memory leak in ffmpeg_mux_init.c
2024-04-12 16:44 [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: fix memory leak in ffmpeg_mux_init.c LuMingYin
@ 2024-04-12 19:11 ` James Almer
2024-04-13 2:42 ` lumingyindetect
2024-04-13 2:44 ` lumingyindetect
0 siblings, 2 replies; 4+ messages in thread
From: James Almer @ 2024-04-12 19:11 UTC (permalink / raw)
To: ffmpeg-devel
On 4/12/2024 1:44 PM, LuMingYin wrote:
> Signed-off-by: LuMingYin <lumingyindetect@163.com>
> ---
> fftools/ffmpeg_mux_init.c | 7 ++++---
> libavutil/mem.c | 5 ++++-
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> index 6d8bd5bcdf..e7e2281bd0 100644
> --- a/fftools/ffmpeg_mux_init.c
> +++ b/fftools/ffmpeg_mux_init.c
> @@ -2851,9 +2851,10 @@ static int parse_forced_key_frames(void *log, KeyframeForceCtx *kf,
>
> if (nb_ch > INT_MAX - size ||
> !(pts = av_realloc_f(pts, size += nb_ch - 1,
> - sizeof(*pts))))
> - return AVERROR(ENOMEM);
> -
> + sizeof(*pts)))) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> if (p[8]) {
> ret = av_parse_time(&t, p + 8, 1);
> if (ret < 0) {
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index b205d3fb25..7f34765fe7 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -177,11 +177,14 @@ void *av_realloc_f(void *ptr, size_t nelem, size_t elsize)
>
> if (size_mult(elsize, nelem, &size)) {
> av_free(ptr);
> + ptr = NULL;
> return NULL;
> }
> r = av_realloc(ptr, size);
> - if (!r)
> + if (!r) {
> av_free(ptr);
> + ptr = NULL;
> + }
This doesn't do what you think it does. ptr is a local variable in this
scope. Setting it to NULL will not be reflected in ffmpeg_mux_init.c
And even if it did, it would be an unexpected behavior change in a
public API function.
> return r;
> }
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: fix memory leak in ffmpeg_mux_init.c
2024-04-12 19:11 ` James Almer
@ 2024-04-13 2:42 ` lumingyindetect
2024-04-13 2:44 ` lumingyindetect
1 sibling, 0 replies; 4+ messages in thread
From: lumingyindetect @ 2024-04-13 2:42 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Thank you for your patient explanation! As you pointed out, I made unnecessary modifications in the public functions in the previous patch. Now, I have rectified them and created a new patch. The patch link is:https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240413023726.1843852-1-lumingyindetect@163.com/
在 2024-04-13 03:11:22，"James Almer" <jamrial@gmail.com> 写道：
On 4/12/2024 1:44 PM, LuMingYin wrote:
> Signed-off-by: LuMingYin <lumingyindetect@163.com>
> ---
> fftools/ffmpeg_mux_init.c | 7 ++++---
> libavutil/mem.c | 5 ++++-
> 2 files changed, 8 insertions(+), 4 deletions(-)
>
> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
> index 6d8bd5bcdf..e7e2281bd0 100644
> --- a/fftools/ffmpeg_mux_init.c
> +++ b/fftools/ffmpeg_mux_init.c
> @@ -2851,9 +2851,10 @@ static int parse_forced_key_frames(void *log, KeyframeForceCtx *kf,
>
> if (nb_ch > INT_MAX - size ||
> !(pts = av_realloc_f(pts, size += nb_ch - 1,
> - sizeof(*pts))))
> - return AVERROR(ENOMEM);
> -
> + sizeof(*pts)))) {
> + ret = AVERROR(ENOMEM);
> + goto fail;
> + }
> if (p[8]) {
> ret = av_parse_time(&t, p + 8, 1);
> if (ret < 0) {
> diff --git a/libavutil/mem.c b/libavutil/mem.c
> index b205d3fb25..7f34765fe7 100644
> --- a/libavutil/mem.c
> +++ b/libavutil/mem.c
> @@ -177,11 +177,14 @@ void *av_realloc_f(void *ptr, size_t nelem, size_t elsize)
>
> if (size_mult(elsize, nelem, &size)) {
> av_free(ptr);
> + ptr = NULL;
> return NULL;
> }
> r = av_realloc(ptr, size);
> - if (!r)
> + if (!r) {
> av_free(ptr);
> + ptr = NULL;
> + }
This doesn't do what you think it does. ptr is a local variable in this
scope. Setting it to NULL will not be reflected in ffmpeg_mux_init.c
And even if it did, it would be an unexpected behavior change in a
public API function.
> return r;
> }
>
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: fix memory leak in ffmpeg_mux_init.c
2024-04-12 19:11 ` James Almer
2024-04-13 2:42 ` lumingyindetect
@ 2024-04-13 2:44 ` lumingyindetect
1 sibling, 0 replies; 4+ messages in thread
From: lumingyindetect @ 2024-04-13 2:44 UTC (permalink / raw)
To: FFmpeg development discussions and patches
Thank you for your patient explanation! As you pointed out, I made unnecessary modifications in the public functions in the previous patch. Now, I have rectified them and created a new patch. The patch link is:https://patchwork.ffmpeg.org/project/ffmpeg/patch/20240413023726.1843852-1-lumingyindetect@163.com/
At 2024-04-13 03:11:22, "James Almer" <jamrial@gmail.com> wrote:
>On 4/12/2024 1:44 PM, LuMingYin wrote:
>> Signed-off-by: LuMingYin <lumingyindetect@163.com>
>> ---
>> fftools/ffmpeg_mux_init.c | 7 ++++---
>> libavutil/mem.c | 5 ++++-
>> 2 files changed, 8 insertions(+), 4 deletions(-)
>>
>> diff --git a/fftools/ffmpeg_mux_init.c b/fftools/ffmpeg_mux_init.c
>> index 6d8bd5bcdf..e7e2281bd0 100644
>> --- a/fftools/ffmpeg_mux_init.c
>> +++ b/fftools/ffmpeg_mux_init.c
>> @@ -2851,9 +2851,10 @@ static int parse_forced_key_frames(void *log, KeyframeForceCtx *kf,
>>
>> if (nb_ch > INT_MAX - size ||
>> !(pts = av_realloc_f(pts, size += nb_ch - 1,
>> - sizeof(*pts))))
>> - return AVERROR(ENOMEM);
>> -
>> + sizeof(*pts)))) {
>> + ret = AVERROR(ENOMEM);
>> + goto fail;
>> + }
>> if (p[8]) {
>> ret = av_parse_time(&t, p + 8, 1);
>> if (ret < 0) {
>> diff --git a/libavutil/mem.c b/libavutil/mem.c
>> index b205d3fb25..7f34765fe7 100644
>> --- a/libavutil/mem.c
>> +++ b/libavutil/mem.c
>> @@ -177,11 +177,14 @@ void *av_realloc_f(void *ptr, size_t nelem, size_t elsize)
>>
>> if (size_mult(elsize, nelem, &size)) {
>> av_free(ptr);
>> + ptr = NULL;
>> return NULL;
>> }
>> r = av_realloc(ptr, size);
>> - if (!r)
>> + if (!r) {
>> av_free(ptr);
>> + ptr = NULL;
>> + }
>
>This doesn't do what you think it does. ptr is a local variable in this
>scope. Setting it to NULL will not be reflected in ffmpeg_mux_init.c
>And even if it did, it would be an unexpected behavior change in a
>public API function.
>
>> return r;
>> }
>>
>_______________________________________________
>ffmpeg-devel mailing list
>ffmpeg-devel@ffmpeg.org
>https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
>To unsubscribe, visit link above, or email
>ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-13 2:44 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-12 16:44 [FFmpeg-devel] [PATCH] fftools/ffmpeg_mux_init: fix memory leak in ffmpeg_mux_init.c LuMingYin
2024-04-12 19:11 ` James Almer
2024-04-13 2:42 ` lumingyindetect
2024-04-13 2:44 ` lumingyindetect
Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
This inbox may be cloned and mirrored by anyone:
git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git
# If you have public-inbox 1.1+ installed, you may
# initialize and index your mirror using the following commands:
public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
ffmpegdev@gitmailbox.com
public-inbox-index ffmpegdev
Example config snippet for mirrors.
AGPL code for this site: git clone https://public-inbox.org/public-inbox.git