[asterisk-dev] [Code Review]: Add variable length displayprompt packet to skinny and use octals

[wedhorn]

> On Feb. 1, 2013, 3:43 p.m., opticron wrote:
> > /trunk/channels/chan_skinny.c, line 2860
> > <https://reviewboard.asterisk.org/r/2294/diff/3/?file=33257#file33257line2860>
> >
> >     I'm not quite sure how the SCCP devices work with regards to this message.  Is this intentionally overwriting the null terminator of the octal string when writing extratext into the string?

This is the correct way. A message with an octal code followed by text will not have a separator. The promptMessage field is a single text field with optional (appears to be 2 byte) octal code at bytes 0 and 1.


> On Feb. 1, 2013, 3:43 p.m., opticron wrote:
> > /trunk/channels/chan_skinny.c, line 2893
> > <https://reviewboard.asterisk.org/r/2294/diff/3/?file=33257#file33257line2893>
> >
> >     Just to be on the safe side, strlen(extratext) should be conditionally added to packetlen only in the first branch above.  Relying on extratext to be "" when *text != '\200' could produce interesting bugs later on.

Good catch. Will fix.


> On Feb. 1, 2013, 3:43 p.m., opticron wrote:
> > /trunk/channels/chan_skinny.c, line 2857
> > <https://reviewboard.asterisk.org/r/2294/diff/3/?file=33257#file33257line2857>
> >
> >     Why not compare with '\200' to avoid the cast and the extra comment?

No idea. Guess that what happens when you're trying to work out what a bit of data does and code it at the same time. Will change.


- wedhorn


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


On Feb. 1, 2013, 3:21 p.m., wedhorn wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/2294/
> -----------------------------------------------------------
> 
> (Updated Feb. 1, 2013, 3:21 p.m.)
> 
> 
> Review request for Asterisk Developers and Brad Latus.
> 
> 
> Summary
> -------
> 
> Add new variable length displayprompt packet (0x0145) to skinny. This uses a similar method to the variable length callinfo packet in https://reviewboard.asterisk.org/r/2290/. Uses the new packet if the device is reporting protocol versions >= 17.
> 
> Add the use of octal codes for sending prompts to both the new and old displayprompt messages (also cleaned up soft_key_template_default to use the defined octal codes).
> 
> 
> Diffs
> -----
> 
>   /trunk/channels/chan_skinny.c 380755 
> 
> Diff: https://reviewboard.asterisk.org/r/2294/diff
> 
> 
> Testing
> -------
> 
> Several calls on devices with protocol versions of 6 and 17. Prompts as expected.
> 
> 
> Thanks,
> 
> wedhorn
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20130201/9a4d668e/attachment.htm>