Hello,

Would anyone like to review the final changes, before I merge them into master?

I have created personal branch cchance/registrar, with the relevant commits being:

https://github.com/kamailio/kamailio/commit/635f23b12eff2431ca9a14bb39f4204dc2a7227b

https://github.com/kamailio/kamailio/commit/4ff2c48d66a3bce2c491b44d0f1b5e939e5508ff

Thanks in advance,

Charles

On 15 December 2014 at 12:58, Charles Chance <charles.chance@sipcentric.com> wrote:

On 14 December 2014 at 21:40, Daniel-Constantin Mierla <miconda@gmail.com> wrote:

Hello,

looking at the patch, I see that the block for parsing first path uri:

+			if (parse_uri(path_dst.s, path_dst.len, &path_uri) < 0){
+				LM_ERR("failed to parse the Path URI\n");
+				ret = -3;
+				goto done;
+			}


Is done outside of parameter check:


 +			if (path_check_local > 0

But seems to be used only inside it (when the parameter is set >0). It should be moved inside that IF, to avoid parsing the path uri when the parameter is not set, because that happens for each lookup.

Yes, spotted that one already and it has been moved inside the parameter check.

 

Another thing that has to be double-checked: you change the value of ptr->path.s if there is a match of local URI for first path. That can mess the structure from usrloc, if it needs to do a free later or is a pointer inside shared memory that is going to be used later -- practically the pointer is no longer at the beginning of allocated memory. I didn't have time to look deeper in usrloc and all its db modes (iirc, for db-only, the location record is temporarily built in memory and freed). The best and safest for the future is to make a copy of the path str and work with it inside the function where you need to change it

str path_str;

...
path_str = ptr->path;
// and use path_str instead of ptr->path from here on
...

Sorry, oversight on my part, will make a copy and work with that instead.

 

The new parameter has to be documented in the readme as well.

 

Of course :)

Thanks for your time,

Charles

www.sipcentric.com

Follow us on twitter @sipcentric

Sipcentric Ltd. Company registered in England & Wales no. 7365592. Registered office: Faraday Wharf, Innovation Birmingham Campus, Holt Street, Birmingham Science Park, Birmingham B7 4BB.

-- 
Daniel-Constantin Mierla
http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda