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

Derek Buitenhuis derek.buitenhuis at gmail.com
Sun Dec 31 08:29:10 EET 2017


On 12/31/2017 5:21 AM, Aman Gupta wrote:
> I really don't think it makes any sense in this case, since it is expected
> that the av_opt_get will fail on non-http io contexts. It doesn't matter if
> the failure is due to AVERROR_OPTION_NOT_FOUND or AVERROR(ENOMEM). The only
> thing that matters is whether http_version_opt is set or remains NULL.

A behavioral change that may occur due to OOM rather than failing is not
very fun to debug.

Anyway, part of my point was that using error checks inconsistently often
leads to sloppiness elsewhere, because, well, checks are not always used,
and judgement on when to use them may not always be right, or the underlying
implementation of the unchecked function may change, such that it now does
matter (assumptions of the API being checked as per documentation, etc.).
There are long term benefits to a culture of always checking, IMO. I'm
sure people will reply to disagree with me, of course.

> Anyway, I don't feel strongly about this. If you think the extra
> instructions to store and compare the return value make this code cleaner
> or more secure, so be it. I will send a patch with the requested changes.

OK. (Fretting about extra instructions for return value checks in playlist
code is a tad silly...)

Sorry for the rant -- this sort of lax API use has been a pain in the
past when trying to change around implementation details, where API
users have neglected to check return values.

- Derek


More information about the ffmpeg-devel mailing list