On Mon, 12 Oct 2020, Andriy Gelman wrote:
From: Andriy Gelman <andriy.gel...@gmail.com>
In sdp_read_header() some ff_network_close() calls were missed.
Also in rtp_read_header() update comment to explain why a single
call to ff_network_close() is enough to cover all cases even if
sdp_read_header() returns an error.
Signed-off-by: Andriy Gelman <andriy.gel...@gmail.com>
---
libavformat/rtsp.c | 9 +++++++--
1 file changed, 7 insertions(+), 2 deletions(-)
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c
index cb9fc31166..a8c7ec4a46 100644
--- a/libavformat/rtsp.c
+++ b/libavformat/rtsp.c
@@ -2347,11 +2347,14 @@ static int sdp_read_header(AVFormatContext *s)
/* read the whole sdp file */
/* XXX: better loading */
content = av_malloc(SDP_MAX_SIZE);
- if (!content)
+ if (!content) {
+ ff_network_close();
return AVERROR(ENOMEM);
+ }
size = avio_read(s->pb, content, SDP_MAX_SIZE - 1);
if (size <= 0) {
av_free(content);
+ ff_network_close();
return AVERROR_INVALIDDATA;
}
content[size] ='\0';
@@ -2550,7 +2553,9 @@ static int rtp_read_header(AVFormatContext *s)
ffio_init_context(&pb, sdp.str, sdp.len, 0, NULL, NULL, NULL, NULL);
s->pb = &pb;
- /* sdp_read_header initializes this again */
+ /* if sdp_read_header() fails then following ff_network_close() cancels
out */
+ /* ff_network_init() at the start of this function. Otherwise it cancels
out */
+ /* ff_network_init() inside sdp_read_header() */
ff_network_close();
rt->media_type_mask = (1 << (AVMEDIA_TYPE_SUBTITLE+1)) - 1;
--
2.28.0
Ok
Unrelatedly, as we seem to have a problem with
ff_network_init()/ff_network_close() often being mismatched and not
handled correctly, do you think it'd be good as a development aid, to make
certain network functions (ff_url_join, the low level tcp protocol etc)
error out on unix systems as well, if ff_network_init() has been missed?
And same for counting the calls to init/close to make sure they're
properly paired. Otherwise these issues only hit people on windows, where
the code probably is tested much less.
The problem, I guess, is finding the right level of obnoxity for the
error. We probably don't want such an issue to break code that otherwise
has run fine in production on unix platforms, even though it points out a
portability problem. And as far as I know, most developers don't really
build/test ffmpeg in debug mode anyway, so if the checks are hidden in
such a mode, they won't help developers notice the issues either. And we
don't have much fate coverage for the protocols.
// Martin
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
