[asterisk-dev] [Code Review] 3780: res_pjsip_outbound_publish / res_pjsip_publish_asterisk: Add outbound PUBLISH support with 'asterisk' event type.

[Kevin Harwell]

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



/trunk/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/3780/#comment22954>

    Should be able to remove the RAII_VAR here and just return the value from sorcery (no reason for the extra ref inc/dec).



/trunk/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/3780/#comment22955>

    uuid_buf goes out of scope here and uid_ptr could potentially point to trash?  Also this could be replace with a call to "ast_uuid_generate_str" instead.



/trunk/res/res_pjsip_outbound_publish.c
<https://reviewboard.asterisk.org/r/3780/#comment22957>

    "Invalid from ..."?



/trunk/res/res_pjsip_outbound_publish.exports.in
<https://reviewboard.asterisk.org/r/3780/#comment22958>

    This could be collapsed into "LINKER_SYMBOL_PREFIXast_sip_*"  That way any future function additions shouldn't need to modify the file.



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22959>

    The json object should be unreferenced not freed.



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22960>

    use "ast_json_unref" on the json object instead of "ast_json_free"



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22963>

    datastore is an RAII_VAR here, but it looks like in all the off nominal/error paths its ref gets removed, so can probably remove the RAII_VAR (and add an unref at the end) or remove all the off nominal unref(s) as they are taken care of when the RAII_VAR goes out of scope.



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22964>

    datastore is an RAII_VAR so the ref decrements on it should be able to be removed.



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22965>

    use "ast_json_unref" here.



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22966>

    use ast_json_unref here as well for the json object.



/trunk/res/res_pjsip_publish_asterisk.c
<https://reviewboard.asterisk.org/r/3780/#comment22967>

    red blob


- Kevin Harwell


On July 14, 2014, 10:25 a.m., Joshua Colp wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3780/
> -----------------------------------------------------------
> 
> (Updated July 14, 2014, 10:25 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> -------
> 
> This adds two PJSIP modules which add outbound PUBLISH support and an 'asterisk' event type.
> 
> The res_pjsip_outbound_publish module is a common module which provides basic logic for setting up outbound PUBLISH clients, handling authentication requests, handling configuration, and lifetime. Extra modules implement specific event types which are registered with res_pjsip_outbound_publish. Since it takes care of configuration when an outbound PUBLISH is configured extra configuration can be passed to the event type implementation to further configure itself.
> 
> The res_pjsip_publish_asterisk module implements inbound and outbound support for an 'asterisk' event type. This event type conveys device and mailbox state between Asterisk instances using a JSON content body. As internal device or mailbox state changes the module sends a PUBLISH message to other configured instances. When a PUBLISH is received the contents are examined and a device or mailbox state change queued up within Asterisk. To restrict what is sent and received filtering is available using regular expressions which can reduce SIP traffic.
> 
> A wiki page is available at https://wiki.asterisk.org/wiki/display/~jcolp/Exchanging+Device+and+Mailbox+State+Using+PJSIP which has some configuration details with some examples. This should also be reviewed.
> 
> 
> Diffs
> -----
> 
>   /trunk/res/res_pjsip_pubsub.exports.in 418582 
>   /trunk/res/res_pjsip_pubsub.c 418582 
>   /trunk/res/res_pjsip_publish_asterisk.c PRE-CREATION 
>   /trunk/res/res_pjsip_outbound_publish.exports.in PRE-CREATION 
>   /trunk/res/res_pjsip_outbound_publish.c PRE-CREATION 
>   /trunk/include/asterisk/res_pjsip_pubsub.h 418582 
>   /trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION 
>   /trunk/configs/pjsip.conf.sample 418582 
> 
> Diff: https://reviewboard.asterisk.org/r/3780/diff/
> 
> 
> Testing
> -------
> 
> Set up two Asterisk instances, configured both sides to publish to eachother, made calls and manipulated voicemail. Watched PUBLISH messages go between them and state change.
> 
> 
> Thanks,
> 
> Joshua Colp
> 
>

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