[asterisk-dev] [Code Review] Avoid infinite loop with certain local channel connected line updates

[Terry Wilson]

> On 2010-09-20 18:16:55, Russell Bryant wrote:
> > /branches/1.8/main/channel.c, lines 8191-8195
> > <https://reviewboard.asterisk.org/r/932/diff/3/?file=12582#file12582line8191>
> >
> >     You could shorten this to:
> >     
> >     return memcmp(...);

Good point. Will do.


> On 2010-09-20 18:16:55, Russell Bryant wrote:
> > /branches/1.8/main/channel.c, line 8184
> > <https://reviewboard.asterisk.org/r/932/diff/3/?file=12582#file12582line8184>
> >
> >     Same thing, but I prefer ... if (!c1 || !c2)

And rely on *compilers* to optimize out the extra operation? NEVER! Oh, ok. ;-)


- Terry


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/932/#review2758
-----------------------------------------------------------


On 2010-09-20 17:56:40, Terry Wilson wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/932/
> -----------------------------------------------------------
> 
> (Updated 2010-09-20 17:56:40)
> 
> 
> Review request for Asterisk Developers, Russell Bryant and Mark Michelson.
> 
> 
> Summary
> -------
> 
> It is possible to create an infinite loop with connected line updates and local channels. One side of the local channel queues an update to the other side in local_indicate() which then causes the other side to do the same ad infinitum. This patch adds a function to compare the connected line update with the connected line info already on a channel. If there is no difference, it doesn't send the update frame.
> 
> It uses ast_connected_line_build_data since that function would need to be changed if any of the ast_connected_line-related structs changed and trying to compare each setting individually is really a pain and would be just another function to update.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/channel.c 287638 
> 
> Diff: https://reviewboard.asterisk.org/r/932/diff
> 
> 
> Testing
> -------
> 
> queues.conf
> ---------
> [general]
> persistentmembers = yes
> autofill = yes
> monitor-type = MixMonitor
> shared_lastcall=no
> 
> [test]
> strategy=ringall
> member => Local/6001 at sip/nm
> reportholdtime=yes
> 
> extensions.conf
> -------------
> [sip]
> exten => 123,1,NoOp
> exten => 123,n,Answer
> exten => 123,n,Queue(test)
> 
> exten => _XXXX,1,Set(CONNECTEDLINE(num,i)=5551212)
> exten => _XXXX,n,Set(CONNECTEDLINE(name,i)=Widgets)
> exten => _XXXX,n,Set(__CONNECTED_LINE_CALLEE_SEND_MACRO=CONNECTED_LINE_MANIPULATION)
> exten => _XXXX,n,Set(__CONNECTED_LINE_CALLER_SEND_MACRO=CONNECTED_LINE_MANIPULATION)
> exten => _XXXX,n,Dial(SIP/${EXTEN})
> 
> [macro-CONNECTED_LINE_MANIPULATION]
> exten => s,1,NoOp(MACRO WAS CALLED)
> 
> sip.conf
> ------
> 
> [test](!)
> type=friend
> host=dynamic
> secret=whatever
> canreinvite=no
> context=sip
> callgroup=1
> pickupgroup=1
> 
> 
> [6001](test)
> [6002](test)
> 
> To reproduce
> -----------
> 6001 calls 100, puts on hold and dials *8 to pickup own call and transfers the the original call (in Eyebeam dials 100 on line 1, dials *8 on line two, hit XFER and click on line 1).
> 
> Before the patch, the macro is executed indefinitely. After, only once.
> 
> 
> Thanks,
> 
> Terry
> 
>