[sr-dev] Registrar - check for local path

[Charles Chance]

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 at sipcentric.com>
wrote:

>
> On 14 December 2014 at 21:40, Daniel-Constantin Mierla <miconda at 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 <http://twitter.com/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.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20150109/1acb2f36/attachment-0001.html>