<p><b>@henningw</b> commented on this pull request.</p>
<p>From the code change all looks fine to me. I've added some comments inline about a few things I noticed.</p><hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/1832#discussion_r257831161">src/modules/dialog/dlg_hash.c</a>:</p>
<pre style='color:#555'>> @@ -528,7 +533,7 @@ int dlg_set_leg_info(struct dlg_cell *dlg, str* tag, str *rr, str *contact,
if(dlg->tag[leg].s)
shm_free(dlg->tag[leg].s);
- dlg->tag[leg].s = (char*)shm_malloc( tag->len + rr->len );
</pre>
<p>Why you decrease here the shm malloc'ed area, do you change something in the tag as well?</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/1832#discussion_r257832973">src/modules/dialog/dlg_handlers.c</a>:</p>
<pre style='color:#555'>> @@ -269,6 +271,25 @@ int populate_leg_info( struct dlg_cell *dlg, struct sip_msg *msg,
if (rr_set.s) pkg_free(rr_set.s);
</pre>
<p>This free is in current git master at the end of this function and seems redundant now. Maybe it make sense to combine it with the other free in line 290 and move it to the end of the function?</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/1832#discussion_r257832993">src/modules/dialog/dlg_handlers.c</a>:</p>
<pre style='color:#555'>> + /* skip_recs contains the number of RR's for the callee */
+ skip_recs -= own_rr;
+ /* Add local RR's to caller's routeset */
+ if( print_rr_body(msg->record_route, &rr_set, DLG_CALLER_LEG,
+ &skip_recs) != 0) {
+ LM_ERR("failed to print route records \n");
+ goto error0;
+ }
+ LM_DBG("updating caller route_set %.*s\n",
+ rr_set.len, rr_set.s);
+ if (dlg_update_rr_set( dlg, DLG_CALLER_LEG, &rr_set)!=0) {
+ LM_ERR("dlg_update_rr_set failed\n");
+ if (rr_set.s) pkg_free(rr_set.s);
+ goto error0;
+ }
+ if (rr_set.s) pkg_free(rr_set.s);
</pre>
<p>see above</p>
<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/kamailio/kamailio/pull/1832#pullrequestreview-204957921">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/AF36ZZ2gC1IP39_yhSqAipWNT7F9-Fciks5vOyAAgaJpZM4aeP-h">mute the thread</a>.<img src="https://github.com/notifications/beacon/AF36ZSWVLQ6BSgr34Va8I_m0whFxgerAks5vOyAAgaJpZM4aeP-h.gif" height="1" width="1" alt="" /></p>
<script type="application/json" data-scope="inboxmarkup">{"api_version":"1.0","publisher":{"api_key":"05dde50f1d1a384dd78767c55493e4bb","name":"GitHub"},"entity":{"external_key":"github/kamailio/kamailio","title":"kamailio/kamailio","subtitle":"GitHub repository","main_image_url":"https://github.githubassets.com/images/email/message_cards/header.png","avatar_image_url":"https://github.githubassets.com/images/email/message_cards/avatar.png","action":{"name":"Open in GitHub","url":"https://github.com/kamailio/kamailio"}},"updates":{"snippets":[{"icon":"PERSON","message":"@henningw commented on #1832"}],"action":{"name":"View Pull Request","url":"https://github.com/kamailio/kamailio/pull/1832#pullrequestreview-204957921"}}}</script>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/1832#pullrequestreview-204957921",
"url": "https://github.com/kamailio/kamailio/pull/1832#pullrequestreview-204957921",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>