[asterisk-dev] [Code Review] 3046: framehooks: Re-iterate when frame is changed

[Matt Jordan]

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



/branches/12/main/framehook.c
<https://reviewboard.asterisk.org/r/3046/#comment19729>

    I'd scrap the goto loop.
    
    do {
    
    
    } while (frame != original_frame)
    
    does the same without being ew.
    
    The other issue here is that two frame hooks could cancel each other out:
    
    * original_frame = foo
    * frame hook A returns converts foo to bar
    * frame hook A is passed on
    * frame hook B converts bar to foo
    * repeat... :-(
    
    We could ignore this, but if we ever hit this problem, it will be incredibly ugly.
    
    The other option is to clone the framehook list before we start the pass. When a framehook modifies the frame, we remove the framehook from the cloned list and then re-iterate. That guarantees that a framehook is never called twice and prevents this situation.
      



/branches/12/main/framehook.c
<https://reviewboard.asterisk.org/r/3046/#comment19728>

    Blob


My only major fear with this change is that it effectively adds a substantial amount of processing on every frame. It does guarantee corectness, but I think it would be worthwhile to take a look at our framehook implementations to ensure that they aren't doing anything beyond the bare minimum for frames that they don't otherwise consume.

- Matt Jordan


On Dec. 5, 2013, 5:20 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3046/
> -----------------------------------------------------------
> 
> (Updated Dec. 5, 2013, 5:20 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> Currently when a frame is changed by a frame hook previous hooks aren't aware of it. This can be a problem when a previous hook reacts to the types of frames that a subsequent hook produces. An example of this would be the fax gateway hook and the PJSIP T.38 hook. Since the fax gateway hook injects T.38 control frames after the T.38 hook, nothing happens.
> 
> The attached change makes it so if a frame hook produces a different frame the hook iteration loop is restarted, skipping the hook that has produced the frame.
> 
> 
> Diffs
> -----
> 
>   /branches/12/main/framehook.c 403362 
> 
> Diff: https://reviewboard.asterisk.org/r/3046/diff/
> 
> 
> Testing
> -------
> 
> Ran fax tests, confirmed fixed.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20131207/1cbe5066/attachment-0001.html>