[Asterisk-code-review] res pjsip session: Use DNID on outgoing call leg. (asterisk[master])

[Richard Mudgett]

Richard Mudgett has posted comments on this change. ( https://gerrit.asterisk.org/5545 )

Change subject: res_pjsip_session: Use DNID on outgoing call leg.
......................................................................


Patch Set 3:

(6 comments)

NOTE: DNID isn't going to be set on originated channels or channels created by call files.

https://gerrit.asterisk.org/#/c/5545/3/apps/app_dial.c
File apps/app_dial.c:

PS3, Line 2604: 		ast_channel_dialed(tc)->transit_network_select = ast_channel_dialed(chan)->transit_network_select;
              : 
              : 		if (ast_channel_dialed(chan)->number.str) {
              : 			ast_party_dialed_copy(ast_channel_dialed(tc), ast_channel_dialed(chan));
              : 		}
There is no need to conditionally copy.

Replace these highlighted lines with the copy function line because the transit_network_select copy is part of the dialed copy function.

ast_party_dialed_copy(ast_channel_dialed(tc), ast_channel_dialed(chan));


The same thing needs to be done to the do_forward() function in this file.


https://gerrit.asterisk.org/#/c/5545/3/res/res_pjsip_session.c
File res/res_pjsip_session.c:

Line 880: static void set_to_header(struct ast_sip_session *session){
Formatting guidelines:

Use tabs not spaces.
Opening function curly on its own line.

static void foo(void)
{
  pj_pool_t *dlg_pool;

  if (!session->channel) {
    return;
  }
}


Line 886:         struct ast_party_dialed dialed;
dialed isn't even needed.  It is setup but then not really used.


PS3, Line 892:         ast_party_dialed_init(&dialed);
             :         ast_party_dialed_set(&dialed, ast_channel_dialed(session->channel));
             :         if (!dialed.number.str) {
             : 		ast_party_dialed_free(&dialed);
             :                 return;
             :         }
Change to below and there is almost nothing left referencing the dialed variable:

ast_channel_lock(session->channel);
if (!ast_strlen_zero(ast_channel_dialed(session->channel)->number.str)) {
  ast_channel_unlock(session->channel);
  return;
}

We need to lock the channel to be sure that the dialed information is valid while we access the data.


PS3, Line 906: 	ast_party_dialed_free(&dialed);
             : 
             :         return;
             : 
Replace:

ast_channel_unlock(session->channel);


Line 1279: 	set_to_header(session);
This likely also needs to be done in ast_sip_session_refresh() for the same reason.  (The other place where set_from_header() is called.)


-- 
To view, visit https://gerrit.asterisk.org/5545
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I77261b6e6e7962f7371b91d78b8124af050cab61
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Yasin CANER <yasin.caner at netgsm.com.tr>
Gerrit-Reviewer: Jenkins2
Gerrit-Reviewer: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: Richard Mudgett <rmudgett at digium.com>
Gerrit-HasComments: Yes