diff mbox

[FFmpeg-devel,v2] avformat/hls: release mem resource to fix memleak

Message ID 20171231055017.49285-1-ffmpeg@tmm1.net
State Accepted
Commit 2906363d1bb804994cf741ad2c99d8c76efff5f0
Headers show

Commit Message

Aman Karmani Dec. 31, 2017, 5:50 a.m. UTC 
From: Steven Liu <lq@chinaffmpeg.org>

fix CID: 1426991

Signed-off-by: Steven Liu <lq@chinaffmpeg.org>
Signed-off-by: Aman Gupta <aman@tmm1.net>
---
 libavformat/hls.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Comments

Derek Buitenhuis Dec. 31, 2017, 6:19 a.m. UTC | #1 
On 12/31/2017 5:50 AM, Aman Gupta wrote:
> +        int r = av_opt_get(v->input, "http_version", AV_OPT_SEARCH_CHILDREN, &http_version_opt);
> +        if (r >= 0) {
> +            c->http_multiple = strncmp((const char *)http_version_opt, "1.1", 3) == 0;
> +            av_freep(&http_version_opt);
> +        }

I mean, we should probably actually fail on ENOMEM, but I guess av_opt_get is
poorly designed in the sense that it can return fatal and nonfatal sorts of
errors, so I guess this gets no complaints from me.

Looking closer into the implementation of av_opt_get just kind of made me sad,
though. It has a bunch of unchecked allocations, such that it can return 0 but
have the string be NULL. I'll send a patch tomorrow to address that properly.
(Yes, I strongly believe handling allocation failures is still relevant in 2017...)

- Derek
Derek Buitenhuis Dec. 31, 2017, 10:42 p.m. UTC | #2 
On 12/31/2017 6:19 AM, Derek Buitenhuis wrote:
> Looking closer into the implementation of av_opt_get just kind of made me sad,
> though. It has a bunch of unchecked allocations, such that it can return 0 but
> have the string be NULL. I'll send a patch tomorrow to address that properly.

Looks like this was already fixed, and I had an old copy of the code open in a tab.

- Derek
diff mbox

Patch

diff --git a/libavformat/hls.c b/libavformat/hls.c
index d9f7c6de4d..950cc4c3bd 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -1473,8 +1473,11 @@  reload:
 
     if (c->http_multiple == -1) {
         uint8_t *http_version_opt = NULL;
-        av_opt_get(v->input, "http_version", AV_OPT_SEARCH_CHILDREN, &http_version_opt);
-        c->http_multiple = http_version_opt && strncmp((const char *)http_version_opt, "1.1", 3) == 0;
+        int r = av_opt_get(v->input, "http_version", AV_OPT_SEARCH_CHILDREN, &http_version_opt);
+        if (r >= 0) {
+            c->http_multiple = strncmp((const char *)http_version_opt, "1.1", 3) == 0;
+            av_freep(&http_version_opt);
+        }
     }
 
     seg = next_segment(v);