[sr-dev] git:master:27bed654: tm: check all the outgoing branches when appending a new branch

[Federico Cabiddu]

Thank you Daniel for the suggestion. I realized that I misunderstood the
meaning of nr_branches. My intention was to use it as a "starting point" to
check only the newly created branches in the destination set. I will change
it according to your suggestion and also remove the history if you agree.

Thank you,

Federico

On Fri, Sep 4, 2015 at 3:27 PM, Daniel-Constantin Mierla <miconda at gmail.com>
wrote:

> It is not a big problem, just wanted to underline it to be aware of. And
> it would be good to be removed, indeed.
>
> As I looked at the code in the file with this occasion, if you want to go
> through all outgoing branches in the destination set, then you need to do:
>
> init_branch_iterator();
>
> instead of:
>
> set_branch_iterator(nr_branches-1);
>
> That will look at all next hops that are already in destination set.
>
> Assuming that you mainly use this function when you get a new registration
> and have one new outgoing branch, then nr_branches-1 can result to be 0.
> However, if for what so ever reason you get more branches, then you will
> use only one with existing code. As I got it from the code, you check if
> the destination is already used, so just going again from branch 0 in
> destination set looks like what you wanted to do. Maybe I am wrong ...
>
> Cheers,
> Daniel
>
>
> On 04/09/15 15:21, Federico Cabiddu wrote:
>
> I'm sorry,
> I didn't see it with git difftool before committing. Do you want that I
> clean the file?
>
> Cheers,
>
> Federico
>
> On Fri, Sep 4, 2015 at 3:18 PM, Daniel-Constantin Mierla <
> <miconda at gmail.com>miconda at gmail.com> wrote:
>
>> Hello,
>>
>> you re-introduced the history at the top of the file, which was removed
>> from tm module and going to be removed from everywhere in the code, as
>> most of it is irrelevant. The changes are better tracked via git now,
>> with the commit message and git diff/blame being very handy.
>>
>> Cheers,
>> Daniel
>>
>> On 04/09/15 15:13, Federico Cabiddu wrote:
>> > Module: kamailio
>> > Branch: master
>> > Commit: 27bed654dcc4e9dd543a7f2f92e2174478bf7f26
>> > URL:
>> https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26
>> >
>> > Author: Federico Cabiddu < <federico.cabiddu at gmail.com>
>> federico.cabiddu at gmail.com>
>> > Committer: Federico Cabiddu < <federico.cabiddu at gmail.com>
>> federico.cabiddu at gmail.com>
>> > Date: 2015-09-04T15:12:29+02:00
>> >
>> > tm: check all the outgoing branches when appending a new branch
>> >
>> > ---
>> >
>> > Modified: modules/tm/t_append_branches.c
>> >
>> > ---
>> >
>> > Diff:
>> https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26.diff
>> > Patch:
>> https://github.com/kamailio/kamailio/commit/27bed654dcc4e9dd543a7f2f92e2174478bf7f26.patch
>> >
>> > ---
>> >
>> > diff --git a/modules/tm/t_append_branches.c
>> b/modules/tm/t_append_branches.c
>> > index 8060021..f16c672 100644
>> > --- a/modules/tm/t_append_branches.c
>> > +++ b/modules/tm/t_append_branches.c
>> > @@ -23,6 +23,9 @@
>> >   * NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
>> SOFTWARE,
>> >   * EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
>> >   *
>> > + * History:
>> > + * -------
>> > + *  2014-09-09  first release of t_append_branches
>> >   *
>> >   */
>> >
>> > @@ -106,8 +109,6 @@ int t_append_branches(void) {
>> >               set_branch_route(t->on_branch_delayed);
>> >       }
>> >
>> > -     outgoings = t->nr_of_outgoings;
>> > -
>> >       /* not really sure that the following is needed */
>> >
>> >       set_branch_iterator(nr_branches-1);
>> > @@ -116,7 +117,7 @@ int t_append_branches(void) {
>> >
>>        &bflags, &si, &ruid, &instance, &location_ua))) {
>> >               LM_DBG("Current uri %.*s\n",current_uri.len,
>> current_uri.s);
>> >
>> > -             for (i=0; i<=nr_branches; i++) {
>> > +             for (i=0; i<outgoings; i++) {
>> >                       if (t->uac[i].ruid.len == ruid.len
>> >                                       && !memcmp(t->uac[i].ruid.s,
>> ruid.s, ruid.len)) {
>> >                               LM_DBG("branch already added [%.*s]\n",
>> ruid.len, ruid.s);
>> > @@ -133,6 +134,8 @@ int t_append_branches(void) {
>> >                                       &path, 0, si,
>> orig_msg->fwd_send_flags,
>> >                                       orig_msg->rcv.proto,
>> (dst_uri.len)?-1:UAC_SKIP_BR_DST_F, &instance,
>> >                                       &ruid, &location_ua);
>> > +
>> > +             LM_DBG("added branch [%.*s] with ruid [%.*s]\n",
>> current_uri.len, current_uri.s, ruid.len, ruid.s);
>> >
>> >               /* test if cancel was received meanwhile */
>> >               if (t->flags & T_CANCELED) goto canceled;
>> >
>> >
>> > _______________________________________________
>> > sr-dev mailing list
>> > sr-dev at lists.sip-router.org
>> > http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
>>
>> --
>> Daniel-Constantin Mierla
>> http://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
>> Book: SIP Routing With Kamailio - <http://www.asipto.com>
>> http://www.asipto.com
>>
>>
>
> --
> Daniel-Constantin Mierlahttp://twitter.com/#!/miconda - http://www.linkedin.com/in/miconda
> Book: SIP Routing With Kamailio - http://www.asipto.com
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.sip-router.org/pipermail/sr-dev/attachments/20150904/8737b18d/attachment.html>