@henningw commented on this pull request.

Thanks you Yasin for the pull request. I've did a quick review, and have added some comments and questions to the code.


In src/core/parser/parse_uri.c:

> @@ -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) {

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?


In src/modules/nathelper/nathelper.c:

> +
+	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){

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.


In src/modules/nathelper/nathelper.c:

> +
+	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

spelling - parameter


In src/modules/nathelper/nathelper.c:

> +
+	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){

Same as above, consider to return an error in case this function fails to properly evaluate it in the calling function.


In src/modules/nathelper/nathelper.c:

> +
+}
+/**
+* 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");

Spelling - "Could not find Contact Header"


In src/modules/nathelper/nathelper.c:

> +	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");

same


In src/modules/nathelper/nathelper.c:

> +	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);

as referenced below, return value checks for both functions


In src/modules/nathelper/nathelper.c:

> +	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);

When is alias_uri->s freed?


You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or unsubscribe.