<p><b>@henningw</b> commented on this pull request.</p>
<p>Thanks you Yasin for the pull request. I've did a quick review, and have added some comments and questions to the code.</p><hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344201100">src/core/parser/parse_uri.c</a>:</p>
<pre style='color:#555'>> @@ -1439,3 +1439,26 @@ void proto_type_to_str(unsigned short type, str *s) {
*s = s_null;
}
}
+
+void proto_type_int_to_str(int type, str *s) {
</pre>
<p>What about just using the existing function proto_type_to_str? You can easily cast the input parameter from int to unsigned short, or I am wrong?</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344202344">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> +
+ contact.s = ((contact_body_t *)msg->contact->parsed)->contacts->name.s;
+ contact.len = ((contact_body_t *)msg->contact->parsed)->contacts->len;
+
+ alias_to_uri(&contact,&alias_uri);
+ write_to_avp(msg, &alias_uri, uri_avp);
+
+ return 1;
+
+}
+/**
+* write_to_avp function writes data to given avp
+* @param data source data
+* @param uri_avp destination avp name
+*/
+void write_to_avp(struct sip_msg *msg, str *data, str *uri_avp){
</pre>
<p>It would be probably a good idea to also return an error code if this function fails, to later evaluate it in the set_alias_to_avp function.</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344202691">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> +
+ if(!data->s){
+ LM_ERR("There isnt any data to write avp\n");
+ return;
+ }
+
+ valx.flags = PV_VAL_STR;
+ valx.rs.s = data->s;
+ valx.rs.len = data->len;
+
+ LM_DBG("result: %.*s\n", valx.rs.len, valx.rs.s);
+ pvresult->setf(msg, &pvresult->pvp, (int)EQ_T, &valx);
+
+}
+/**
+* function alias_to_uri select alias paramter from @param contact_header
</pre>
<p>spelling - parameter</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344203014">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> +
+ valx.flags = PV_VAL_STR;
+ valx.rs.s = data->s;
+ valx.rs.len = data->len;
+
+ LM_DBG("result: %.*s\n", valx.rs.len, valx.rs.s);
+ pvresult->setf(msg, &pvresult->pvp, (int)EQ_T, &valx);
+
+}
+/**
+* function alias_to_uri select alias paramter from @param contact_header
+* then writes to @param alias_uri
+* @param contact_header Source contact header
+* @param alias_uri destination string
+*/
+void alias_to_uri(str *contact_header, str *alias_uri){
</pre>
<p>Same as above, consider to return an error in case this function fails to properly evaluate it in the calling function.</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344203570">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> +
+}
+/**
+* function reads from @param msg then write to given avp @param uri_avp
+* as sip uri
+* @result 1 succes , -1 fail
+* @param msg sip message
+* @param uri_avp given avp name
+*/
+static int set_alias_to_avp_f(struct sip_msg *msg, str *uri_avp){
+ str contact;
+ str alias_uri={0,0};
+
+
+ if(parse_headers(msg,HDR_CONTACT_F,0) < 0 ) {
+ LM_ERR("Finding Contact Header is failed\n");
</pre>
<p>Spelling - "Could not find Contact Header"</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344203642">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> + str contact;
+ str alias_uri={0,0};
+
+
+ if(parse_headers(msg,HDR_CONTACT_F,0) < 0 ) {
+ LM_ERR("Finding Contact Header is failed\n");
+ return -1;
+ }
+
+ if(!msg->contact)
+ return -1;
+
+ if(parse_contact(msg->contact)<0 || !msg->contact->parsed ||
+ ((contact_body_t *)msg->contact->parsed)->contacts==NULL ||
+ ((contact_body_t *)msg->contact->parsed)->contacts->next!=NULL){
+ LM_ERR("Parsing Contact Header is failed\n");
</pre>
<p>same</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344203786">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> + if(!msg->contact)
+ return -1;
+
+ if(parse_contact(msg->contact)<0 || !msg->contact->parsed ||
+ ((contact_body_t *)msg->contact->parsed)->contacts==NULL ||
+ ((contact_body_t *)msg->contact->parsed)->contacts->next!=NULL){
+ LM_ERR("Parsing Contact Header is failed\n");
+ return -1;
+ }
+
+
+
+ contact.s = ((contact_body_t *)msg->contact->parsed)->contacts->name.s;
+ contact.len = ((contact_body_t *)msg->contact->parsed)->contacts->len;
+
+ alias_to_uri(&contact,&alias_uri);
</pre>
<p>as referenced below, return value checks for both functions</p>
<hr>
<p>In <a href="https://github.com/kamailio/kamailio/pull/2124#discussion_r344209171">src/modules/nathelper/nathelper.c</a>:</p>
<pre style='color:#555'>> + if(memchr_pointer == NULL) {
+ LM_ERR("No alias param found for port\n");
+ return ;
+ } else {
+ port.len = memchr_pointer - port.s;
+ i=i+port.len;
+ }
+
+ //last char is proto 0,1,2,3,4..7
+ proto.s= &port.s[port.len+1];
+ proto_type_int_to_str(atoi(proto.s), &proto);
+
+ LM_DBG("Host [%.*s][port: %.*s][proto: %.*s] \r\n",host.len,host.s,port.len,port.s,proto.len,proto.s);
+
+ //sip:host:port;transport=udp
+ alias_uri->s =(char *) pkg_malloc(port.len+host.len+proto.len+16);
</pre>
<p>When is alias_uri->s freed?</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/2124?email_source=notifications&email_token=ABO7UZJ32NJDQFGQO6GGNV3QSV4LNA5CNFSM4JKWMD52YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCK5OGOI#pullrequestreview-314237753">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABO7UZMJIS2D7CC6YDPABODQSV4LNANCNFSM4JKWMD5Q">unsubscribe</a>.<img src="https://github.com/notifications/beacon/ABO7UZIRPBNMYJ6HFPLTVZ3QSV4LNA5CNFSM4JKWMD52YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCK5OGOI.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/kamailio/kamailio/pull/2124?email_source=notifications\u0026email_token=ABO7UZJ32NJDQFGQO6GGNV3QSV4LNA5CNFSM4JKWMD52YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCK5OGOI#pullrequestreview-314237753",
"url": "https://github.com/kamailio/kamailio/pull/2124?email_source=notifications\u0026email_token=ABO7UZJ32NJDQFGQO6GGNV3QSV4LNA5CNFSM4JKWMD52YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCK5OGOI#pullrequestreview-314237753",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>