[asterisk-dev] [Code Review] 3705: res_pjsip_dialog_info_body_generator: Add dialog-info+xml implementation and tweak res_pjsip_pubsub to support it.

[Mark Michelson]

> On July 4, 2014, 3:43 p.m., Matt Jordan wrote:
> > /branches/12/res/res_pjsip_dialog_info_body_generator.c, lines 152-155
> > <https://reviewboard.asterisk.org/r/3705/diff/1/?file=62059#file62059line152>
> >
> >     Doxygen comments on these #defines would be nice, as they aren't 'obvious' sizes (3? 39?)
> 
> Joshua Colp wrote:
>     I'll admit I shamelessly stole these from Mark.

I can explain them:

MAX_STRING_GROWTHS is a limit to how many times the ast_str() for the body text can grow before we declare an XML body to be too large to send. The MAX_STRING_GROWTHS should take into consideration how large a body of a given document type may be, knowing that the initial allocation for the body string is 64 chars. Upon writing this down, I'm realizing that a better way of doing this would be to declare a maximum size for an XML body and ensuring we don't grow beyond that. The current method requires the knowledge of the initial string allocation and would require changes to the define if the initial string allocation changes.

XML_PROLOG deals with a condition I came across while testing. The documentation for pj_xml_print() claims that it will return -1 if there is insufficient space in the buffer to print the XML message. However, setting the fourth argument of pj_xml_print() to PJ_TRUE will cause the XML prolog of "<?xml version="1.0" encoding="UTF-8"?>" to be printed to the buffer unconditionally, and its length (39) is returned if the rest of the document could not be printed to the buffer. Therefore, checking for an error in pj_xml_print() requires checking if the return is 39, not -1. The define could potentially be renamed to something like XML_PROLOG_LEN to more properly indicate what it is, or it could even be defined as XML_PRINT_ERROR or XML_PRINT_INSUFFICIENT_SPACE since we're using the define as a way of checking for an error.


- Mark


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


On July 2, 2014, 8:57 p.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3705/
> -----------------------------------------------------------
> 
> (Updated July 2, 2014, 8:57 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-21443
>     https://issues.asterisk.org/jira/browse/ASTERISK-21443
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This change does the following:
> 
> 1. Provides the underlying subscription to body generators for extension state by placing it in the data structure.
> 2. Makes the ast_sip_presence_xml_create_node function optionally add the created node to a parent node.
> 3. Adds dialog as a supported event to res_pjsip_exten_state.
> 4. Adds a res_pjsip_dialog_info_body_generator module for generating the dialog-info+xml body for NOTIFY messages.
> 
> 
> Diffs
> -----
> 
>   /branches/12/res/res_pjsip_exten_state.c 417756 
>   /branches/12/res/res_pjsip_dialog_info_body_generator.c PRE-CREATION 
>   /branches/12/res/res_pjsip/presence_xml.c 417756 
>   /branches/12/include/asterisk/res_pjsip_presence_xml.h 417756 
>   /branches/12/include/asterisk/res_pjsip_body_generator_types.h 417756 
> 
> Diff: https://reviewboard.asterisk.org/r/3705/diff/
> 
> 
> Testing
> -------
> 
> Used a Grandstream phone to subscribe and watched NOTIFY messages go to it as I did things.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-dev/attachments/20140707/32614877/attachment.html>