[asterisk-dev] [Code Review] Convert character pointers in sip_request structure to be offsets from the beginning of the buffer

[Mark Michelson]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.digium.com/r/126/
-----------------------------------------------------------

(Updated 2009-01-19 16:02:24.575177)


Review request for Asterisk Developers, Russell Bryant and Kevin Fleming.


Changes
-------

I have made the changes outlined by Kevin.


Summary
-------

chan_sip underwent a change to use an ast_str structure inside the sip_request structure in order to allow for arbitrary
packet sizes (to a certain extent). This also gives us the benefit of making the assumption early on that a packet will be
small (less than 1024 chars) instead of hard-coding 4K for every packet.

One problem that arose from this conversion was that while the actual buffer in the sip_request was changed, other pointers
in the sip_request structure were not updated appropriately. The way these previously worked was to point to an appropriate 
spot within the static buffer so that information could be easily referenced via these pointers. The problem that has occurred 
is that if the ast_str expands to allow more characters to be written, these pointers become invalid since they are pointing
to "old" memory.

The rlPart1, rlPart2, header array, and line array have been changed from char * to ptrdiff_t. These represent the offsets
from the beginning of the string. This way, the offset can be applied to the beginning of the string even if it is reallocated
due to a size increase. To ease things a bit, I have included a macro called REQ_OFFSET_TO_STR which, if supplied with a sip_request
pointer and one of the offsets, will do the pointer math for you and give you a string to work with.

My tests show that my logic is sound; however, I would like to have some more eyes look at this in case there are suggestions 
for how to improve things further. I gave some serious thought to rewriting the parse_request and determine_firstline_parts
functions from scratch since I was already modifying them, but decided instead to just add my new logic into what existed.

Also, if someone has a better idea for a name than REQ_OFFSET_TO_STR, I'm all ears.


This addresses bug 14220.
    http://bugs.digium.com/view.php?id=14220


Diffs (updated)
-----

  /trunk/channels/chan_sip.c 169442 

Diff: http://reviewboard.digium.com/r/126/diff


Testing
-------

I tested by running calls from one SIP phone through Asterisk to another SIP phone. The calls connected just
fine, and no errors, warnings, or other strange messages appeared on the console. I also turned on SIP debug
just to be sure that the messages looked correct and they did. I made sure to run some tests where the size of
SIP requests originating from Asterisk were longer than 1024 characters, too. Finally, I made the same calls
while running Asterisk under Valgrind's memcheck tool. Valgrind made no complaints during these tests.


Thanks,

Mark