On Thursday 11 October 2012, Daniel-Constantin Mierla wrote: > On 10/11/12 2:19 PM, Alex Hermann wrote: > > i found that $T_barnch_idx returns inconsistent numbers for the same > > branch. If printed in REQUEST_ROUTE, the value is alwasy 1 more that in > > REPLY_ROUTE. > > do you actually mean BRANCH_ROUTE instead of REQUEST_ROUTE?
Yes, and for completeness, i also meant TM_ONREPLY_ROUTE instead of REPLY_ROUTE. > > The branch index is set using tm_ctx_set_branch_index(branch) in a lot of > > places, but only in t_fwd.c it is set with > > tm_ctx_set_branch_index(branch+1). > > > > If i change that to the same as elsewhere, the numbering is consistent. > > I'd like to get an ACK from a tm guru before i commit this fix, because > > i have no idea what side-effects this might have. > > Indeed it should be same value. Probably when was added first time for > branch_route was more like: > - if 0, then is like unset or before transaction is created > - if >0, then is branch index + 1 > > To bring proper coherence, I see two options: > - keep 0 for unset and return branch index + 1 otherwise > - use -1 for unset and return branch index otherwise I prefer the latter option because it corresponds to the internal state. I think this eases debugging and avoids mistakes. In this way the T_branch_idx is also the same as the last digit(s) in the added Via header. > In either way, the patch has to updated. No problem. Additional patch: commit 5c44d5f5d91f84757586d2af44c9ac016ec9fa96 Author: Alex Hermann <[email protected]> Date: Fri Oct 12 14:06:33 2012 +0200 modules/tm: Set branch_index to T_BR_UNDEFINED when outside BRANCH_ROUTE or TM_ONREPLY_ROUTE. The inconsistent value of $T_branch_idx between BRANCH_ROUTE and TM_ON_REPLY_ROUTE was fixed in an earlier commit, but now the value 0 has a double meaning (branch 0 or invalid branch). This patch makes the invalid branch distinguishable by setting it to -1. Now $T_branch_idx will return the branch number (0-based) in BRANCH_ROUTE and TM_ON_REPLY_ROUTE and -1 in other route types or if the message is not part of a transaction. diff --git a/modules/tm/t_fwd.c b/modules/tm/t_fwd.c index 6f8661d..90b36b8 100644 --- a/modules/tm/t_fwd.c +++ b/modules/tm/t_fwd.c @@ -351,14 +351,14 @@ static int prepare_new_uac( struct cell *t, struct sip_msg *i_req, /* if DROP was called in cfg, don't forward, jump to end */ if (unlikely(ctx.run_flags&DROP_R_F)) { - tm_ctx_set_branch_index(0); + tm_ctx_set_branch_index(T_BR_UNDEFINED); set_route_type(backup_route_type); /* triggered by drop in CFG */ ret=E_CFG; goto error03; } } - tm_ctx_set_branch_index(0); + tm_ctx_set_branch_index(T_BR_UNDEFINED); set_route_type(backup_route_type); } diff --git a/modules/tm/t_lookup.c b/modules/tm/t_lookup.c index c070948..a89744e 100644 --- a/modules/tm/t_lookup.c +++ b/modules/tm/t_lookup.c @@ -1970,6 +1970,7 @@ tm_ctx_t* tm_ctx_get(void) void tm_ctx_init(void) { memset(&_tm_ctx, 0, sizeof(tm_ctx_t)); + _tm_ctx.branch_index = T_BR_UNDEFINED; } void tm_ctx_set_branch_index(int v) diff --git a/modules/tm/t_reply.c b/modules/tm/t_reply.c index e210452..ce1dccb 100644 --- a/modules/tm/t_reply.c +++ b/modules/tm/t_reply.c @@ -2363,7 +2363,7 @@ int reply_received( struct sip_msg *p_msg ) } /* provisional replies */ done: - tm_ctx_set_branch_index(0); + tm_ctx_set_branch_index(T_BR_UNDEFINED); /* we are done with the transaction, so unref it - the reference * was incremented by t_check() function -bogdan*/ t_unref(p_msg); diff --git a/modules/tm/tm.c b/modules/tm/tm.c index fcfa328..8352f3f 100644 --- a/modules/tm/tm.c +++ b/modules/tm/tm.c @@ -847,6 +847,10 @@ static int mod_init(void) #endif /* WITH_EVENT_LOCAL_REQUEST */ if (goto_on_sl_reply && onreply_rt.rlist[goto_on_sl_reply]==0) WARN("empty/non existing on_sl_reply route\n"); + +#ifdef WITH_TM_CTX + tm_ctx_init(); +#endif tm_init = 1; return 0; } diff --git a/modules_k/tmx/t_var.c b/modules_k/tmx/t_var.c index b89d729..7a0c159 100644 --- a/modules_k/tmx/t_var.c +++ b/modules_k/tmx/t_var.c @@ -379,7 +379,7 @@ int pv_get_tm_branch_idx(struct sip_msg *msg, pv_param_t *param, if(tcx != NULL) idx = tcx->branch_index; - ch = int2str(idx, &l); + ch = sint2str(idx, &l); res->rs.s = ch; res->rs.len = l; -- Met vriendelijke groet, Alex Hermann SpeakUp BV T: 088-SPEAKUP (088-7732587) F: 088-7732588 _______________________________________________ sr-dev mailing list [email protected] http://lists.sip-router.org/cgi-bin/mailman/listinfo/sr-dev
