[asterisk-dev] [Code Review] fix odd logic in get_msg_text() and a couple of related quirks

[mjordan]

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



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1586/#comment9062>

    It seems very arbitrary that we only support message bodies with 1400 characters.  This whole thing seems like it would be much simpler using ast_str.



/branches/1.8/channels/chan_sip.c
<https://reviewboard.asterisk.org/r/1586/#comment9061>

    For SIP MESSAGEs, do we strip the linefeed when creating the sip_request?  If not, this will now append an extra '\n' to each line


- mjordan


On Nov. 13, 2011, 9:02 a.m., wdoekes wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/1586/
> -----------------------------------------------------------
> 
> (Updated Nov. 13, 2011, 9:02 a.m.)
> 
> 
> Review request for Asterisk Developers and Olle E Johansson.
> 
> 
> Summary
> -------
> 
> The chan_sip get_msg_text was a bit of a mess:
> - it contained strlen(buf) where buf was always terminated on 0
> - it reserved 5 bytes for the buffer for who knows what
> - it decremented the allowed size by more than needed if addnewline was off
> - the addnewline parameter was a misnomer -- and imho the wrong fix to a problem
> - the \brief docs referred to a SIP MESSAGE, but it was used to get any sip request body
> 
> I did the following:
> - I fixed the doxygen docs
> - looked over the numbers in get_msg_text() so buf is always filled to the brim if too small
> - removed the addnewline parameter that Olle added in r116240
> - instead, I stripped all trailing newlines in the function that needed it
> 
> I believe r116240 was the wrong fix for the problem of trailing newlines: if the content is text/plain, newlines should certainly be allowed. Unless they're somehow used for folding the text/plain body -- which I haven't found any docs about -- removing them could cause linefeed separated text to be incorrectly joined.
> 
> (This is not an important issue at all.. but this review is here to clean up the patch in r1533.)
> 
> 
> This addresses bug ASTERISK-18389.
>     https://issues.asterisk.org/jira/browse/ASTERISK-18389
> 
> 
> Diffs
> -----
> 
>   /branches/1.8/channels/chan_sip.c 345022 
> 
> Diff: https://reviewboard.asterisk.org/r/1586/diff
> 
> 
> Testing
> -------
> 
> It compiles.
> 
> 
> Thanks,
> 
> wdoekes
> 
>

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