Message ID | 20171231055017.49285-1-ffmpeg@tmm1.net |
---|---|
State | Accepted |
Commit | 2906363d1bb804994cf741ad2c99d8c76efff5f0 |
Headers | show |
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
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 --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);