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

[wdoekes]

> On Nov. 23, 2011, 12:46 p.m., mjordan wrote:
> > /branches/1.8/channels/chan_sip.c, line 16013
> > <https://reviewboard.asterisk.org/r/1586/diff/1/?file=21725#file21725line16013>
> >
> >     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.
> 
> wdoekes wrote:
>     Agreed that it is better. But IMHO beyond the scope of this fix. I mainly did this patch to get rid of the "XXX" in the somments.
> 
> Terry Wilson wrote:
>     Actually, 1400 is very much *not* arbitrary. It is an RFC supplied value for the maximum length of a SIP MESSAGE request to prevent fragmentation.

It seems there already is a get_msg_text2() in the 10 branch which takes an ast_str.


- wdoekes


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


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/8c7cc032/attachment-0001.htm>