[Asterisk-code-review] res/res pjsip: Resources (udptl fd) are leaking for T.38 calls (asterisk[13])

[Joshua C. Colp]

Joshua C. Colp has posted comments on this change. ( https://gerrit.asterisk.org/10994 )

Change subject: res/res_pjsip: Resources (udptl fd) are leaking for T.38 calls
......................................................................


Patch Set 1: Code-Review-1

(2 comments)

https://gerrit.asterisk.org/#/c/10994/1/res/res_pjsip_t38.c
File res/res_pjsip_t38.c:

https://gerrit.asterisk.org/#/c/10994/1/res/res_pjsip_t38.c@156
PS1, Line 156: 		return NULL;
I think this still leaks the datastore if ast_sip_session_add_datastore fails.


https://gerrit.asterisk.org/#/c/10994/1/res/res_pjsip_t38.c@159
PS1, Line 159: 	ao2_ref(datastore, -1);
I don't think this is the complete fix. The problem is that you are returning datastore to the caller, but never actually using it there - it's just used to see if this was a success or failure.

I think the return value of the function should be changed to an int to return 0 on success, and -1 on failure.

The ao2_ref(datastore, -1); should also be moved to after the datastore->data is set.



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

Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-MessageType: comment
Gerrit-Change-Id: Id6dceceb06651b03f611bf33deb3061022fe5d0c
Gerrit-Change-Number: 10994
Gerrit-PatchSet: 1
Gerrit-Owner: Paulo Vicentini <paulo.vicentini at gmail.com>
Gerrit-Reviewer: Friendly Automation (1000185)
Gerrit-Reviewer: Joshua C. Colp <jcolp at digium.com>
Gerrit-Comment-Date: Wed, 13 Feb 2019 13:46:36 +0000
Gerrit-HasComments: Yes
Gerrit-HasLabels: Yes
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190213/6c7d66f0/attachment.html>