<div dir="ltr">Hello,<div><br></div><div>Would anyone like to review the final changes, before I merge them into master?</div><div><br></div><div>I have created personal branch cchance/registrar, with the relevant commits being:</div><div><br></div><div><a href="https://github.com/kamailio/kamailio/commit/635f23b12eff2431ca9a14bb39f4204dc2a7227b">https://github.com/kamailio/kamailio/commit/635f23b12eff2431ca9a14bb39f4204dc2a7227b</a><br></div><div><a href="https://github.com/kamailio/kamailio/commit/4ff2c48d66a3bce2c491b44d0f1b5e939e5508ff">https://github.com/kamailio/kamailio/commit/4ff2c48d66a3bce2c491b44d0f1b5e939e5508ff</a><br></div><div><br></div><div>Thanks in advance,<br></div><div><br></div><div>Charles<br></div><div><br></div><div><br></div><div class="gmail_extra"><div class="gmail_quote">On 15 December 2014 at 12:58, Charles Chance <span dir="ltr"><<a href="mailto:charles.chance@sipcentric.com" target="_blank">charles.chance@sipcentric.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><br><div class="gmail_quote"><span class="">On 14 December 2014 at 21:40, Daniel-Constantin Mierla <span dir="ltr"><<a href="mailto:miconda@gmail.com" target="_blank">miconda@gmail.com</a>></span> wrote:<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000">
Hello,<br>
<br>
looking at the patch, I see that the block for parsing first path
uri:<br>
<br>
<pre>+ 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</pre>
<br>
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.<br>
<br></div></blockquote><div><br></div></span><div>Yes, spotted that one already and it has been moved inside the parameter check.</div><span class=""><div><br></div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">
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<br>
<br>
str path_str;<br>
<br>
...<br>
path_str = ptr->path;<br>
// and use path_str instead of ptr->path from here on<br>
...<br>
<br></div></blockquote><div><br></div></span><div>Sorry, oversight on my part, will make a copy and work with that instead.</div><span class=""><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div bgcolor="#FFFFFF" text="#000000">
<br>
The new parameter has to be documented in the readme as well.<br>
<br></div></blockquote><div> </div></span><div>Of course :)</div><div><br></div><div>Thanks for your time,</div><div><br></div><div>Charles</div><div><br></div></div>
</div></div>
</blockquote></div><br><br clear="all"><div><br></div>
</div></div>
<br>
<font face="Helvetica, Arial, sans-serif"><font size="2"><span style="font-size:10pt"><a href="http://www.sipcentric.com/" title="blocked::http://www.sipcentric.com/" target="_blank">www.sipcentric.com</a><br>
<br>
Follow us on twitter <a href="http://twitter.com/sipcentric" title="blocked::http://twitter.com/sipcentric" target="_blank">@sipcentric</a><br>
<br>
<font color="gray">Sipcentric Ltd.
Company registered in England & Wales no. 7365592.</font> <font color="gray">Registered
office: Faraday Wharf, Innovation Birmingham Campus, Holt Street, Birmingham Science Park, Birmingham B7 4BB.</font></span></font></font>