| Message ID | 1638765904-2521-4-git-send-email-lance.lmwang@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v2,1/6] avformat/rtsp: remove redundant assignment | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Mon, 6 Dec 2021, lance.lmwang@gmail.com wrote: > From: Limin Wang <lance.lmwang@gmail.com> > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > --- > libavformat/rtsp.c | 7 ++----- > 1 file changed, 2 insertions(+), 5 deletions(-) > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > index 3e4a68a..b3d1e91 100644 > --- a/libavformat/rtsp.c > +++ b/libavformat/rtsp.c > @@ -1240,7 +1240,7 @@ start: > av_strlcpy(rt->session_id, reply->session_id, sizeof(rt->session_id)); > > content_length = reply->content_length; > - if (content_length > 0) { > + if (content_ptr && content_length > 0) { > /* leave some room for a trailing '\0' (useful for simple parsing) */ > content = av_malloc(content_length + 1); > if (!content) > @@ -1250,11 +1250,8 @@ start: > return AVERROR(EIO); > } > content[content_length] = '\0'; > - } > - if (content_ptr) > *content_ptr = content; > - else > - av_freep(&content); > + } I don't think this is right. If the reply that we read contain a body of contents, but the caller didn't pass any content_ptr, then we still need to read and consume the body of contents from the stream, even if we don't pass it to the caller. (Maybe we should warn in this case, that there was some potentially relevant data that the caller didn't care about?) But if we don't read it, like this patch does, we would end up desynced from the protocol stream. // Martin
On Tue, Dec 07, 2021 at 11:30:11AM +0200, Martin Storsjö wrote: > On Mon, 6 Dec 2021, lance.lmwang@gmail.com wrote: > > > From: Limin Wang <lance.lmwang@gmail.com> > > > > Signed-off-by: Limin Wang <lance.lmwang@gmail.com> > > --- > > libavformat/rtsp.c | 7 ++----- > > 1 file changed, 2 insertions(+), 5 deletions(-) > > > > diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c > > index 3e4a68a..b3d1e91 100644 > > --- a/libavformat/rtsp.c > > +++ b/libavformat/rtsp.c > > @@ -1240,7 +1240,7 @@ start: > > av_strlcpy(rt->session_id, reply->session_id, sizeof(rt->session_id)); > > > > content_length = reply->content_length; > > - if (content_length > 0) { > > + if (content_ptr && content_length > 0) { > > /* leave some room for a trailing '\0' (useful for simple parsing) */ > > content = av_malloc(content_length + 1); > > if (!content) > > @@ -1250,11 +1250,8 @@ start: > > return AVERROR(EIO); > > } > > content[content_length] = '\0'; > > - } > > - if (content_ptr) > > *content_ptr = content; > > - else > > - av_freep(&content); > > + } > > I don't think this is right. > > If the reply that we read contain a body of contents, but the caller didn't > pass any content_ptr, then we still need to read and consume the body of > contents from the stream, even if we don't pass it to the caller. (Maybe we > should warn in this case, that there was some potentially relevant data that > the caller didn't care about?) But if we don't read it, like this patch > does, we would end up desynced from the protocol stream. Yes, I haven't realized your valid case, ignore this change anyway. > > // Martin >
diff --git a/libavformat/rtsp.c b/libavformat/rtsp.c index 3e4a68a..b3d1e91 100644 --- a/libavformat/rtsp.c +++ b/libavformat/rtsp.c @@ -1240,7 +1240,7 @@ start: av_strlcpy(rt->session_id, reply->session_id, sizeof(rt->session_id)); content_length = reply->content_length; - if (content_length > 0) { + if (content_ptr && content_length > 0) { /* leave some room for a trailing '\0' (useful for simple parsing) */ content = av_malloc(content_length + 1); if (!content) @@ -1250,11 +1250,8 @@ start: return AVERROR(EIO); } content[content_length] = '\0'; - } - if (content_ptr) *content_ptr = content; - else - av_freep(&content); + } if (request) { char buf[MAX_URL_SIZE];