[prev in list] [next in list] [prev in thread] [next in thread] 

List:       asterisk-dev
Subject:    [asterisk-dev] [Code Review] SIP: refactoring parsing functions and
From:       "David Vossel" <dvossel () digium ! com>
Date:       2010-01-30 22:39:52
Message-ID: 20100130223952.5763.14527 () hotblack ! digium ! internal
[Download RAW message or body]


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

Review request for Asterisk Developers.


Summary
-------

----- Changes -----

New files
- sip.h – A new header for shared #define, enum, and struct definitions.
- config-parser.c  – Contains sip.conf parsing helper functions with unit tests.
- request-parser.c – Contains sip request and response parsing helper functions \
with unit tests.

New Unit Tests 
- sip_parse_uri_test
- sip_parse_host_test
- sip_parse_register_line_test

Code Refactoring
- All reusable #define, enum, and struct definitions were moved out of chan_sip.c \
into sip.h. During this process formatting changes were made to comments in both \
                sip.h and chan_sip.c in order to better adhere to the coding \
                guidelines.
- Function prototypes for functions shared among chan_sip.c and the new \
                request-parser.c and config-parser.c files were added to sip.h.
- parse_uri() and get_calleridname() were moved from chan_sip.c to request-parser.c \
                along with unit tests for both functions.
- sip_parse_host() and sip_parse_register_line() were moved from chan_sip.c to \
config-parser.c along with unit tests for both functions.

Changes to parse_uri()
-removal of the options parameter.  It was never used and did not behave correctly.
-additional check for [?header] field. When this field was present, the transport \
type was not being set correctly.

----- Overview -----

This patch is introduced with the hope that unit tests for all our sip parsing \
functions will be written soon.  chan_sip is a huge file, and with the addition of \
each unit test chan_sip is going to grow larger and harder to maintain.  I'm \
proposing we begin refactoring chan_sip, starting with the parsing functions.  With \
each parsing function we move into a separate helper file, a unit test should \
accompany it.  I've attempted to lay down the ground work for this change by creating \
two new parser helper files (config-parser.c and request-parser.c) and moving all \
shared structs, enums, and defines from chan_sip.c into a shared sip.h file.  We \
can't verify everything in Asterisk using unit tests, but string parsing is one area \
where unit tests make the most sense.  By beginning to restructure the code in this \
way, chan_sip not only becomes less bloated, but Asterisk as a whole will become more \
stable.


Diffs
-----

  /trunk/channels/Makefile 244062 
  /trunk/channels/chan_sip.c 244062 
  /trunk/channels/sip/config-parser.c PRE-CREATION 
  /trunk/channels/sip/request-parser.c PRE-CREATION 
  /trunk/channels/sip/sip.h PRE-CREATION 

Diff: https://reviewboard.asterisk.org/r/477/diff


Testing
-------


Thanks,

David


-- 
_____________________________________________________________________
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http://lists.digium.com/mailman/listinfo/asterisk-dev


[prev in list] [next in list] [prev in thread] [next in thread]