[Asterisk-code-review] Geolocation: chan_pjsip Capability Preview (asterisk[16])

[Kevin Harwell]

Attention is currently required from: Stanislav Abramenkov, George Joseph.
Kevin Harwell has posted comments on this change. ( https://gerrit.asterisk.org/c/asterisk/+/18697 )

Change subject: Geolocation:  chan_pjsip Capability Preview
......................................................................


Patch Set 2: Code-Review-1

(4 comments)

File res/res_pjsip_geolocation.c:

https://gerrit.asterisk.org/c/asterisk/+/18697/comment/31472bb4_f2eb49da 
PS1, Line 300: 	geoloc_hdr_value = ast_alloca(geoloc_hdr->hvalue.slen + 1);
             : 	ast_copy_pj_str(geoloc_hdr_value, &geoloc_hdr->hvalue, geoloc_hdr->hvalue.slen + 1);
             : 	duped_geoloc_hdr_value = ast_strdupa(geoloc_hdr_value);
geoloc_hdr_value contains a local copy that appears unused, so I think you can just use that directly without having to dupe/copy it a second time.


https://gerrit.asterisk.org/c/asterisk/+/18697/comment/4813b863_6cfe6b2d 
PS1, Line 381: 		rc = ast_geoloc_datastore_add_eprofile(ds, eprofile);
             : 		if (rc <= 0) {
             : 			SCOPE_EXIT_LOG_RTN_VALUE(0, LOG_WARNING,
             : 				"%s: Couldn't add eprofile '%s' to datastore\n", session_name,
             : 				eprofile->id);
             : 		}
             : 		eprofile = NULL;
I think this causes a ref leak on eprofile.

ast_geoloc_eprofile_create_from_profile adds a ref, ast_geoloc_datastore_add_eprofile bumps the eprofile ref, so eprofile = +2 ref. But then you set it to NULL here so RAII_VAR does not remove the extra ref when leaving this function


https://gerrit.asterisk.org/c/asterisk/+/18697/comment/b844a1dc_ab0adb93 
PS1, Line 491: 	SCOPE_EXIT_RTN_VALUE(0, "%s: PIDF-LO added with cid '%s'\n", session_name, base_cid);
base_cid is leaked here.


https://gerrit.asterisk.org/c/asterisk/+/18697/comment/97d52ce6_d402988b 
PS1, Line 612: 		struct ast_geoloc_eprofile *ep = ast_geoloc_datastore_get_eprofile(tempds, i);
ep is ref leaked on each loop.



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

Gerrit-Project: asterisk
Gerrit-Branch: 16
Gerrit-Change-Id: Ie2e2bcd87243c2cfabc43eb823d4427c7086f4d9
Gerrit-Change-Number: 18697
Gerrit-PatchSet: 2
Gerrit-Owner: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Stanislav Abramenkov <stas.abramenkov at gmail.com>
Gerrit-Attention: Stanislav Abramenkov <stas.abramenkov at gmail.com>
Gerrit-Attention: George Joseph <gjoseph at digium.com>
Gerrit-Comment-Date: Thu, 07 Jul 2022 16:56:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20220707/1ac1c9b6/attachment.html>