[asterisk-dev] [Code Review] 3588: chan_sip: Flip the order of variables supplied to SIPNotify

[Mark Michelson]

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



/branches/1.8/include/asterisk/config.h
<https://reviewboard.asterisk.org/r/3588/#comment22104>

    Recommendation: s/flip/reverse/
    
    "flip" is not a commonly-used API word, whereas "reverse" is universal.


- Mark Michelson


On June 5, 2014, 10:20 p.m., Jonathan Rose wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3588/
> -----------------------------------------------------------
> 
> (Updated June 5, 2014, 10:20 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> While working on https://reviewboard.asterisk.org/r/3587/ I noticed a problem with the following Action:
> 
> Action: PJSIPNotify
> Endpoint: pj123
> Variable: var1=val1
> Variable: var2=val2
> Variable: var3=val3,var4=val4,var5=val5
> 
> When this was received, the order of the variables was reversed.
> 
> This alone wouldn't be a big deal, but it means that if multiple Content variables are specified, they also will be reversed and that is unlikely to be anticipated.
> 
> I checked this problem against SIPNotify as well, and it occurs there too, so I'm fixing it there first.
> 
> This patch keeps the existing functionality of astman_get_variables so that people who use that API don't have their own work-arounds mangled. I've turned the original function into a wrapper to a more fully featured function that allows the intended order to be specified. The original function will still return the list in its current (reversed) order.
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/main/manager.c 415300 
>   /branches/1.8/main/config.c 415300 
>   /branches/1.8/include/asterisk/manager.h 415300 
>   /branches/1.8/include/asterisk/config.h 415300 
>   /branches/1.8/channels/chan_sip.c 415300 
> 
> Diff: https://reviewboard.asterisk.org/r/3588/diff/
> 
> 
> Testing
> -------
> 
> Tested the following action:
> 
> Action: SIPNotify
> Channel: 123
> Variable: var1=val1
> Variable: var2=val2
> Variable: var3=val3,var4=val4,var5=val5
> 
> to confirm correct order of the additional headers.  Also checked
> 
> Action: SIPNotify
> Channel: 123
> Variable: Content=item1
> Variable: Content=item2
> Variable: Content=item3,Content=item4,Content=item5
> 
> to verify that the order of items in the body was correct.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

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