Hi, I noticed that the GET URLContext can be leaked if some of the later operations fail before assigning the handles to rt->rtsp_hd{,_out}. This patch should fix the issue. Josh, do you think this is ok, or would you rather assign the handles directly to rt->rtsp_hd without using the local variables? // Martin
Hi Martin, On 18 June 2010 00:45, Martin Storsjö <martin@martin.st> wrote:
Hi,
I noticed that the GET URLContext can be leaked if some of the later operations fail before assigning the handles to rt->rtsp_hd{,_out}. This patch should fix the issue.
Josh, do you think this is ok, or would you rather assign the handles directly to rt->rtsp_hd without using the local variables?
Doesn't the jump to fail: clean things up? If I'm missing something here, then it looks OK. Josh
// Martin _______________________________________________ FFmpeg-soc mailing list FFmpeg-soc@mplayerhq.hu https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
On Fri, 18 Jun 2010, Josh Allmann wrote:
On 18 June 2010 00:45, Martin Storsjö <martin@martin.st> wrote:
Hi,
I noticed that the GET URLContext can be leaked if some of the later operations fail before assigning the handles to rt->rtsp_hd{,_out}. This patch should fix the issue.
Josh, do you think this is ok, or would you rather assign the handles directly to rt->rtsp_hd without using the local variables?
Doesn't the jump to fail: clean things up? If I'm missing something here, then it looks OK.
No, it doesn't clean up everything. The fail: codepath only does ff_rtsp_close_connections(s), which closes rt->rtsp_hd, but you don't assign the newly opened connection handles to rt->rtsp_hd until after the whole if (tunnel) {} else { normal } block, you just keep them in local variables. // Martin
On Fri, 18 Jun 2010, Josh Allmann wrote:
On 18 June 2010 00:45, Martin Storsjö <martin@martin.st> wrote:
Hi,
I noticed that the GET URLContext can be leaked if some of the later operations fail before assigning the handles to rt->rtsp_hd{,_out}. This patch should fix the issue.
Josh, do you think this is ok, or would you rather assign the handles directly to rt->rtsp_hd without using the local variables?
Doesn't the jump to fail: clean things up? If I'm missing something here, then it looks OK.
Applied this fix, feel free to propose other ways to reorganize it to make it cleaner (e.g. by assigning the handles directly to rt->rtsp_hd, so the normal cleanup path can take care of it. // Martin
Hi Martin, On 18 June 2010 10:57, Martin Storsjö <martin@martin.st> wrote:
On Fri, 18 Jun 2010, Josh Allmann wrote:
On 18 June 2010 00:45, Martin Storsjö <martin@martin.st> wrote:
Hi,
I noticed that the GET URLContext can be leaked if some of the later operations fail before assigning the handles to rt->rtsp_hd{,_out}. This patch should fix the issue.
Josh, do you think this is ok, or would you rather assign the handles directly to rt->rtsp_hd without using the local variables?
Doesn't the jump to fail: clean things up? If I'm missing something here, then it looks OK.
Applied this fix, feel free to propose other ways to reorganize it to make it cleaner (e.g. by assigning the handles directly to rt->rtsp_hd, so the normal cleanup path can take care of it.
Thanks for the fix. I do agree that getting rid of the local variables leads to cleaner code overall, so here's a patch for that. Josh
// Martin _______________________________________________ FFmpeg-soc mailing list FFmpeg-soc@mplayerhq.hu https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-soc
On Sat, 19 Jun 2010, Josh Allmann wrote:
On 18 June 2010 10:57, Martin Storsjö <martin@martin.st> wrote:
On Fri, 18 Jun 2010, Josh Allmann wrote:
On 18 June 2010 00:45, Martin Storsjö <martin@martin.st> wrote:
Hi,
I noticed that the GET URLContext can be leaked if some of the later operations fail before assigning the handles to rt->rtsp_hd{,_out}. This patch should fix the issue.
Josh, do you think this is ok, or would you rather assign the handles directly to rt->rtsp_hd without using the local variables?
Doesn't the jump to fail: clean things up? If I'm missing something here, then it looks OK.
Applied this fix, feel free to propose other ways to reorganize it to make it cleaner (e.g. by assigning the handles directly to rt->rtsp_hd, so the normal cleanup path can take care of it.
Thanks for the fix. I do agree that getting rid of the local variables leads to cleaner code overall, so here's a patch for that.
Looks good, applied. // Martin
participants (2)
-
Josh Allmann -
Martin Storsjö