[Serdev] t_is_canceled() useless?

[Andrei Pelinescu-Onciul]

On Jun 02, 2008 at 13:49, Martin Hoffmann <hn at nvnc.de> wrote:
> Salve,
> 
> is it me or is t_is_canceled() completely useless? According to the
> code, it will only return true if a CANCEL was received while no
> requests have yet been sent out or some other case which I don't quite
> understand that involves calling t_newtran() while a CANCEL is being
> processed. (Grep for T_CANCELED to get the relevant code.)

It will return true if a CANCEL arrived out of order (before the INVITE)
or if it arrived in the same time, but the CANCEL get first processed
(that was the t_newtran() part).

It should return true also for an e2e cancel. The problem here is if we
 set T_CANCELED for any cancel attempts (always) or only for cancel to
 transactions that haven't received a 2xx.
OTOH I  think it doesn't matter if we set it every time (a 2xx replied 
transaction would be on its final wait timer anyway, it won't even look
at the T_CANCELED flag) and I think it can be set w/o locking replies
(in the wrost case it will race w/ setting T_6xx, but it shouldn't be a 
problem).

> 
> Now, unlike whoever introduced the function had something else in mind
> then me, you need t_is_canceled() to stop call forwarding when a
> transaction has been canceled. This is necessary since a 408 is
> preferred over a 487 in branch picking and subsequently a
> 
>    if (t_check_status("408")) {
>    	# append branch and t_relay()
>    }
> 
> will be looping. Therefore, I need t_is_canceled() to break out of the
> loop.
> 
> For now I can fix this by setting a flag after first call forwarding and
> then only forward once. However, I am also going to do some call hunting
> where this approach will not work anymore.

You could use also t_grep_status("487"), if any branch did receive a
487, it will return true.

> 
> Do I miss something or should this be changed?

No, it should be changed.
> 


Andrei