[Asterisk-code-review] fax: Fix crashes in PJSIP re-negotiation scenarios. (asterisk[16])

[Kevin Harwell]

Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/14274 )

Change subject: fax: Fix crashes in PJSIP re-negotiation scenarios.
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

-1 for question visibility

https://gerrit.asterisk.org/c/asterisk/+/14274/1/bridges/bridge_simple.c 
File bridges/bridge_simple.c:

https://gerrit.asterisk.org/c/asterisk/+/14274/1/bridges/bridge_simple.c@109 
PS1, Line 109: ast_stream_get_formats(stream)
If the stream's format structure should be immutable, should we make ast_stream_get_formats return a const pointer instead thus avoiding this potential situation from happening again?


https://gerrit.asterisk.org/c/asterisk/+/14274/1/res/res_pjsip_session.c 
File res/res_pjsip_session.c:

https://gerrit.asterisk.org/c/asterisk/+/14274/1/res/res_pjsip_session.c@1733 
PS1, Line 1733: ast_format_cap_get_compatible(ast_stream_get_formats(stream), session->endpoint->media.codecs, joint_cap);
> Shouldn't ast_stream_get_formats need to be checked here too?
I believe the idea here is in these cases it should never be NULL. Which was part of the problem this patch is trying to solve (some off nominal fax path). That said I think we do NULL checks for this elsewhere, and I believe it has been a problem in the past, and could be again moving forward.

I propose instead of checking we allocate (if needed) an empty format at stream allocation time. That way it's never NULL, and we can avoid NULL checks now and moving forward for this. "empty" format bugs may arise, but at least things shouldn't crash. Thoughts?



-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/14274
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: I8ed5924b53be9fe06a385c58817e5584b0f25cc2
Gerrit-Change-Number: 14274
Gerrit-PatchSet: 1
Gerrit-Owner: Joshua Colp <jcolp at sangoma.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-CC: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-Comment-Date: Mon, 20 Apr 2020 21:24:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: Alexei Gradinari <alex2grad at gmail.com>
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20200420/319b4cee/attachment.html>