date:20140721


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

(Updated July 21, 2014, 4:40 a.m.)


Review request for Asterisk Developers.


Changes
---

Add JIRA ticket


Bugs: ASTERISK-24066
https://issues.asterisk.org/jira/browse/ASTERISK-24066


Repository: Asterisk


Description
---

This code converts res_smdi to use astobj2.

* Remove functions: ast_smdi_interface_unref, ast_smdi_md_message_putback, 
ast_smdi_mwi_message_putback, ast_smdi_md_message destructor, 
ast_smdi_mwi_message destructor.

The putback methods were unused and difficult to reimplement using 
ao2_containers.

Includes for astobj.h are removed everywhere it's possible.  The only remaining 
users are netsock.c (deprecated) and chan_sip (to be updated in a separate 
review).


Diffs
-

  /trunk/res/res_smdi.c 418788 
  /trunk/include/asterisk/smdi.h 418788 
  /trunk/channels/sig_analog.c 418788 
  /trunk/channels/chan_motif.c 418788 
  /trunk/channels/chan_dahdi.c 418788 
  /trunk/apps/app_voicemail.c 418788 

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


Testing
---

Compile only (I do not have SMDI hardware).


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3758: res_smdi: convert to astobj2


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

(Updated July 21, 2014, 3:41 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419060


Bugs: ASTERISK-24066
https://issues.asterisk.org/jira/browse/ASTERISK-24066


Repository: Asterisk


Description
---

This code converts res_smdi to use astobj2.

* Remove functions: ast_smdi_interface_unref, ast_smdi_md_message_putback, 
ast_smdi_mwi_message_putback, ast_smdi_md_message destructor, 
ast_smdi_mwi_message destructor.

The putback methods were unused and difficult to reimplement using 
ao2_containers.

Includes for astobj.h are removed everywhere it's possible.  The only remaining 
users are netsock.c (deprecated) and chan_sip (to be updated in a separate 
review).


Diffs
-

  /trunk/res/res_smdi.c 418788 
  /trunk/include/asterisk/smdi.h 418788 
  /trunk/channels/sig_analog.c 418788 
  /trunk/channels/chan_motif.c 418788 
  /trunk/channels/chan_dahdi.c 418788 
  /trunk/apps/app_voicemail.c 418788 

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


Testing
---

Compile only (I do not have SMDI hardware).


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2


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

(Updated July 21, 2014, 4:54 a.m.)


Review request for Asterisk Developers.


Changes
---

Add JIRA ticket.


Bugs: ASTERISK-24067
https://issues.asterisk.org/jira/browse/ASTERISK-24067


Repository: Asterisk


Description
---

Upgrade all ASTOBJ objects in chan_sip to ao2.


Diffs
-

  /trunk/channels/sip/include/sip.h 419043 
  /trunk/channels/chan_sip.c 419043 

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


Testing
---

Full testsuite run.


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3811: Move main/manager_*.c to loadable modules


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

(Updated July 21, 2014, 4:56 a.m.)


Review request for Asterisk Developers.


Changes
---

Add JIRA ticket.


Bugs: ASTERISK-24068
https://issues.asterisk.org/jira/browse/ASTERISK-24068


Repository: Asterisk


Description
---

This change moves main/manager_*.c to loadable modules, allowing those events 
to be disabled by not loading the modules.  This can be accomplished by 
eventfilter, but eventfilter has a couple issues.  It actually adds more 
overhead to asterisk since the outbound events must be parsed for each AMI 
user.  Additionally it causes skips in SequenceNumber, preventing use of that 
tag to determine if any events were missed during a reconnect.

Besides converting from built-in units to modules, changes are made to VarSet, 
ChannelTalkingStart and ChannelTalkingStop.  They no longer use .to_ami 
callbacks, but instead subscribe to the stasis events like the rest of the 
res_manager_channels events.  A couple functions were also moved from 
manager_bridging.c and manager_channels.c to manager.c since they are still 
needed even if these modules are noload'ed.

AST_MODULE_INFO_STANDARD for all modules will be updated once r3802 is 
committed.  This or r3812 will need to be updated depending on which is 
committed first.


Diffs
-

  /trunk/main/stasis_channels.c 418738 
  /trunk/main/manager_system.c HEAD 
  /trunk/main/manager_system.c 418738 
  /trunk/main/manager_mwi.c HEAD 
  /trunk/main/manager_mwi.c 418738 
  /trunk/main/manager_endpoints.c HEAD 
  /trunk/main/manager_endpoints.c 418738 
  /trunk/main/manager_channels.c HEAD 
  /trunk/main/manager_channels.c 418738 
  /trunk/main/manager_bridges.c HEAD 
  /trunk/main/manager_bridges.c 418738 
  /trunk/main/manager.c 418738 
  /trunk/include/asterisk/manager.h 418738 

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


Testing
---

Ran some testsuite's to verify some of the events were still being sent to AMI:
tests/manager/originate
tests/apps/channel_redirect
tests/bridge/connected_line_update
tests/feature_call_pickup
tests/apps/dial/dial_answer
tests/apps/chanspy/chanspy_barge
tests/funcs/func_push

This did not provide complete coverage for all effected events, but does verify 
many events from res_manager_channels.c.  Events from other files were not 
tested, though res_manager_channels.c was the most likely to cause problems.


Thanks,

Corey Farrell

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3818: Deprecate astobj.h


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

(Updated July 21, 2014, 4:59 a.m.)


Review request for Asterisk Developers.


Changes
---

Add JIRA ticket.


Bugs: ASTERISK-24069
https://issues.asterisk.org/jira/browse/ASTERISK-24069


Repository: Asterisk


Description
---

This flags astobj.h as deprecated, warns people to use astobj2.h instead.  
After 3758 and 3759 are committed the only thing that will still use astobj.h 
is netsock.c, and that is already deprecated.


Diffs
-

  /trunk/include/asterisk/astobj.h 418788 

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


Testing
---


Thanks,

Corey Farrell

-- 
_
-- 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

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

On July 16, 2014, 4:54 p.m., Kevin Harwell wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 203-212
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line203

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

This is actually returning the state, not the publish configuration itself.

- Joshua

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

On July 14, 2014, 3:25 p.m., Joshua Colp wrote:

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

(Updated July 14, 2014, 3:25 p.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

--
_
-- 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

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

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

(Updated July 21, 2014, 12:14 p.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_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.

Diffs (updated)
-

/trunk/res/res_pjsip_pubsub.exports.in 419075
/trunk/res/res_pjsip_pubsub.c 419075
/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 419075
/trunk/include/asterisk/res_pjsip_outbound_publish.h PRE-CREATION

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

--
_
-- 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

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

On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 530-568
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line530

None of this is necessary. pjsip_publishc_init() parses URIs and will
return PJSIP_EINVALIDURI if any URIs are invalid. You won't have the
granularity that you currently have here, but you'll save on the repeated
parsing of URIs.

This actually came up when doing the outbound registration work. People want
the granularity so they can see, in more detail, what part of their
configuration is incorrect.

On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71

This seems like an odd default behavior. I would suspect that by
default we would actually magic up a From URI of our own based instead of
telling the server that the message is coming from itself.

Doing this when no to_uri is provided makes good sense though.

It's a PJSIPism, and it actually makes sense (imo). In the case of PUBLISH you
are publishing information about yourself usually. It makes sense that it
should be coming from you.

On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_publish_asterisk.c, lines 428-430
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63304#file63304line428

Is there some sort of stasis cache removal you could perform here?

So the best I could do is:

1. For device state do a full cache dump, run the regex against, and set
everything it matches to unknown.
2. For mailbox state do a full cache dump, run the regex against, and set every
mailbox to 0/0

I would also need to extend things to store a bit of state information about
the publisher we are receiving updates from (specifically the eid).

Thoughts?

- Joshua

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

On July 14, 2014, 3:25 p.m., Joshua Colp wrote:

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

(Updated July 14, 2014, 3:25 p.m.)

Review request for Asterisk Developers.

Repository: Asterisk

Description
---

This adds two PJSIP modules which add outbound PUBLISH support and an
'asterisk' event type.

Diffs
-

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

--
_
-- 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

Re: [asterisk-dev] [Code Review] 3831: res_fax: unregister manager actions on unload


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

Ship it!


Ship It!

- opticron


On July 18, 2014, 7:29 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3831/
 ---
 
 (Updated July 18, 2014, 7:29 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24058
 https://issues.asterisk.org/jira/browse/ASTERISK-24058
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Unregisters manager actions FAXSessions, FAXSession and FAXStats at shutdown.
 
 Additionally make ast_manager_register2 use ao2_t_alloc with the action name 
 for the tag, to make it easier to track leaks like this in the future.
 
 
 Diffs
 -
 
   /trunk/res/res_fax.c 419018 
   /trunk/main/manager.c 419018 
 
 Diff: https://reviewboard.asterisk.org/r/3831/diff/
 
 
 Testing
 ---
 
 Verified that tests/manager/originate no longer produces REF_DEBUG leaks.
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- 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

[asterisk-dev] [Code Review] 3781: Retrieve the source port of an incoming (chan_sip) SIP invite

2014-07-21 Thread dtryba


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

Review request for Asterisk Developers.


Bugs: ASTERISK-24040
https://issues.asterisk.org/jira/browse/ASTERISK-24040


Repository: Asterisk


Description
---

Retrieve the source port of an incoming (chan_sip) SIP invite in the dialplan 
with ${CHANNEL(recvport)}
To complement ${CHANNEL(recvip)} and enable me to make dialplan decisions based 
on source port (in a peerless setup, handle everything as guests using AGI to 
lookup source ip/port for routing/handling).

pjsip appears to have this capability through the CHANNEL function 
(pjsip,local_addr/remote_addr).

Simple 2 line patch using ast_sockaddr_stringify_fmt(const struct ast_sockaddr 
*sa, int format)
to return the port as a string.


Diffs
-

  /trunk/channels/sip/dialplan_functions.c 418610 

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


Testing
---

Tested on 11.10.2 (Debian Jessie) and trunk (418610) using IPv4. Having a few 
SIP endpoints connect from different address/ports combinations 
Logged ${CHANNEL(recvip)}:${CHANNEL(recvport)} corresponds with source ip:port 
in packetdumps on the asterisk machine.


Thanks,

dtryba

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2


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



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23085

This will destroy the container on reloads.


- opticron


On July 21, 2014, 3:54 a.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3759/
 ---
 
 (Updated July 21, 2014, 3:54 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24067
 https://issues.asterisk.org/jira/browse/ASTERISK-24067
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Upgrade all ASTOBJ objects in chan_sip to ao2.
 
 
 Diffs
 -
 
   /trunk/channels/sip/include/sip.h 419043 
   /trunk/channels/chan_sip.c 419043 
 
 Diff: https://reviewboard.asterisk.org/r/3759/diff/
 
 
 Testing
 ---
 
 Full testsuite run.
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3817: res_pjsip_notify: Add the ability for PJSIPNotify AMI command and pjsip send notify CLI command to send to a URI instead of an endpoint



 On July 18, 2014, 3:40 p.m., Mark Michelson wrote:
  /trunk/include/asterisk/res_pjsip.h, lines 1195-1203
  https://reviewboard.asterisk.org/r/3817/diff/2/?file=64713#file64713line1195
 
  This function isn't necessary. When PJSIP is passed a URI string, PJSIP 
  will perform URI validation for us and return an error if a badly-formed 
  URI is passed in.
 
 Jonathan Rose wrote:
 I was absolutely getting crashes when using URIs that didn't start with 
 'sip:' or else were only 'sip:' without anything appended to them.  I'm not 
 sure if that will keep being the case if we use the default endpoint like 
 suggested below, but if it does then I might need to keep this around.
 
 Jonathan Rose wrote:
 After testing it with the default outbound endpoint, it was still able to 
 cause crashes. Rats.

Where does the crash occur? Is it in Asterisk's lower-level res_pjsip code, or 
is it in PJSIP itself? If the crash is in Asterisk, it sounds like there are 
some assumptions being made in the lower levels that shouldn't be made and 
those should be fixed there. If the crash is in PJSIP (especially if it's an 
assertion failure), then that means we do have to do some validation before 
calling down into PJSIP, but that validation should be left to the core 
res_pjsip code rather than being the duty of users of the res_pjsip API.

If we do need to perform validation of the URI, the implementation has some 
issues. It's limiting since it only allows for sip: URIs and not, for instance, 
sips: URIs. It also only checks that the prefix is valid for a sip: URI. It 
does no other checking of the rest of the URI. A more thorough way of checking 
validity of the URI is to use pjsip_parse_uri() from PJSIP. If you then want to 
make sure that the URI is a sip: or sips: URI, you can use the 
PJSIP_URI_SCHEME_IS_SIP() and PJSIP_URI_SCHEME_IS_SIPS() macros to do so.


- Mark


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


On July 18, 2014, 6:32 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3817/
 ---
 
 (Updated July 18, 2014, 6:32 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Example:
 
 Action: PJSIPNotify
 URI: sip:10.10.10.10
 
 pjsip send notify uri type uri uri [uri...]
 
 
 Adds the ability to use URIs with the PJSIP notify commands instead of 
 endpoints. These mostly work the same as the endpoint notifications.
 
 
 Diffs
 -
 
   /trunk/res/res_pjsip_notify.c 418868 
   /trunk/res/res_pjsip.c 418868 
   /trunk/include/asterisk/res_pjsip.h 418868 
 
 Diff: https://reviewboard.asterisk.org/r/3817/diff/
 
 
 Testing
 ---
 
 Used existing endpoint notification and compared it to similar notifies done 
 against URIs.  The outgoing messages looked the same as far as the details I 
 was concerned with went.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3819: Substitute Variables In Features Application Map Before Execution


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

Ship it!



/trunk/main/bridge_channel.c
https://reviewboard.asterisk.org/r/3819/#comment23087

Super pedantic nit-pick:

I'd use substituted_args as opposed to substitutedargs.


- Matt Jordan


On July 18, 2014, 3:51 p.m., Michael Young wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3819/
 ---
 
 (Updated July 18, 2014, 3:51 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-22608
 https://issues.asterisk.org/jira/browse/ASTERISK-22608
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Say you wanted to include variables in an application map and have those 
 variables substituted and passed along to the application being executed; 
 currently this does not happen.
 This patch adds this new feature.
 
 This simple patch was originally written for 1.4 but I have been maintaining 
 it all the way up to version 11.  It is time to see if anyone else sees any 
 value in this and get it committed.
 
 
 Diffs
 -
 
   /trunk/main/bridge_channel.c 418910 
   /trunk/CHANGES 418910 
 
 Diff: https://reviewboard.asterisk.org/r/3819/diff/
 
 
 Testing
 ---
 
 This has been in production since 1.4 and updated for trunk.  The trunk 
 version of the patch was tested on dev machine.
 
 
 Thanks,
 
 Michael Young
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3830: Fix build when pjproject is installed in non-standard location


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

Ship it!


Ship It!

- Matt Jordan


On July 18, 2014, 2:15 p.m., Sean Bright wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3830/
 ---
 
 (Updated July 18, 2014, 2:15 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When configuring Asterisk to build against a version of pjproject installed 
 in a non-standard location, the checks for PJSIP Transaction Group Lock 
 Support and PJSIP Media Stream Replacement Support fail.  This is because 
 the secondary checks are not taking the CFLAGS and LIBS returned by the 
 pkg-config check into account.
 
 Given an install of pjproject at /opt/pjsip, the following calls to configure 
 fail:
 
 $ ./configure --with-pjproject=/opt/pjsip
 $ PKG_CONFIG_PATH=/opt/pjsip/lib/pkgconfig ./configure
 
 The first fails because the only check we do for pjproject is with pkg-config 
 and the two related checks will not be run because the first fails.  The 
 second incorrectly determines that the two optional features are not present 
 when they are, causing a failure at compile time.  This works:
 
 $ PKG_CONFIG_PATH=/opt/pjsip/lib/pkgconfig ./configure 
 --with-pjproject=/opt/pjsip
 
 Because the primary check succeeds with pkg-config and the optional features 
 succeed using the PJPROJECT_DIR variable setup by the --with-pjproject 
 argument.
 
 While the included diff is certainly not the cleanest, it allows me to 
 configure and compile Asterisk using:
 
 $ PKG_CONFIG_PATH=/opt/pjsip/lib/pkgconfig ./configure
 
 
 Diffs
 -
 
   /trunk/configure.ac 418979 
   /trunk/configure UNKNOWN 
 
 Diff: https://reviewboard.asterisk.org/r/3830/diff/
 
 
 Testing
 ---
 
 Configured, compiled.
 
 
 Thanks,
 
 Sean Bright
 


-- 
_
-- 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

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



 On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_outbound_publish.c, lines 71-75
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71
 
  This seems like an odd default behavior. I would suspect that by 
  default we would actually magic up a From URI of our own based instead of 
  telling the server that the message is coming from itself.
  
  Doing this when no to_uri is provided makes good sense though.
 
 Joshua Colp wrote:
 It's a PJSIPism, and it actually makes sense (imo). In the case of 
 PUBLISH you are publishing information about yourself usually. It makes sense 
 that it should be coming from you.

Hm, now I'm a bit confused about terminology being used. Who is the server in 
the server_uri? The XML docs say SIP URI of the server and entity to publish 
to . So in other words, it's the URI of the destination of the PUBLISH 
request. So let's say the Asterisk server that sends the PUBLISH is Alice, and 
the Asterisk server that receives the PUBLISH is Bob. You set 
server_uri=b...@example.com and do not set a from_uri or to_uri. Our SIP 
request would look something like:

PUBLISH sip:b...@example.com
To: sip:b...@example.com
From: sip:b...@example.com;tag=blahblahblah

That's what's intended? Alice is publishing information about Alice, not Bob. 
If the From header is supposed to indicate the source of the publication, I'd 
expect that the From would give some indication that the publication is from 
Alice.


 On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_publish_asterisk.c, lines 428-430
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63304#file63304line428
 
  Is there some sort of stasis cache removal you could perform here?
 
 Joshua Colp wrote:
 So the best I could do is:
 
 1. For device state do a full cache dump, run the regex against, and set 
 everything it matches to unknown.
 2. For mailbox state do a full cache dump, run the regex against, and set 
 every mailbox to 0/0
 
 I would also need to extend things to store a bit of state information 
 about the publisher we are receiving updates from (specifically the eid).
 
 Thoughts?

I wasn't thinking of simply marking devstates to unknown and mailboxes to 0/0. 
I was thinking of actually removing the items from the cache entirely.

Does a single publication convey the state of multiple resources? In other 
words, is the same publication used to transmit the state of PJSIP/Alice at eid 
12345 and PJSIP/bob at eid 67890? If so, then I don't necessarily see how you 
could actually remove cache items on a publication expiration, since you could 
presumably still be getting those states published from a separate publisher 
entirely. However, if a publisher is guaranteed to only publish a single 
resource or just resources that are local to its eid, then upon publication 
expiration, you could remove the specific resource from the cache or remove all 
resources from that particular eid.


 On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_outbound_publish.c, lines 530-568
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line530
 
  None of this is necessary. pjsip_publishc_init() parses URIs and will 
  return PJSIP_EINVALIDURI if any URIs are invalid. You won't have the 
  granularity that you currently have here, but you'll save on the repeated 
  parsing of URIs.
 
 Joshua Colp wrote:
 This actually came up when doing the outbound registration work. People 
 want the granularity so they can see, in more detail, what part of their 
 configuration is incorrect.

How about this, then. Don't perform URI validation prior to 
pjsip_publishc_init(). If pjsip_publishc_init() returns PJSIP_EINVALIDURI, then 
you can pass the three supplied URIs through pjsip_parse_uri() to be able to 
report which of the three has issues. That way, for the nominal case, the URIs 
only get validated once by the PJSIP code. Only on the off-nominal path are the 
URIs parsed multiple times.


- Mark


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


On July 21, 2014, 12:14 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3780/
 ---
 
 (Updated July 21, 2014, 12:14 p.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

Re: [asterisk-dev] [Code Review] 3760: ARI: Fix endpoint/channel subscription issues; allow for subscriptions to endpoint technologies


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

(Updated July 21, 2014, 9:48 a.m.)


Review request for Asterisk Developers.


Changes
---

Address Kinsey's finding.


Bugs: ASTERISK-23692
https://issues.asterisk.org/jira/browse/ASTERISK-23692


Repository: Asterisk


Description
---

This is the preliminary work needed for ASTERISK-23692, which allows for 
sending/receiving arbitrary out of call text messages through ARI in a 
technology agnostic fashion.

The messaging functionality described on ASTERISK-23692 requires two things:
(1) The ability to send/receive messages associated with an endpoint. This is 
relatively straight forwards with the endpoint core in Asterisk now.
(2) The ability to send/receive messages associated with a technology and an 
arbitrary technology defined URI. This is less straight forward, as endpoints 
are formed from a tech + resource pair. We don't have a mechanism to note that 
a technology that *may* have endpoints exists.

This patch provides such a mechanism, and fixes a few bugs along the way.

The first major bug this patch fixes is the forwarding of channel messages to 
their respective endpoints. Prior to this patch, there were two problems:
(1) Channel caching messages weren't forwarded. Thus, the endpoints missed most 
of the interesting bits (such as channel creation, destruction, state changes, 
etc.)
(2) Channels weren't associated with their endpoint until after creation. This 
resulted in endpoints missing the channel creation message, which limited the 
usefulness of the subscription in the first place (a major use case being 'tell 
me when this endpoint has a channel'). Unfortunately, this meant another 
parameter to ast_channel_alloc. Since not all channel technologies support an 
ast_endpoint, this patch makes such a call optional and opts for a new 
function, ast_channel_alloc_with_endpoint.

When endpoints are created, they will implicitly create a technology endpoint 
for their technology (if one does not already exist). A technology endpoint is 
special in that it has no state, cannot have channels created for it, cannot be 
created explicitly, and cannot be destroyed except on shutdown. It does, 
however, have all messages from other endpoints in its technology forwarded to 
it.

Combined with the bug fixes, we now have Stasis messages being properly 
forwarded. Consider the following scenario: two PJSIP endpoints (foo and bar), 
where bar has a single channel associated with it and foo has two channels 
associated with it. The messages would be forwarded as follows:

ast_channel (PJSIP/foo-1) --
\
 -- ast_endpoint (PJSIP/foo) --
/   \
ast_channel (PJSIP/foo-2) -- \
    
ast_endpoint (PJSIP)
 /
ast_channel (PJSIP/bar-1) - ast_endpoint (PJSIP/bar) --

ARI, through the applications resource, can:
 - subscribe to endpoint:PJSIP/foo and get notifications for channels 
PJSIP/foo-1,PJSIP/foo-2 and endpoint PJSIP/foo
 - subscribe to endpoint:PJSIP/bar and get notifications for channels 
PJSIP/bar-1 and endpoint PJSIP/bar
 - subscribe to endpoint:PJSIP and get notifications for channels 
PJSIP/foo-1,PJSIP/foo-2,PJSIP/bar-1 and endpoints PJSIP/foo,PJSIP/bar

Note that since endpoint PJSIP never changes, it never has events itself. It 
merely provides an aggregation point for all other endpoints in its technology 
(which in turn aggregate all channel messages associated with that endpoint).

This patch also adds endpoints to res_xmpp and chan_motif, because the actual 
messaging work will need it (messaging without XMPP is just sad)


Diffs (updated)
-

  /branches/12/rest-api/api-docs/applications.json 419075 
  /branches/12/res/res_xmpp.c 419075 
  /branches/12/res/ari/resource_endpoints.c 419075 
  /branches/12/res/ari/resource_applications.h 419075 
  /branches/12/main/endpoints.c 419075 
  /branches/12/main/channel_internal_api.c 419075 
  /branches/12/main/channel.c 419075 
  /branches/12/include/asterisk/xmpp.h 419075 
  /branches/12/include/asterisk/endpoints.h 419075 
  /branches/12/include/asterisk/channel.h 419075 
  /branches/12/channels/chan_sip.c 419075 
  /branches/12/channels/chan_pjsip.c 419075 
  /branches/12/channels/chan_motif.c 419075 
  /branches/12/channels/chan_iax2.c 419075 

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


Testing
---

Automated tests were written and are up on 
https://reviewboard.asterisk.org/r/3762

In addition, OpenFire was set up and HTTP requests manually made to verify the 
XMPP endpoint had the appropriate state.


Thanks,

Matt Jordan

--

Re: [asterisk-dev] [Code Review] 3830: Fix build when pjproject is installed in non-standard location

2014-07-21 Thread Sean Bright


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

(Updated July 21, 2014, 9:49 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 419077


Repository: Asterisk


Description
---

When configuring Asterisk to build against a version of pjproject installed in 
a non-standard location, the checks for PJSIP Transaction Group Lock Support 
and PJSIP Media Stream Replacement Support fail.  This is because the 
secondary checks are not taking the CFLAGS and LIBS returned by the pkg-config 
check into account.

Given an install of pjproject at /opt/pjsip, the following calls to configure 
fail:

$ ./configure --with-pjproject=/opt/pjsip
$ PKG_CONFIG_PATH=/opt/pjsip/lib/pkgconfig ./configure

The first fails because the only check we do for pjproject is with pkg-config 
and the two related checks will not be run because the first fails.  The second 
incorrectly determines that the two optional features are not present when they 
are, causing a failure at compile time.  This works:

$ PKG_CONFIG_PATH=/opt/pjsip/lib/pkgconfig ./configure 
--with-pjproject=/opt/pjsip

Because the primary check succeeds with pkg-config and the optional features 
succeed using the PJPROJECT_DIR variable setup by the --with-pjproject argument.

While the included diff is certainly not the cleanest, it allows me to 
configure and compile Asterisk using:

$ PKG_CONFIG_PATH=/opt/pjsip/lib/pkgconfig ./configure


Diffs
-

  /trunk/configure.ac 418979 
  /trunk/configure UNKNOWN 

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


Testing
---

Configured, compiled.


Thanks,

Sean Bright

-- 
_
-- 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

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



 On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_outbound_publish.c, lines 71-75
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71
 
  This seems like an odd default behavior. I would suspect that by 
  default we would actually magic up a From URI of our own based instead of 
  telling the server that the message is coming from itself.
  
  Doing this when no to_uri is provided makes good sense though.
 
 Joshua Colp wrote:
 It's a PJSIPism, and it actually makes sense (imo). In the case of 
 PUBLISH you are publishing information about yourself usually. It makes sense 
 that it should be coming from you.
 
 Mark Michelson wrote:
 Hm, now I'm a bit confused about terminology being used. Who is the 
 server in the server_uri? The XML docs say SIP URI of the server and 
 entity to publish to . So in other words, it's the URI of the destination of 
 the PUBLISH request. So let's say the Asterisk server that sends the PUBLISH 
 is Alice, and the Asterisk server that receives the PUBLISH is Bob. You set 
 server_uri=b...@example.com and do not set a from_uri or to_uri. Our SIP 
 request would look something like:
 
 PUBLISH sip:b...@example.com
 To: sip:b...@example.com
 From: sip:b...@example.com;tag=blahblahblah
 
 That's what's intended? Alice is publishing information about Alice, not 
 Bob. If the From header is supposed to indicate the source of the 
 publication, I'd expect that the From would give some indication that the 
 publication is from Alice.

The name of the entity you are publishing to is generally yourself. I am Alice 
and I am publishing the state of Alice. This is speaking from a SIP 
perspective. You'd use server_uri=sip:al...@example.com. This only differs if 
you are publishing to an entity which is different than your own. I am Alice 
but I'm publishing the state of Bob.

server_uri=sip:b...@example.com
from_uri=sip:al...@example.com


 On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_publish_asterisk.c, lines 428-430
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63304#file63304line428
 
  Is there some sort of stasis cache removal you could perform here?
 
 Joshua Colp wrote:
 So the best I could do is:
 
 1. For device state do a full cache dump, run the regex against, and set 
 everything it matches to unknown.
 2. For mailbox state do a full cache dump, run the regex against, and set 
 every mailbox to 0/0
 
 I would also need to extend things to store a bit of state information 
 about the publisher we are receiving updates from (specifically the eid).
 
 Thoughts?
 
 Mark Michelson wrote:
 I wasn't thinking of simply marking devstates to unknown and mailboxes to 
 0/0. I was thinking of actually removing the items from the cache entirely.
 
 Does a single publication convey the state of multiple resources? In 
 other words, is the same publication used to transmit the state of 
 PJSIP/Alice at eid 12345 and PJSIP/bob at eid 67890? If so, then I don't 
 necessarily see how you could actually remove cache items on a publication 
 expiration, since you could presumably still be getting those states 
 published from a separate publisher entirely. However, if a publisher is 
 guaranteed to only publish a single resource or just resources that are local 
 to its eid, then upon publication expiration, you could remove the specific 
 resource from the cache or remove all resources from that particular eid.

A single publication currently conveys only a single resource (device or 
mailbox). A PUBLISH will only be sent for updates from the originating system, 
it does not act as a relay and thus has one eid. As it is I don't think I can 
safely manipulate the cache to produce the desired result. I can only change 
them to some unknown values.

Matt - Thoughts?


- Joshua


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


On July 21, 2014, 12:14 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3780/
 ---
 
 (Updated July 21, 2014, 12:14 p.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

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

On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71

This seems like an odd default behavior. I would suspect that by
default we would actually magic up a From URI of our own based instead of
telling the server that the message is coming from itself.

Doing this when no to_uri is provided makes good sense though.

Joshua Colp wrote:
It's a PJSIPism, and it actually makes sense (imo). In the case of
PUBLISH you are publishing information about yourself usually. It makes sense
that it should be coming from you.

Mark Michelson wrote:
Hm, now I'm a bit confused about terminology being used. Who is the
server in the server_uri? The XML docs say SIP URI of the server and
entity to publish to . So in other words, it's the URI of the destination of
the PUBLISH request. So let's say the Asterisk server that sends the PUBLISH
is Alice, and the Asterisk server that receives the PUBLISH is Bob. You set
server_uri=b...@example.com and do not set a from_uri or to_uri. Our SIP
request would look something like:

PUBLISH sip:b...@example.com
To: sip:b...@example.com
From: sip:b...@example.com;tag=blahblahblah

That's what's intended? Alice is publishing information about Alice, not
Bob. If the From header is supposed to indicate the source of the
publication, I'd expect that the From would give some indication that the
publication is from Alice.

Joshua Colp wrote:
The name of the entity you are publishing to is generally yourself. I am
Alice and I am publishing the state of Alice. This is speaking from a SIP
perspective. You'd use server_uri=sip:al...@example.com. This only differs if
you are publishing to an entity which is different than your own. I am Alice
but I'm publishing the state of Bob.

server_uri=sip:b...@example.com
from_uri=sip:al...@example.com

Ah, so the server URI is in essence the URI of the resource whose state is
being published. It's not the URI of the UAS that is to receive the PUBLISH
request. If that's the case, then is the to_uri used to populate the RURI of
the PUBLISH request?

- Mark

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

On July 21, 2014, 12:14 p.m., Joshua Colp wrote:

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

(Updated July 21, 2014, 12:14 p.m.)

Review request for Asterisk Developers.

Repository: Asterisk

Description
---

This adds two PJSIP modules which add outbound PUBLISH support and an
'asterisk' event type.

Diffs
-

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.

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

On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71

This seems like an odd default behavior. I would suspect that by
default we would actually magic up a From URI of our own based instead of
telling the server that the message is coming from itself.

Doing this when no to_uri is provided makes good sense though.

Joshua Colp wrote:
It's a PJSIPism, and it actually makes sense (imo). In the case of
PUBLISH you are publishing information about yourself usually. It makes sense
that it should be coming from you.

PUBLISH sip:b...@example.com
To: sip:b...@example.com
From: sip:b...@example.com;tag=blahblahblah

server_uri=sip:b...@example.com
from_uri=sip:al...@example.com

Mark Michelson wrote:
Ah, so the server URI is in essence the URI of the resource whose state
is being published. It's not the URI of the UAS that is to receive the
PUBLISH request. If that's the case, then is the to_uri used to populate the
RURI of the PUBLISH request?

It just controls the To URI.

server_uri = Target URI (including user for request URI and the location of
where to send it)
to_uri = URI in To header
from_uri = URI in From header

- Joshua

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

On July 21, 2014, 12:14 p.m., Joshua Colp wrote:

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

(Updated July 21, 2014, 12:14 p.m.)

Review request for Asterisk Developers.

Repository: Asterisk

Description
---

This adds two PJSIP modules which add outbound PUBLISH support and an
'asterisk' event type.

Diffs
-

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

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

On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
/trunk/res/res_pjsip_outbound_publish.c, lines 71-75
https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71

This seems like an odd default behavior. I would suspect that by
default we would actually magic up a From URI of our own based instead of
telling the server that the message is coming from itself.

Doing this when no to_uri is provided makes good sense though.

Joshua Colp wrote:
It's a PJSIPism, and it actually makes sense (imo). In the case of
PUBLISH you are publishing information about yourself usually. It makes sense
that it should be coming from you.

PUBLISH sip:b...@example.com
To: sip:b...@example.com
From: sip:b...@example.com;tag=blahblahblah

server_uri=sip:b...@example.com
from_uri=sip:al...@example.com

Joshua Colp wrote:
It just controls the To URI.

server_uri = Target URI (including user for request URI and the location
of where to send it)
to_uri = URI in To header
from_uri = URI in From header

Okay, now I think I understand. I think you were focusing more on username, and
I was focusing more on location in this conversation. So if Alice is a server
and is located at foo.com, and Bob is a second server and is located at
bar.com, then when Alice sends a publication to Bob, the server URI would be
something like:

server_uri = al...@bar.com

Right? That way, the message gets routed to Bob's server, but the resource
represented in the user part is alice.

- Mark

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

On July 21, 2014, 12:14 p.m., Joshua Colp wrote:

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

(Updated July 21, 2014, 12:14 p.m.)

Review request for Asterisk Developers.

Repository: Asterisk

Description
---

This adds two PJSIP modules which add outbound PUBLISH support and an
'asterisk' event type.

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

Re: [asterisk-dev] [Code Review] 3760: ARI: Fix endpoint/channel subscription issues; allow for subscriptions to endpoint technologies


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

Ship it!


Ship It!

- opticron


On July 21, 2014, 9:48 a.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3760/
 ---
 
 (Updated July 21, 2014, 9:48 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23692
 https://issues.asterisk.org/jira/browse/ASTERISK-23692
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This is the preliminary work needed for ASTERISK-23692, which allows for 
 sending/receiving arbitrary out of call text messages through ARI in a 
 technology agnostic fashion.
 
 The messaging functionality described on ASTERISK-23692 requires two things:
 (1) The ability to send/receive messages associated with an endpoint. This is 
 relatively straight forwards with the endpoint core in Asterisk now.
 (2) The ability to send/receive messages associated with a technology and an 
 arbitrary technology defined URI. This is less straight forward, as endpoints 
 are formed from a tech + resource pair. We don't have a mechanism to note 
 that a technology that *may* have endpoints exists.
 
 This patch provides such a mechanism, and fixes a few bugs along the way.
 
 The first major bug this patch fixes is the forwarding of channel messages to 
 their respective endpoints. Prior to this patch, there were two problems:
 (1) Channel caching messages weren't forwarded. Thus, the endpoints missed 
 most of the interesting bits (such as channel creation, destruction, state 
 changes, etc.)
 (2) Channels weren't associated with their endpoint until after creation. 
 This resulted in endpoints missing the channel creation message, which 
 limited the usefulness of the subscription in the first place (a major use 
 case being 'tell me when this endpoint has a channel'). Unfortunately, this 
 meant another parameter to ast_channel_alloc. Since not all channel 
 technologies support an ast_endpoint, this patch makes such a call optional 
 and opts for a new function, ast_channel_alloc_with_endpoint.
 
 When endpoints are created, they will implicitly create a technology endpoint 
 for their technology (if one does not already exist). A technology endpoint 
 is special in that it has no state, cannot have channels created for it, 
 cannot be created explicitly, and cannot be destroyed except on shutdown. It 
 does, however, have all messages from other endpoints in its technology 
 forwarded to it.
 
 Combined with the bug fixes, we now have Stasis messages being properly 
 forwarded. Consider the following scenario: two PJSIP endpoints (foo and 
 bar), where bar has a single channel associated with it and foo has two 
 channels associated with it. The messages would be forwarded as follows:
 
 ast_channel (PJSIP/foo-1) --
 \
  -- ast_endpoint (PJSIP/foo) --
 /   \
 ast_channel (PJSIP/foo-2) -- \
     
 ast_endpoint (PJSIP)
  /
 ast_channel (PJSIP/bar-1) - ast_endpoint (PJSIP/bar) --
 
 ARI, through the applications resource, can:
  - subscribe to endpoint:PJSIP/foo and get notifications for channels 
 PJSIP/foo-1,PJSIP/foo-2 and endpoint PJSIP/foo
  - subscribe to endpoint:PJSIP/bar and get notifications for channels 
 PJSIP/bar-1 and endpoint PJSIP/bar
  - subscribe to endpoint:PJSIP and get notifications for channels 
 PJSIP/foo-1,PJSIP/foo-2,PJSIP/bar-1 and endpoints PJSIP/foo,PJSIP/bar
 
 Note that since endpoint PJSIP never changes, it never has events itself. It 
 merely provides an aggregation point for all other endpoints in its 
 technology (which in turn aggregate all channel messages associated with that 
 endpoint).
 
 This patch also adds endpoints to res_xmpp and chan_motif, because the actual 
 messaging work will need it (messaging without XMPP is just sad)
 
 
 Diffs
 -
 
   /branches/12/rest-api/api-docs/applications.json 419075 
   /branches/12/res/res_xmpp.c 419075 
   /branches/12/res/ari/resource_endpoints.c 419075 
   /branches/12/res/ari/resource_applications.h 419075 
   /branches/12/main/endpoints.c 419075 
   /branches/12/main/channel_internal_api.c 419075 
   /branches/12/main/channel.c 419075 
   /branches/12/include/asterisk/xmpp.h 419075 
   /branches/12/include/asterisk/endpoints.h 419075 
   /branches/12/include/asterisk/channel.h 419075 
   /branches/12/channels/chan_sip.c 419075 
   /branches/12/channels/chan_pjsip.c 419075

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



 On July 17, 2014, 9:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_outbound_publish.c, lines 71-75
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63302#file63302line71
 
  This seems like an odd default behavior. I would suspect that by 
  default we would actually magic up a From URI of our own based instead of 
  telling the server that the message is coming from itself.
  
  Doing this when no to_uri is provided makes good sense though.
 
 Joshua Colp wrote:
 It's a PJSIPism, and it actually makes sense (imo). In the case of 
 PUBLISH you are publishing information about yourself usually. It makes sense 
 that it should be coming from you.
 
 Mark Michelson wrote:
 Hm, now I'm a bit confused about terminology being used. Who is the 
 server in the server_uri? The XML docs say SIP URI of the server and 
 entity to publish to . So in other words, it's the URI of the destination of 
 the PUBLISH request. So let's say the Asterisk server that sends the PUBLISH 
 is Alice, and the Asterisk server that receives the PUBLISH is Bob. You set 
 server_uri=b...@example.com and do not set a from_uri or to_uri. Our SIP 
 request would look something like:
 
 PUBLISH sip:b...@example.com
 To: sip:b...@example.com
 From: sip:b...@example.com;tag=blahblahblah
 
 That's what's intended? Alice is publishing information about Alice, not 
 Bob. If the From header is supposed to indicate the source of the 
 publication, I'd expect that the From would give some indication that the 
 publication is from Alice.
 
 Joshua Colp wrote:
 The name of the entity you are publishing to is generally yourself. I am 
 Alice and I am publishing the state of Alice. This is speaking from a SIP 
 perspective. You'd use server_uri=sip:al...@example.com. This only differs if 
 you are publishing to an entity which is different than your own. I am Alice 
 but I'm publishing the state of Bob.
 
 server_uri=sip:b...@example.com
 from_uri=sip:al...@example.com
 
 Mark Michelson wrote:
 Ah, so the server URI is in essence the URI of the resource whose state 
 is being published. It's not the URI of the UAS that is to receive the 
 PUBLISH request. If that's the case, then is the to_uri used to populate the 
 RURI of the PUBLISH request?
 
 Joshua Colp wrote:
 It just controls the To URI.
 
 server_uri = Target URI (including user for request URI and the location 
 of where to send it)
 to_uri = URI in To header
 from_uri = URI in From header
 
 Mark Michelson wrote:
 Okay, now I think I understand. I think you were focusing more on 
 username, and I was focusing more on location in this conversation. So if 
 Alice is a server and is located at foo.com, and Bob is a second server and 
 is located at bar.com, then when Alice sends a publication to Bob, the server 
 URI would be something like:
 
 server_uri = al...@bar.com
 
 Right? That way, the message gets routed to Bob's server, but the 
 resource represented in the user part is alice.

CORRECT!


- Joshua


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


On July 21, 2014, 12:14 p.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3780/
 ---
 
 (Updated July 21, 2014, 12:14 p.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

Re: [asterisk-dev] [Code Review] 3829: Voicemail send email to multiple email addresses


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


Two points:
1) This was technically ready for review prior to feature freeze, and hence is 
a candidate for Asterisk 13.
2) The CHANGES file needs to be updated noting the new feature in app_voicemail.


/branches/12/apps/app_voicemail.c
https://reviewboard.asterisk.org/r/3829/#comment23097

tmp is never a good variable name. I'd pick something more descriptive.



/branches/12/apps/app_voicemail.c
https://reviewboard.asterisk.org/r/3829/#comment23098

This needs to be structured differently.

It should be:

for each space separate e-mail to send to {
   if (check_mime(vmu-fullname)) {
   ...
   } else {
  fprintf(p, To: ... );
   }
}

Right now, if we have a space separated list of e-mail addresses, we won't 
check if the full name needs mime encoding. This will incorrectly construct the 
To headers in such a situation.


- Matt Jordan


On July 18, 2014, 1:35 p.m., Jacob Barber wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3829/
 ---
 
 (Updated July 18, 2014, 1:35 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24045
 https://issues.asterisk.org/jira/browse/ASTERISK-24045
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Currently voicemail to email only works with a single email. This patch 
 allows a user to use a space separated list of emails (up to 512 characters 
 long), where the user would like for the emails to be sent. This is useful 
 for people who don't want to go through setting up mailing groups, or for 
 people who host provide VoIP services with asterisk as a backend, where their 
 customers don't know how to set up mailing groups.
 
 
 Diffs
 -
 
   /branches/12/apps/app_voicemail.c 418713 
 
 Diff: https://reviewboard.asterisk.org/r/3829/diff/
 
 
 Testing
 ---
 
 Tested calling and sending voicemails using the mysql realtime database and 
 using the standard voicemail.conf implementation.
 
 
 Thanks,
 
 Jacob Barber
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow


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



trunk/main/stasis.c
https://reviewboard.asterisk.org/r/3823/#comment23101

Yay, I get to be that guy again :-(

I should have noted this in the last review, but technically, the message 
types should be an enum for this configOption.

As an example on structuring this, take a look at the DTMF menu options in 
confbridge.


- Matt Jordan


On July 18, 2014, 12:22 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3823/
 ---
 
 (Updated July 18, 2014, 12:22 p.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This introduces stasis.conf and a mechanism to prevent certain message types 
 from being published.
 
 
 Diffs
 -
 
   trunk/tests/test_stasis_channels.c 418910 
   trunk/tests/test_stasis.c 418910 
   trunk/main/stasis_message.c 418910 
   trunk/main/stasis_cache.c 418910 
   trunk/main/stasis.c 418910 
   trunk/include/asterisk/stasis.h 418910 
   trunk/configs/samples/stasis.conf.sample PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/3823/diff/
 
 
 Testing
 ---
 
 Tested by causing the stasis unittests to fail with the following stasis.conf 
 configuration:
 [declined_message_types]
 decline=TestMessage
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3777: module loader: Unload modules in reverse order of their start order



 On July 16, 2014, 7:52 p.m., Corey Farrell wrote:
  /trunk/main/loader.c, lines 606-607
  https://reviewboard.asterisk.org/r/3777/diff/1/?file=63328#file63328line606
 
  Not sure we want this, but if we do I think it should be ast_debug.

It will be useful when debugging ref leaks in the future. I do agree that it is 
more of a 'debug' appropriate message; demoted.


- Matt


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


On July 14, 2014, 3:16 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3777/
 ---
 
 (Updated July 14, 2014, 3:16 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When Asterisk starts a module (calling its load_module function), it 
 re-orders the module list, sorting it alphabetically. Ostensibly, this was 
 done so that the output of 'module show' listed modules in alphabetic order. 
 This had the unfortunate side effect of making modules with complex usage 
 patterns practically unloadable. A module that has a large number of modules 
 that depend on it is typically abandoned during the unloading process. This 
 results in its memory not being reclaimed during exit.
 
 Generally, this isn't harmful - when the process is destroyed, the operating 
 system will reclaim all memory allocated by the process. However, it makes 
 tracking memory leaks or ref debug leaks a real pain.
 
 While this patch is not a complete overhaul of the module loader - such an 
 effort would be beyond the scope of what could be done for Asterisk 13 - this 
 does make some marginal improvements to the loader such that modules like 
 res_pjsip or res_stasis *may* be made properly un-loadable in the future.
 1. The linked list of modules has been replaced with a doubly linked list. 
 This allows traversal of the module list to occur backwards. The module 
 shutdown routine now walks the global list backwards when it attempts to 
 unload modules.
 2. The alphabetic reorganization of the module list on startup has been 
 removed. Instead, a started module is placed at the end of the module list.
 3. The ast_update_module_list function - which is used by the CLI to display 
 the modules - now does the sorting alphabetically itself. It creates its own 
 linked list and inserts the modules into it in alphabetic order. This allows 
 for the intent of the previous code to be maintained.
 
 This patch also contains a fix for res_calendar. Without calendar.conf, the 
 calendar modules were improperly bumping the use count of res_calendar, then 
 failing to load themselves. This patch makes it so that we detect whether or 
 not calendaring is enabled before altering the use count.
 
 
 Diffs
 -
 
   /trunk/res/res_calendar.c 418436 
   /trunk/main/loader.c 418436 
 
 Diff: https://reviewboard.asterisk.org/r/3777/diff/
 
 
 Testing
 ---
 
 Verified that the CLI command worked appropriately.
 
 Verified that module loading/unloading of res_calendar (whose calendar 
 modules modify the res_calendar use count) loaded/unloaded properly:
 
  Asterisk Dynamic Loader Starting:
 [Jul 14 15:14:52] NOTICE[11877]: loader.c:1317 load_modules: 6 modules will 
 be loaded.
  Loading res_calendar.so.
  Loading res_calendar_icalendar.so.
  Loading res_calendar_exchange.so.
  Loading res_calendar_caldav.so.
  Loading res_calendar_ews.so.
 Asterisk Ready.
 *CLI core stop gracefully
 Waiting for inactivity to perform halt...
  Unloading res_calendar_ews.so
  Unloading res_calendar_caldav.so
  Unloading res_calendar_exchange.so
  Unloading res_calendar_icalendar.so
  Unloading res_calendar.so
 Asterisk cleanly ending (0).
 Executing last minute cleanups
 
 Verified that using autoload with all modules, module are started as they 
 were previously, and now are stopped/unloaded in the reverse order.
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3777: module loader: Unload modules in reverse order of their start order


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

(Updated July 21, 2014, 11:58 a.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Corey's finding.


Repository: Asterisk


Description
---

When Asterisk starts a module (calling its load_module function), it re-orders 
the module list, sorting it alphabetically. Ostensibly, this was done so that 
the output of 'module show' listed modules in alphabetic order. This had the 
unfortunate side effect of making modules with complex usage patterns 
practically unloadable. A module that has a large number of modules that depend 
on it is typically abandoned during the unloading process. This results in its 
memory not being reclaimed during exit.

Generally, this isn't harmful - when the process is destroyed, the operating 
system will reclaim all memory allocated by the process. However, it makes 
tracking memory leaks or ref debug leaks a real pain.

While this patch is not a complete overhaul of the module loader - such an 
effort would be beyond the scope of what could be done for Asterisk 13 - this 
does make some marginal improvements to the loader such that modules like 
res_pjsip or res_stasis *may* be made properly un-loadable in the future.
1. The linked list of modules has been replaced with a doubly linked list. This 
allows traversal of the module list to occur backwards. The module shutdown 
routine now walks the global list backwards when it attempts to unload modules.
2. The alphabetic reorganization of the module list on startup has been 
removed. Instead, a started module is placed at the end of the module list.
3. The ast_update_module_list function - which is used by the CLI to display 
the modules - now does the sorting alphabetically itself. It creates its own 
linked list and inserts the modules into it in alphabetic order. This allows 
for the intent of the previous code to be maintained.

This patch also contains a fix for res_calendar. Without calendar.conf, the 
calendar modules were improperly bumping the use count of res_calendar, then 
failing to load themselves. This patch makes it so that we detect whether or 
not calendaring is enabled before altering the use count.


Diffs (updated)
-

  /trunk/res/res_calendar.c 419109 
  /trunk/main/loader.c 419109 

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


Testing
---

Verified that the CLI command worked appropriately.

Verified that module loading/unloading of res_calendar (whose calendar modules 
modify the res_calendar use count) loaded/unloaded properly:

 Asterisk Dynamic Loader Starting:
[Jul 14 15:14:52] NOTICE[11877]: loader.c:1317 load_modules: 6 modules will be 
loaded.
 Loading res_calendar.so.
 Loading res_calendar_icalendar.so.
 Loading res_calendar_exchange.so.
 Loading res_calendar_caldav.so.
 Loading res_calendar_ews.so.
Asterisk Ready.
*CLI core stop gracefully
Waiting for inactivity to perform halt...
 Unloading res_calendar_ews.so
 Unloading res_calendar_caldav.so
 Unloading res_calendar_exchange.so
 Unloading res_calendar_icalendar.so
 Unloading res_calendar.so
Asterisk cleanly ending (0).
Executing last minute cleanups

Verified that using autoload with all modules, module are started as they were 
previously, and now are stopped/unloaded in the reverse order.


Thanks,

Matt Jordan

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3817: res_pjsip_notify: Add the ability for PJSIPNotify AMI command and pjsip send notify CLI command to send to a URI instead of an endpoint

2014-07-21 Thread Jonathan Rose



 On July 18, 2014, 10:40 a.m., Mark Michelson wrote:
  /trunk/include/asterisk/res_pjsip.h, lines 1195-1203
  https://reviewboard.asterisk.org/r/3817/diff/2/?file=64713#file64713line1195
 
  This function isn't necessary. When PJSIP is passed a URI string, PJSIP 
  will perform URI validation for us and return an error if a badly-formed 
  URI is passed in.
 
 Jonathan Rose wrote:
 I was absolutely getting crashes when using URIs that didn't start with 
 'sip:' or else were only 'sip:' without anything appended to them.  I'm not 
 sure if that will keep being the case if we use the default endpoint like 
 suggested below, but if it does then I might need to keep this around.
 
 Jonathan Rose wrote:
 After testing it with the default outbound endpoint, it was still able to 
 cause crashes. Rats.
 
 Mark Michelson wrote:
 Where does the crash occur? Is it in Asterisk's lower-level res_pjsip 
 code, or is it in PJSIP itself? If the crash is in Asterisk, it sounds like 
 there are some assumptions being made in the lower levels that shouldn't be 
 made and those should be fixed there. If the crash is in PJSIP (especially if 
 it's an assertion failure), then that means we do have to do some validation 
 before calling down into PJSIP, but that validation should be left to the 
 core res_pjsip code rather than being the duty of users of the res_pjsip API.
 
 If we do need to perform validation of the URI, the implementation has 
 some issues. It's limiting since it only allows for sip: URIs and not, for 
 instance, sips: URIs. It also only checks that the prefix is valid for a sip: 
 URI. It does no other checking of the rest of the URI. A more thorough way of 
 checking validity of the URI is to use pjsip_parse_uri() from PJSIP. If you 
 then want to make sure that the URI is a sip: or sips: URI, you can use the 
 PJSIP_URI_SCHEME_IS_SIP() and PJSIP_URI_SCHEME_IS_SIPS() macros to do so.

Oi, it turned out that adding the outbound endpoint did fix it, I just forgot 
to add the outbound endpoint to the call that actually created the request... I 
only added it the function that sent it.

Update coming shortly.


- Jonathan


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


On July 18, 2014, 1:32 p.m., Jonathan Rose wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3817/
 ---
 
 (Updated July 18, 2014, 1:32 p.m.)
 
 
 Review request for Asterisk Developers, Matt Jordan and Mark Michelson.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Example:
 
 Action: PJSIPNotify
 URI: sip:10.10.10.10
 
 pjsip send notify uri type uri uri [uri...]
 
 
 Adds the ability to use URIs with the PJSIP notify commands instead of 
 endpoints. These mostly work the same as the endpoint notifications.
 
 
 Diffs
 -
 
   /trunk/res/res_pjsip_notify.c 418868 
   /trunk/res/res_pjsip.c 418868 
   /trunk/include/asterisk/res_pjsip.h 418868 
 
 Diff: https://reviewboard.asterisk.org/r/3817/diff/
 
 
 Testing
 ---
 
 Used existing endpoint notification and compared it to similar notifies done 
 against URIs.  The outgoing messages looked the same as far as the details I 
 was concerned with went.
 
 
 Thanks,
 
 Jonathan Rose
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2


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



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23106

Heh, it's a bit late for this now, but it would have probably saved you a 
bunch of searching and replacing to just change the definition of 
registry_unref() and registry_addref() to use ao2 in their implementations.



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23100

I agree with this comment. Since mwi-call was created via sip_alloc(), 
destruction of it should be accomplished with dialog_unref()



/trunk/channels/chan_sip.c
https://reviewboard.asterisk.org/r/3759/#comment23103

You sometimes search for sip_registrys using OBJ_KEY, but your hash and cmp 
callbacks do not account for a search key being passed to the callback instead 
of a sip_registry.

Use the templates on this wiki page 
https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=25919686 to define 
your hash and comparison callbacks and you should be good to go.


- Mark Michelson


On July 21, 2014, 8:54 a.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3759/
 ---
 
 (Updated July 21, 2014, 8:54 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24067
 https://issues.asterisk.org/jira/browse/ASTERISK-24067
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Upgrade all ASTOBJ objects in chan_sip to ao2.
 
 
 Diffs
 -
 
   /trunk/channels/sip/include/sip.h 419043 
   /trunk/channels/chan_sip.c 419043 
 
 Diff: https://reviewboard.asterisk.org/r/3759/diff/
 
 
 Testing
 ---
 
 Full testsuite run.
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3799: manager: Add ExtensionStateList, PresenceStateList, and DeviceStateList commands



 On July 18, 2014, 5:18 p.m., opticron wrote:
 

I gave this review a look and only found the same findings as opticron.


- Mark


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


On July 15, 2014, 8:55 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3799/
 ---
 
 (Updated July 15, 2014, 8:55 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch adds three new AMI commands:
  * ExtensionStateList (pbx.c) - list all known extension state hints and 
 their current statuses. Events emitted by the list action are equivalent to 
 the ExtensionStatus events.
  * PresenceStateList (res_manager_presencestate) - list all known presence 
 state values. Events emitted are generated by the stasis message type, and 
 hence are PresenceStateChange events.
  * DeviceStateList (res_manager_devicestate) - list all known device state 
 values. Events emitted are generated by the stasis message type, and hence 
 are DeviceStateChange events.
 
 
 Diffs
 -
 
   /trunk/res/res_manager_presencestate.c 418612 
   /trunk/res/res_manager_devicestate.c 418612 
   /trunk/main/pbx.c 418612 
   /trunk/main/manager.c 418612 
 
 Diff: https://reviewboard.asterisk.org/r/3799/diff/
 
 
 Testing
 ---
 
 Currently, only manual verification:
 
  * Made two hints, one with presence.
  * Ran all three commands and checked the output
  * Used a Custom device state and a CustomPresence provider and changed their 
 statuses using a Local channel and the DEVICE_STATE/PRESENCE_STATE functions
  * Ran all three commands again and got back the expected updated values
 
 Note that before this is committed, it must have tests covering the new AMI 
 actions. This review will be updated when that test review is put up.
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow


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

(Updated July 21, 2014, 12:37 p.m.)


Review request for Asterisk Developers and Corey Farrell.


Changes
---

Address Matt's comments about configuration documentation.


Repository: Asterisk


Description
---

This introduces stasis.conf and a mechanism to prevent certain message types 
from being published.


Diffs (updated)
-

  trunk/tests/test_stasis_channels.c 419109 
  trunk/tests/test_stasis.c 419109 
  trunk/main/stasis_message.c 419109 
  trunk/main/stasis_cache.c 419109 
  trunk/main/stasis.c 419109 
  trunk/include/asterisk/stasis.h 419109 
  trunk/configs/samples/stasis.conf.sample PRE-CREATION 

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


Testing
---

Tested by causing the stasis unittests to fail with the following stasis.conf 
configuration:
[declined_message_types]
decline=TestMessage


Thanks,

opticron

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3777: module loader: Unload modules in reverse order of their start order


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



/trunk/main/loader.c
https://reviewboard.asterisk.org/r/3777/#comment23107

Feel free to cringe at this suggestion, but since you've created both an 
AST_DLLIST_ENTRY and an AST_LIST_ENTRY on the ast_module structure, you could 
maintain two parallel lists of modules. You'd have the module_list, that is 
important for module loading and unloading, and you could have the alphabetical 
list that is used by the CLI command.

At the time that you add the module to the module_list (what I've 
highlighted), you can also add it to the alphabetical list. That way, you would 
not have to lock the module_list and rebuild the alphabetical list each time 
ast_update_module_list() is called. Up to you really.


- Mark Michelson


On July 21, 2014, 4:58 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3777/
 ---
 
 (Updated July 21, 2014, 4:58 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When Asterisk starts a module (calling its load_module function), it 
 re-orders the module list, sorting it alphabetically. Ostensibly, this was 
 done so that the output of 'module show' listed modules in alphabetic order. 
 This had the unfortunate side effect of making modules with complex usage 
 patterns practically unloadable. A module that has a large number of modules 
 that depend on it is typically abandoned during the unloading process. This 
 results in its memory not being reclaimed during exit.
 
 Generally, this isn't harmful - when the process is destroyed, the operating 
 system will reclaim all memory allocated by the process. However, it makes 
 tracking memory leaks or ref debug leaks a real pain.
 
 While this patch is not a complete overhaul of the module loader - such an 
 effort would be beyond the scope of what could be done for Asterisk 13 - this 
 does make some marginal improvements to the loader such that modules like 
 res_pjsip or res_stasis *may* be made properly un-loadable in the future.
 1. The linked list of modules has been replaced with a doubly linked list. 
 This allows traversal of the module list to occur backwards. The module 
 shutdown routine now walks the global list backwards when it attempts to 
 unload modules.
 2. The alphabetic reorganization of the module list on startup has been 
 removed. Instead, a started module is placed at the end of the module list.
 3. The ast_update_module_list function - which is used by the CLI to display 
 the modules - now does the sorting alphabetically itself. It creates its own 
 linked list and inserts the modules into it in alphabetic order. This allows 
 for the intent of the previous code to be maintained.
 
 This patch also contains a fix for res_calendar. Without calendar.conf, the 
 calendar modules were improperly bumping the use count of res_calendar, then 
 failing to load themselves. This patch makes it so that we detect whether or 
 not calendaring is enabled before altering the use count.
 
 
 Diffs
 -
 
   /trunk/res/res_calendar.c 419109 
   /trunk/main/loader.c 419109 
 
 Diff: https://reviewboard.asterisk.org/r/3777/diff/
 
 
 Testing
 ---
 
 Verified that the CLI command worked appropriately.
 
 Verified that module loading/unloading of res_calendar (whose calendar 
 modules modify the res_calendar use count) loaded/unloaded properly:
 
  Asterisk Dynamic Loader Starting:
 [Jul 14 15:14:52] NOTICE[11877]: loader.c:1317 load_modules: 6 modules will 
 be loaded.
  Loading res_calendar.so.
  Loading res_calendar_icalendar.so.
  Loading res_calendar_exchange.so.
  Loading res_calendar_caldav.so.
  Loading res_calendar_ews.so.
 Asterisk Ready.
 *CLI core stop gracefully
 Waiting for inactivity to perform halt...
  Unloading res_calendar_ews.so
  Unloading res_calendar_caldav.so
  Unloading res_calendar_exchange.so
  Unloading res_calendar_icalendar.so
  Unloading res_calendar.so
 Asterisk cleanly ending (0).
 Executing last minute cleanups
 
 Verified that using autoload with all modules, module are started as they 
 were previously, and now are stopped/unloaded in the reverse order.
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3810: res_hep_rtcp: Add module that sends RTCP information to a Homer Server


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



/branches/12/res/res_rtp_asterisk.c
https://reviewboard.asterisk.org/r/3810/#comment23108

ast_sockaddr_stringify uses a thread-local buffer to store the stringified 
address. If you call it twice like this, the second call is going to overwrite 
the result of the first. The result is that both strings in this json object 
are likely to be the same.

Typically, the way to work around this is to use ast_strdupa() on the 
result of ast_sockaddr_stringify() so that the local frame has a copy of the 
result of stringification.



/branches/12/res/res_rtp_asterisk.c
https://reviewboard.asterisk.org/r/3810/#comment23109

Same warning for ast_sockaddr_stringify() as before.


- Mark Michelson


On July 16, 2014, 10:37 p.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3810/
 ---
 
 (Updated July 16, 2014, 10:37 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch adds a new module to Asterisk, res_hep_rtcp. The module subscribes 
 to Stasis and receives RTCP information back from the message bus, which it 
 encodes into HEPv3 packets and sends to the res_hep module for transmission.
 
 Using this, someone with a Homer server can get live call quality monitoring 
 for all channels in their Asterisk 12+ systems.
 
 
 Diffs
 -
 
   /branches/12/res/res_rtp_asterisk.c 418783 
   /branches/12/res/res_hep_rtcp.c PRE-CREATION 
   /branches/12/CHANGES 418783 
 
 Diff: https://reviewboard.asterisk.org/r/3810/diff/
 
 
 Testing
 ---
 
 Some manual testing has be done, and automated tests have started being 
 written in the Asterisk Test Suite. Note that they need to be completed 
 before this is submitted.
 
 As a side note, Alexander actually demo'd this at Kamailio World - you can 
 see it on the 'dangerous demos' here - 
 http://www.youtube.com/watch?v=ykBdOTCCSHs
 
 
 Thanks,
 
 Matt Jordan
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3781: Retrieve the source port of an incoming (chan_sip) SIP invite


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

Ship it!



/trunk/channels/sip/dialplan_functions.c
https://reviewboard.asterisk.org/r/3781/#comment23110

There's an inline function in include/asterisk/netsock2.h called 
ast_sockaddr_stringify_port() that you can use in place of the 
ast_sockaddr_stringify_fmt() call.


- Mark Michelson


On July 21, 2014, 1:15 p.m., dtryba wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3781/
 ---
 
 (Updated July 21, 2014, 1:15 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24040
 https://issues.asterisk.org/jira/browse/ASTERISK-24040
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Retrieve the source port of an incoming (chan_sip) SIP invite in the dialplan 
 with ${CHANNEL(recvport)}
 To complement ${CHANNEL(recvip)} and enable me to make dialplan decisions 
 based on source port (in a peerless setup, handle everything as guests using 
 AGI to lookup source ip/port for routing/handling).
 
 pjsip appears to have this capability through the CHANNEL function 
 (pjsip,local_addr/remote_addr).
 
 Simple 2 line patch using ast_sockaddr_stringify_fmt(const struct 
 ast_sockaddr *sa, int format)
 to return the port as a string.
 
 
 Diffs
 -
 
   /trunk/channels/sip/dialplan_functions.c 418610 
 
 Diff: https://reviewboard.asterisk.org/r/3781/diff/
 
 
 Testing
 ---
 
 Tested on 11.10.2 (Debian Jessie) and trunk (418610) using IPv4. Having a few 
 SIP endpoints connect from different address/ports combinations 
 Logged ${CHANNEL(recvip)}:${CHANNEL(recvport)} corresponds with source 
 ip:port in packetdumps on the asterisk machine.
 
 
 Thanks,
 
 dtryba
 


-- 
_
-- 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

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



 On July 17, 2014, 4:49 p.m., Mark Michelson wrote:
  /trunk/res/res_pjsip_publish_asterisk.c, lines 428-430
  https://reviewboard.asterisk.org/r/3780/diff/1/?file=63304#file63304line428
 
  Is there some sort of stasis cache removal you could perform here?
 
 Joshua Colp wrote:
 So the best I could do is:
 
 1. For device state do a full cache dump, run the regex against, and set 
 everything it matches to unknown.
 2. For mailbox state do a full cache dump, run the regex against, and set 
 every mailbox to 0/0
 
 I would also need to extend things to store a bit of state information 
 about the publisher we are receiving updates from (specifically the eid).
 
 Thoughts?
 
 Mark Michelson wrote:
 I wasn't thinking of simply marking devstates to unknown and mailboxes to 
 0/0. I was thinking of actually removing the items from the cache entirely.
 
 Does a single publication convey the state of multiple resources? In 
 other words, is the same publication used to transmit the state of 
 PJSIP/Alice at eid 12345 and PJSIP/bob at eid 67890? If so, then I don't 
 necessarily see how you could actually remove cache items on a publication 
 expiration, since you could presumably still be getting those states 
 published from a separate publisher entirely. However, if a publisher is 
 guaranteed to only publish a single resource or just resources that are local 
 to its eid, then upon publication expiration, you could remove the specific 
 resource from the cache or remove all resources from that particular eid.
 
 Joshua Colp wrote:
 A single publication currently conveys only a single resource (device or 
 mailbox). A PUBLISH will only be sent for updates from the originating 
 system, it does not act as a relay and thus has one eid. As it is I don't 
 think I can safely manipulate the cache to produce the desired result. I can 
 only change them to some unknown values.
 
 Matt - Thoughts?

This is a tricky problem.

Internally, the states of all devices/MWI are already stored according to the 
system's eid:

struct stasis_cache_entry {
struct cache_entry_key key;
/*! Aggregate snapshot of the stasis cache. */
struct stasis_message *aggregate;
/*! Local entity snapshot of the stasis event. */
struct stasis_message *local;
/*! Remote entity snapshots of the stasis event. */
AST_VECTOR(, struct stasis_message *) remote;
};

A local update (where eid == this system's eid) is stored in local; remote 
updates are stored in the remote vector. The aggregate is computed if the thing 
being stored has an aggregate function (which only device state has).

stasis_cache_dump_by_eid/stasis_cache_dump_all gives you a mechanism to get a 
particular remote system's updates and/or all updates from all systems (which, 
as you might imagine, is not the most performant thing to do in the world). We 
could conceivably use similar mechanisms to remove a particular system's update 
from the cache, or set all entries from a system to a particular value, etc.

The problem is, I'm not sure we should be doing that unless a remote system 
explicitly tells us to wipe themselves from our cache.

If a PUBLISH request times out, you may want to keep the device state from that 
system in the cache. If the system is truly down, endpoints from that system 
may be falling back to you, and you may want to keep the aggregate state 
computed from what they were on that system. Likewise, we may simply have run 
into network lag (or a dropped packet) and the remote system may be fine - in 
which case, purging the state may cause some rather odd behaviour.

This feels like something we solve once we know it is causing a problem.


- Matt


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


On July 21, 2014, 7:14 a.m., Joshua Colp wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3780/
 ---
 
 (Updated July 21, 2014, 7:14 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

Re: [asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow


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



trunk/main/stasis_message.c
https://reviewboard.asterisk.org/r/3823/#comment23112

Can you describe the reason this function was made NULL-safe? Did you 
encounter a situation in testing where a NULL stasis_message_type was passed to 
this function?

Making this function NULL-safe and returning NULL may have a ripple effect 
that is not addressed by this changeset. Callers of this function assume that 
this function will never return NULL.

I suspect that code paths that would call this function with a NULL 
stasis_message_type are not ever going to execute since the message type was 
declined from being created in the first place.

My thought process on this is, if it's acceptable to have a NULL 
stasis_message_type passed to this function, then callers of this function need 
to be prepared to deal with a NULL return. On the other hand, if it's never 
expected for a code path to ever pass a NULL stasis_message_type to this 
function, then the NULL check in this function should be changed to an 
assertion instead.


- Mark Michelson


On July 21, 2014, 5:37 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3823/
 ---
 
 (Updated July 21, 2014, 5:37 p.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This introduces stasis.conf and a mechanism to prevent certain message types 
 from being published.
 
 
 Diffs
 -
 
   trunk/tests/test_stasis_channels.c 419109 
   trunk/tests/test_stasis.c 419109 
   trunk/main/stasis_message.c 419109 
   trunk/main/stasis_cache.c 419109 
   trunk/main/stasis.c 419109 
   trunk/include/asterisk/stasis.h 419109 
   trunk/configs/samples/stasis.conf.sample PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/3823/diff/
 
 
 Testing
 ---
 
 Tested by causing the stasis unittests to fail with the following stasis.conf 
 configuration:
 [declined_message_types]
 decline=TestMessage
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow



 On July 21, 2014, 2:25 p.m., Mark Michelson wrote:
  trunk/main/stasis_message.c, lines 84-91
  https://reviewboard.asterisk.org/r/3823/diff/7/?file=64893#file64893line84
 
  Can you describe the reason this function was made NULL-safe? Did you 
  encounter a situation in testing where a NULL stasis_message_type was 
  passed to this function?
  
  Making this function NULL-safe and returning NULL may have a ripple 
  effect that is not addressed by this changeset. Callers of this function 
  assume that this function will never return NULL.
  
  I suspect that code paths that would call this function with a NULL 
  stasis_message_type are not ever going to execute since the message type 
  was declined from being created in the first place.
  
  My thought process on this is, if it's acceptable to have a NULL 
  stasis_message_type passed to this function, then callers of this function 
  need to be prepared to deal with a NULL return. On the other hand, if it's 
  never expected for a code path to ever pass a NULL stasis_message_type to 
  this function, then the NULL check in this function should be changed to an 
  assertion instead.

The call and subsequent crash occurred in the unittests when I was testing the 
feature on them. This function is not widely used and is generally only used 
where the type is being pulled from an existing message (which means it is 
non-NULL).  I'll drop the NULL-safety here and update the unit test.


- opticron


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


On July 21, 2014, 12:37 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3823/
 ---
 
 (Updated July 21, 2014, 12:37 p.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This introduces stasis.conf and a mechanism to prevent certain message types 
 from being published.
 
 
 Diffs
 -
 
   trunk/tests/test_stasis_channels.c 419109 
   trunk/tests/test_stasis.c 419109 
   trunk/main/stasis_message.c 419109 
   trunk/main/stasis_cache.c 419109 
   trunk/main/stasis.c 419109 
   trunk/include/asterisk/stasis.h 419109 
   trunk/configs/samples/stasis.conf.sample PRE-CREATION 
 
 Diff: https://reviewboard.asterisk.org/r/3823/diff/
 
 
 Testing
 ---
 
 Tested by causing the stasis unittests to fail with the following stasis.conf 
 configuration:
 [declined_message_types]
 decline=TestMessage
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow


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

(Updated July 21, 2014, 2:58 p.m.)


Review request for Asterisk Developers and Corey Farrell.


Changes
---

Prevent expensive payload creation and address Mark's comments.


Repository: Asterisk


Description
---

This introduces stasis.conf and a mechanism to prevent certain message types 
from being published.


Diffs (updated)
-

  trunk/tests/test_stasis_channels.c 419109 
  trunk/tests/test_stasis.c 419109 
  trunk/main/stasis_message.c 419109 
  trunk/main/stasis_channels.c 419109 
  trunk/main/stasis_cache.c 419109 
  trunk/main/stasis_bridges.c 419109 
  trunk/main/stasis.c 419109 
  trunk/include/asterisk/stasis.h 419109 
  trunk/configs/samples/stasis.conf.sample PRE-CREATION 

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


Testing
---

Tested by causing the stasis unittests to fail with the following stasis.conf 
configuration:
[declined_message_types]
decline=TestMessage


Thanks,

opticron

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3729: Stasis: Allow prestart actions to be queued


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



branches/12/res/stasis/control.c
https://reviewboard.asterisk.org/r/3729/#comment23114

There's no real need for count when you have ao2_container_count() 
available to you. You can cal it prior to iterating over the items since you're 
removing the commands from the queue as they are executed.


- Mark Michelson


On July 18, 2014, 5:48 p.m., opticron wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3729/
 ---
 
 (Updated July 18, 2014, 5:48 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23941
 https://issues.asterisk.org/jira/browse/ASTERISK-23941
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This allows commands to be queued in a channel datastore to be executed upon 
 a channel entering Stasis(). This functionality is only available from 
 components of res_stasis.so. This functionality is required to get a 
 redirected channel back into the bridge for which it was destined due to the 
 attended transfer.
 
 
 Diffs
 -
 
   branches/12/res/stasis/control.c 418910 
   branches/12/res/stasis/control.h 418910 
   branches/12/res/stasis/command.c 418910 
   branches/12/res/stasis/command.h 418910 
   branches/12/res/res_stasis.c 418910 
 
 Diff: https://reviewboard.asterisk.org/r/3729/diff/
 
 
 Testing
 ---
 
 Tested as part of the complete solution to ASTERISK-23941.
 
 
 Thanks,
 
 opticron
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3777: module loader: Unload modules in reverse order of their start order



 On July 21, 2014, 12:49 p.m., Mark Michelson wrote:
  /trunk/main/loader.c, lines 1001-1005
  https://reviewboard.asterisk.org/r/3777/diff/2/?file=64886#file64886line1001
 
  Feel free to cringe at this suggestion, but since you've created both 
  an AST_DLLIST_ENTRY and an AST_LIST_ENTRY on the ast_module structure, you 
  could maintain two parallel lists of modules. You'd have the module_list, 
  that is important for module loading and unloading, and you could have the 
  alphabetical list that is used by the CLI command.
  
  At the time that you add the module to the module_list (what I've 
  highlighted), you can also add it to the alphabetical list. That way, you 
  would not have to lock the module_list and rebuild the alphabetical list 
  each time ast_update_module_list() is called. Up to you really.

/me cringes

So, when I first started at this work, I approached it from the angle of 
ordering the module_list during start up. I had a vector of modules lists, 
where the vector had slots based on the various possible module priorities. 
That was a mistake.

Let me tell you: module loading is some strange joo-joo. Things get opened, 
loaded, and discarded more times than you would believe. By the time 
*something* gets around calling load_module, that particular 'module' has 
probably been in and out of memory (and possibly the modules list) a number of 
times. And don't even get me started on embedded modules. That's a whole other 
ball of string to untangle.

I fully admit that this is the poor man's fix for the module loader - ideally 
we'd have an actual dependency tree, and other niceties. I'm hesitant to make 
too many changes in here - and having two module lists feels... dangerous.


- Matt


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


On July 21, 2014, 11:58 a.m., Matt Jordan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3777/
 ---
 
 (Updated July 21, 2014, 11:58 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 When Asterisk starts a module (calling its load_module function), it 
 re-orders the module list, sorting it alphabetically. Ostensibly, this was 
 done so that the output of 'module show' listed modules in alphabetic order. 
 This had the unfortunate side effect of making modules with complex usage 
 patterns practically unloadable. A module that has a large number of modules 
 that depend on it is typically abandoned during the unloading process. This 
 results in its memory not being reclaimed during exit.
 
 Generally, this isn't harmful - when the process is destroyed, the operating 
 system will reclaim all memory allocated by the process. However, it makes 
 tracking memory leaks or ref debug leaks a real pain.
 
 While this patch is not a complete overhaul of the module loader - such an 
 effort would be beyond the scope of what could be done for Asterisk 13 - this 
 does make some marginal improvements to the loader such that modules like 
 res_pjsip or res_stasis *may* be made properly un-loadable in the future.
 1. The linked list of modules has been replaced with a doubly linked list. 
 This allows traversal of the module list to occur backwards. The module 
 shutdown routine now walks the global list backwards when it attempts to 
 unload modules.
 2. The alphabetic reorganization of the module list on startup has been 
 removed. Instead, a started module is placed at the end of the module list.
 3. The ast_update_module_list function - which is used by the CLI to display 
 the modules - now does the sorting alphabetically itself. It creates its own 
 linked list and inserts the modules into it in alphabetic order. This allows 
 for the intent of the previous code to be maintained.
 
 This patch also contains a fix for res_calendar. Without calendar.conf, the 
 calendar modules were improperly bumping the use count of res_calendar, then 
 failing to load themselves. This patch makes it so that we detect whether or 
 not calendaring is enabled before altering the use count.
 
 
 Diffs
 -
 
   /trunk/res/res_calendar.c 419109 
   /trunk/main/loader.c 419109 
 
 Diff: https://reviewboard.asterisk.org/r/3777/diff/
 
 
 Testing
 ---
 
 Verified that the CLI command worked appropriately.
 
 Verified that module loading/unloading of res_calendar (whose calendar 
 modules modify the res_calendar use count) loaded/unloaded properly:
 
  Asterisk Dynamic Loader Starting:
 [Jul 14 15:14:52] NOTICE[11877]: loader.c:1317 load_modules: 6 modules will 
 be loaded.
  Loading res_calendar.so.
  Loading res_calendar_icalendar.so.
  Loading

Re: [asterisk-dev] [Code Review] 3733: Added a pluggable module for pluggable_modules.py that allows the user to test different attributes of a given sound file

2014-07-21 Thread Christopher Wolfe


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

(Updated July 21, 2014, 9:38 p.m.)


Review request for Asterisk Developers.


Changes
---

Fixed various syntatical and naming issues.  Tried to remove indexes but doing 
so was deemed impossible with how this pluggable module is set up.  
Restructured how data is accessed in order to make this pluggable module appear 
more consistent with the others.


Bugs: ASTERISK-24010
https://issues.asterisk.org/jira/browse/ASTERISK-24010


Repository: testsuite


Description
---

Allows the user to test a recorded sound file in many different ways:
1) This test ALWAYS gets done- Checks whether the given sound file exists using 
a predefined path that can be created by either explicitly defining a filepath 
by declaring the filepath type as defined, or using a default path that is 
relative to the current test's var/spool/asterisk folder. From there, the user 
can add extensions to the file name to tack on relative folders 
(monitor/testaudio.wav being an example).
2) Optional- The sound file is checked whether it fits within a certain size 
criteria (measured in bytes).  A basis size and degree of size tolerance are 
determined by the user.  For example, if the size was 50 and the tolerance 
was set to 5, then the sound file's size would need to be somewhere between 
45 and 55 in order to pass that test.
3) Optional- The sound file's sound energy levels are checked.  This is done by 
creating a Local channel that should get sent to a dialplan extension that 
should contain a BackgroundDetect application that fits the user's 
specifications. The variable that will be used to pass the sound file must be 
called SOUNDFILE, and a UserEvent must give off the name soundcheck in order 
for the event to be picked up.  A sample extension:
[soundtest]
exten = audio,1,Answer()
same = n,Set(TALK_DETECTED=0)
same = n,BackgroundDetect(${SOUNDFILE},1,20,,2)
same = n,GoToIf($[${TALK_DETECTED}=0]?pass:fail)
same = n(fail),UserEvent(soundcheck, status: pass)
same = n,Hangup()

same = n(pass),UserEvent(soundcheck, status: fail)
same = n,Hangup()

A sound-file test only gets called when a specified trigger has gone off.  So 
far, this pluggable module only supports events as triggers.  The list of 
triggers matches to each instance of a sound-file test on a one-to-one basis 
(the first trigger starts the first test, and so on).
Only passes after all tests specified by the user have been passed and the 
correct triggers have been received.


Diffs (updated)
-

  /asterisk/trunk/sample-yaml/sound-check-config.yaml.sample PRE-CREATION 
  /asterisk/trunk/lib/python/asterisk/pluggable_modules.py 5249 

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


Testing
---

- Tried using the pluggable module by putting in incorrect input and 
purposefully leaving out input.  Picks those errors up.
- Not sure if I was supposed to allow the user to name their own dialplan 
variable and Userevent name, so I left it as was.
- Tested the different scenarios of setting the filepath- relative and defined.
- Made sure the various tests could fail if a certain sound file didn't meet 
the size criteria or silence threshold criteria.
- Made sure that more than one test could be run and that things could be run 
sequentially.


Thanks,

Christopher Wolfe

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3823: Stasis: Allow configuration of message types to disallow


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

(Updated July 21, 2014, 4:39 p.m.)


Review request for Asterisk Developers and Corey Farrell.


Changes
---

Fix dup'd content in the new sample config.


Repository: Asterisk


Description
---

This introduces stasis.conf and a mechanism to prevent certain message types 
from being published.


Diffs (updated)
-

  trunk/tests/test_stasis_channels.c 419109 
  trunk/tests/test_stasis.c 419109 
  trunk/main/stasis_message.c 419109 
  trunk/main/stasis_channels.c 419109 
  trunk/main/stasis_cache.c 419109 
  trunk/main/stasis_bridges.c 419109 
  trunk/main/stasis.c 419109 
  trunk/include/asterisk/stasis.h 419109 
  trunk/configs/samples/stasis.conf.sample PRE-CREATION 

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


Testing
---

Tested by causing the stasis unittests to fail with the following stasis.conf 
configuration:
[declined_message_types]
decline=TestMessage


Thanks,

opticron

-- 
_
-- 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

[asterisk-dev] DAHDI-Linux and DAHDI-Tools 2.9.2 Now Available

2014-07-21 Thread Asterisk Development Team

The Asterisk Development Team has announced the releases of:
DAHDI-Linux-v2.9.2
DAHDI-Tools-v2.9.2
dahdi-linux-complete-2.9.2+2.9.2

This release is available for immediate download at:
http://downloads.asterisk.org/pub/telephony/dahdi-linux
http://downloads.asterisk.org/pub/telephony/dahdi-tools
http://downloads.asterisk.org/pub/telephony/dahdi-linux-complete

* Mostly stability patches affecting the xpp, wctc4xxp, and wcte43xp drivers.
* Allows compilation against CentOS 5.5
* Includes fixes for a crash when setting a tone zone on a channel currently playing a tone and spurious signaling bit transitions in wcte13xp and wcte43x drivers when running dahdi_cfg repeatedly.

Shortlog of dahdi-linux changes since v2.9.1:
Doug Bailey (1):
wct4xxp: AMI w/CAS errata applies to octal card as well.

Oron Peled (10):
xpp: fix failed multi-PRI E1-T1 transition
xpp: new xbus attribute: dahdi_registration
xpp: prevent double dahdi un-registration
xpp: stability fixes - xusb mutex
xpp: stability -- cleaner xpp_open/close
xpp: stability -- better debug information
xpp: stability -- deadlock in waitfor_xpds()
xpp: stability -- better xbus shut down
xpp: demote some NOTICE() to DBG()
xpp: re-organize calls so worker_reset()

Shaun Ruffell (58):
wcte13xp: Trivial. Remove duplicate pointer to struct pci_dev.
wcte13xp: Remove redundant call to synchronize_irq().
wcte43xp: Close potential unbalanced call to enable_irq().
dahdi: Define pf_fmt() globally in kernel.h
wctc4xxp: Trivial. Remove unused timer_list from struct tcb.
wcte43x: Trivial fix of 'source' in comment.
wcte43x: Build against 2.6.18 and CentOS 5.5
wctc4xxp: Make sure we call the pci_enable_mwi() function.
wctc4xxp: Disable read-line and read-line-multiple PCI commands.
wctc4xxp: Halt the card when an alert is received.
wctc4xxp: Remove unused debug ioctl interface.
wctc4xxp: Replace channel semaphore with channel mutex.
wctc4xxp: Enable the fatal bus error interrupt.
wctc4xxp: Always ack a response packet.
wctc4xxp: Check for shutdown after acquiring the mutex lock.
wctc4xxp: Do not need locks on the transcoder buffers.
wctc4xxp: Do not allow duplicated sequence numbers to be received for the
channels.
wctc4xxp: Only capture commands once they are on the descriptor ring.
wctc4xxp: We always want to ack the responses.
wctc4xxp: Encode the function in the ACK.
wctc4xxp: All the commands do not need to have completions embedded in
them.
wctc4xxp: Cleanup RTP for unopened channels.
wctc4xxp: Trivial removal of the receiveprep function.
wctc4xxp: Reduce the number of locks grabbed when sending commands
wctc4xxp: Make sure csm_encaps commands are sent before RTP.
wctc4xxp: Use hardware timer for polling and not kernel timer
wctc4xxp: channel count does not need to be atomic.
wctc4xxp: Allow the tx and rx descriptor rings to be different sizes
wctc4xxp: Add debug option to print channel stats to kernel log.
Add #include linux/slab.h to all files that call kzalloc|kmalloc|kfree.
pciradio: interruptible_sleep_on_timeout() - msleep_interruptible()
wctc4xxp: Speed up channel setup / tear-down.
wctc4xxp: Handle all known interrupts regardless of mask.
wctc4xxp: Speed up the rate of polling.
wctc4xxp: Fix the timestamp calculation for the RTP stream.
wctc4xxp: Service tx ring in interrupt handler.
wctc4xxp: Prevent exhausting memory in firmware.
wctc4xxp: Trivial fix typo that was preventing firmware load.
wctc4xxp: Reload the firmware if a fatal alert was received.
wctc4xxp: Constrain RTP payload to 500 bytes.
wctc4xxp: Trivial removal of unused structure members.
wctc4xxp: Trivial reduction of indentation level in wctc4xxp_watchdog()
wctc4xxp: spin_lock() - spin_lock_irqsave() in wctc4xxp_watchdog()
dahdi: Protect echocan creation/destruction with mutex.
dahdi: dahdi_chan.ec_factory can be protected with the mutex.
wct4xxp: Move bottom half processing from tasklet to workqueue.
oct612x: Implement the SerializationObject callbacks.
wct4xxp: Trivial kmalloc + memset - kzalloc.
wct4xxp: Remove unused open/close span_ops callbacks.
tor2: Remove unused open/close callbacks.
Do not call dahdi_span_ops.open with spinlock held.
wcte13xp: Do not get stuck in Not Open state when DAHDI_CONFIG_NOTOPEN
is set.
wcte43x: Change span flags to atomic bitfield.
wcte43x: Do not get stuck in Not Open state when DAHDI_CONFIG_NOTOPEN
is set.
wct4xxp: Report rx signalling bit changes after spanconfig.
dahdi: Stop tones on channel when updating tone zone.
wcte13xp: Do not reconfigure framer when span lineconfig is not changed.
wcte43x: Do not reconfigure framer when span lineconfig

[asterisk-dev] [Code Review] 3832: testsuite: Accountcode propagation.

2014-07-21 Thread rmudgett


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

Review request for Asterisk Developers.


Bugs: AFS-65
https://issues.asterisk.org/jira/browse/AFS-65


Repository: testsuite


Description
---

New tests to check accountcode propagation with the new accouncode/peeracccount 
interaction.

* Made pluggable_modules.py Originator class and test_case.py
SimpleTestCase class call origination allow specifying the accountcode.

* Fix tests/cdr/sqlite3 to work with the new accountcode propagation rules.

* Add accountcode tag to existing tests doing something with accountcode.

Testsuite tests to go with https://reviewboard.asterisk.org/r/3601/


Diffs
-

  /asterisk/trunk/tests/pbx/tests.yaml 5288 
  /asterisk/trunk/tests/pbx/accountcode/tests.yaml PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/configs/ast1/queues.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/queue_peer_preserve/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_peer_none/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/queue_peer_none/configs/ast1/queues.conf 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_peer_none/configs/ast1/pjsip.conf 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/queue_peer_none/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_none/test-config.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_none/configs/ast1/queues.conf 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_none/configs/ast1/pjsip.conf 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/queue_none/configs/ast1/extensions.conf 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/local_originate/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/local_originate/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/local_crossover_back/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/local_crossover_back/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/local_crossover/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/local_crossover/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_straight_override/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_straight_override/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_straight_override/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_straight_none/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_straight_none/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_straight_none/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_predial/test-config.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_predial/configs/ast1/pjsip.conf 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_predial/configs/ast1/extensions.conf 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_peer_override/test-config.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_peer_override/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_peer_override/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_peer_none/test-config.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_peer_none/configs/ast1/pjsip.conf 
PRE-CREATION 
  
/asterisk/trunk/tests/pbx/accountcode/dial_peer_none/configs/ast1/extensions.conf
 PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_none/test-config.yaml PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_none/configs/ast1/pjsip.conf 
PRE-CREATION 
  /asterisk/trunk/tests/pbx/accountcode/dial_none/configs/ast1/extensions.conf 
PRE-CREATION 
  /asterisk/trunk/tests/cdr/sqlite3/test-config.yaml 5288 
  /asterisk/trunk/tests/cdr/console_dial_sip_transfer/test-config.yaml 5288 
  /asterisk/trunk/tests/cdr/console_dial_sip_congestion/test-config.yaml 5288 
  /asterisk/trunk/tests/cdr/console_dial_sip_busy/test-config.yaml 5288 
  /asterisk/trunk/tests/cdr/console_dial_sip_answer/test-config.yaml 5288 
  /asterisk/trunk/tests/cdr/cdr_unanswered_yes/test-config.yaml 5288 
  /asterisk/trunk/tests/cdr/cdr_properties/cdr_accountcode/test-config.yaml 
5288 
  
/asterisk/trunk/tests/cdr/cdr_properties/blind-transfer-accountcode/test-config.yaml
 5288

[asterisk-dev] [Code Review] 3833: Allow sending voicemail to multiple email addresses

2014-07-21 Thread Jason Parker


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

Review request for Asterisk Developers and Jacob Barber.


Bugs: ASTERISK-24045
https://issues.asterisk.org/jira/browse/ASTERISK-24045


Repository: Asterisk


Description
---

Allow sending voicemail to multiple email addresses.

Should this supersede https://reviewboard.asterisk.org/r/3829/ ?  I would think 
so.


Diffs
-

  trunk/apps/app_voicemail.c 404951 

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


Testing
---

Patch in FreePBX distro for ~6 months.


Thanks,

Jason Parker

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3729: Stasis: Allow prestart actions to be queued


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

(Updated July 21, 2014, 4:55 p.m.)


Review request for Asterisk Developers.


Changes
---

Address Mark's comment.


Bugs: ASTERISK-23941
https://issues.asterisk.org/jira/browse/ASTERISK-23941


Repository: Asterisk


Description
---

This allows commands to be queued in a channel datastore to be executed upon a 
channel entering Stasis(). This functionality is only available from components 
of res_stasis.so. This functionality is required to get a redirected channel 
back into the bridge for which it was destined due to the attended transfer.


Diffs (updated)
-

  branches/12/res/stasis/control.c 419109 
  branches/12/res/stasis/control.h 419109 
  branches/12/res/stasis/command.c 419109 
  branches/12/res/stasis/command.h 419109 
  branches/12/res/res_stasis.c 419109 

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


Testing
---

Tested as part of the complete solution to ASTERISK-23941.


Thanks,

opticron

-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3673: RLS: Nominal list tests

2014-07-21 Thread Jonathan Rose


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



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/full_state/test-config.yaml
https://reviewboard.asterisk.org/r/3673/#comment23119

missing comma



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/initial_notify/test-config.yaml
https://reviewboard.asterisk.org/r/3673/#comment23116

This test has no sipp folder and no sipp scenarios.

Also you left out a ',' between 'list_subscribe.xml' and '-i'



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/test-config.yaml
https://reviewboard.asterisk.org/r/3673/#comment23118

missing comma



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/test-config.yaml
https://reviewboard.asterisk.org/r/3673/#comment23120

missing comma



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml
https://reviewboard.asterisk.org/r/3673/#comment23121

missing comma



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml
https://reviewboard.asterisk.org/r/3673/#comment23122

missing comma



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml
https://reviewboard.asterisk.org/r/3673/#comment23117

This folder doesn't have a test/directory called simple. It does have a 
directory called presence, and that isn't referenced in this tests.yaml


There is also no tests.yaml file which actually includes the tests from the 
nominal/presence test folder.

- Jonathan Rose


On July 7, 2014, 10:43 a.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3673/
 ---
 
 (Updated July 7, 2014, 10:43 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23870
 https://issues.asterisk.org/jira/browse/ASTERISK-23870
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 This changeset implements the nominal resource list tests outlined on this 
 page: 
 https://wiki.asterisk.org/wiki/display/AST/Resource+List+Subscription+Test+Plan
 
 There are six tests:
 1. Subscription Establishment: Simply ensures that Asterisk responds with a 
 200 OK when we subscribe to a resource list and that the 200 OK has a 
 Require: eventlist header in it.
 2. Initial NOTIFY: Validates the initial NOTIFY body that Asterisk sends when 
 subscribing to a resource list.
 3. Full State: Establishes a subscription to a resource list and then changes 
 the state of a resource. Ensures that Asterisk sends a NOTIFY with full state 
 of the list.
 4. Partial State: Establishes a subscription to a resource list and then 
 changes the state of a resource. Ensures that Asterisk sends a NOTIFY with 
 partial state, with only the state of the resource whose state was changed.
 5. Resubscription Full State: Establishes a subscription and then 
 resubscribes. Ensures that even though partial state is configured, the 
 NOTIFY that Asterisk sends in response to the resubscription has full state 
 of the list.
 6. Termination Full State: Establishes a subscription and then terminates the 
 subscription. Ensures that even though partial state is configured, the 
 NOTIFY that Asterisk sends in response to the termination has full state of 
 the list.
 
 
 Diffs
 -
 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/tests.yaml 
 5168 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/tests.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/rls_integrity.py
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml
  PRE-CREATION

Re: [asterisk-dev] [Code Review] 3833: Allow sending voicemail to multiple email addresses

2014-07-21 Thread Jason Parker


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



trunk/apps/app_voicemail.c
https://reviewboard.asterisk.org/r/3833/#comment23123

Per r3829, don't use tmp as a variable name.


- Jason Parker


On July 21, 2014, 4:54 p.m., Jason Parker wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3833/
 ---
 
 (Updated July 21, 2014, 4:54 p.m.)
 
 
 Review request for Asterisk Developers and Jacob Barber.
 
 
 Bugs: ASTERISK-24045
 https://issues.asterisk.org/jira/browse/ASTERISK-24045
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Allow sending voicemail to multiple email addresses.
 
 Should this supersede https://reviewboard.asterisk.org/r/3829/ ?  I would 
 think so.
 
 
 Diffs
 -
 
   trunk/apps/app_voicemail.c 404951 
 
 Diff: https://reviewboard.asterisk.org/r/3833/diff/
 
 
 Testing
 ---
 
 Patch in FreePBX distro for ~6 months.
 
 
 Thanks,
 
 Jason Parker
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3601: accountcode: Slightly change accountcode propagation.

2014-07-21 Thread rmudgett


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

(Updated July 21, 2014, 5:19 p.m.)


Review request for Asterisk Developers.


Changes
---

Tweak CHANGES queue accountcode propagation description.


Bugs: AFS-65
https://issues.asterisk.org/jira/browse/AFS-65


Repository: Asterisk


Description (updated)
---

Accountcode propagation:

The current behavior is to simply set the accountcode of an outgoing
channel to the accountcode of the channel initiating the call.  It was
done this way a long time ago to allow the accountcode set on the SIP/100
channel to be propagated to a local channel so the dialplan execution on
the Local;2 channel would have the SIP/100 accountcode available.

SIP/100 - Local;1/Local;2 - SIP/200

Propagating the SIP/100 accountcode to the local channels is very useful.
Without any dialplan manipulation, all channels in this call would have
the same accountcode.

Using dialplan, you can set a different accountcode on the SIP/200 channel
either by setting the accountcode on the Local;2 channel or by the Dial
application's b(pre-dial), M(macro) or U(gosub) options, or by the
FollowMe application's b(pre-dial) option, or by the Queue application's
macro or gosub options.  Before Asterisk v12, the altered accountcode on
SIP/200 will remain until the local channels optimize out and the
accountcode would change to the SIP/100 accountcode.

Asterisk v1.8 attempted to add peeraccount support but ultimately had to
punt on the support.  The peeraccount support was rendered useless because
of how the CDR code needed to unconditionally force the caller's
accountcode onto the peer channel's accountcode.  The CEL events were thus
intentionally made to always use the channel's accountcode as the
peeraccount value.

With the arrival of Asterisk v12, the situation has improved somewhat so
peeraccount support can be made to work.  Using the indicated example, the
the accountcode values become as follows when the peeraccount is set on
SIP/100 before calling SIP/200:

SIP/100 --- Local;1  Local;2 --- SIP/200
acct: 100 \/ acct: 200 \/ acct: 100 \/ acct: 200
peer: 200 /\ peer: 100 /\ peer: 200 /\ peer: 100

If a channel already has an accountcode it can only change by the
following explicit user actions:

1) A channel originate method that can specify an accountcode to use.

2) The calling channel propagating its non-empty peeraccount or its
non-empty accountcode if the peeraccount was empty to the outgoing
channel's accountcode before initiating the dial.  e.g., Dial and
FollowMe.  The exception to this propagation method is Queue.  Queue will
only propagate peeraccounts this way only if the outgoing channel does not
have an accountcode.

3) Dialplan using CHANNEL(accountcode).

4) Dialplan using CHANNEL(peeraccount) on the other end of a local
channel pair.

If a channel does not have an accountcode it can get one from the
following places:

1) The channel driver's configuration at channel creation.

2) Explicit user action as already indicated.

3) Entering a basic or stasis-mixing bridge from a peer channel's
peeraccount value.

You can specify the accountcode for an outgoing channel by setting the
CHANNEL(peeraccount) before using the Dial, FollowMe, and Queue
applications.  Queue adds the wrinkle that it will not overwrite an
existing accountcode on the outgoing channel with the calling channels
values.

Accountcode and peeraccount values propagate to an outgoing channel before
dialing.  Accountcodes also propagate when channels enter or leave a basic
or stasis-mixing bridge.  The peeraccount value only makes sense for
mixing bridges with two channels; it is meaningless otherwise.

* Made peeraccount functional by changing accountcode propagation as
described above.

* Fixed CEL extracting the wrong ie value for the peeraccount.  This was
done intentionally in Asterisk v1.8 when that version had to punt on
peeraccount.

* Fixed a few places dealing with accountcodes that were reading from
channels without the lock held.


Diffs (updated)
-

  /trunk/res/parking/parking_bridge_features.c 419127 
  /trunk/main/pbx.c 419127 
  /trunk/main/dial.c 419127 
  /trunk/main/core_unreal.c 419127 
  /trunk/main/channel.c 419127 
  /trunk/main/cel.c 419127 
  /trunk/main/bridge_basic.c 419127 
  /trunk/main/bridge.c 419127 
  /trunk/include/asterisk/channel.h 419127 
  /trunk/apps/app_queue.c 419127 
  /trunk/apps/app_followme.c 419127 
  /trunk/apps/app_dial.c 419127 
  /trunk/UPGRADE.txt 419127 
  /trunk/CHANGES 419127 

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


Testing (updated)
---

* Ran tests from https://reviewboard.asterisk.org/r/3832/ all tests tagged
with accountcode pass.

* Set the accountcode on the calling channel and let the channel driver
supply or not the accountcode for the outgoing

Re: [asterisk-dev] [Code Review] 3673: RLS: Nominal list tests

2014-07-21 Thread Jonathan Rose


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



/asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/initial_notify/notify.py
https://reviewboard.asterisk.org/r/3673/#comment23124

So this should probably be:
from rls_integrity import RLSValidator

That got me a little deeper into execution anyway.


- Jonathan Rose


On July 7, 2014, 10:43 a.m., Mark Michelson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3673/
 ---
 
 (Updated July 7, 2014, 10:43 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-23870
 https://issues.asterisk.org/jira/browse/ASTERISK-23870
 
 
 Repository: testsuite
 
 
 Description
 ---
 
 This changeset implements the nominal resource list tests outlined on this 
 page: 
 https://wiki.asterisk.org/wiki/display/AST/Resource+List+Subscription+Test+Plan
 
 There are six tests:
 1. Subscription Establishment: Simply ensures that Asterisk responds with a 
 200 OK when we subscribe to a resource list and that the 200 OK has a 
 Require: eventlist header in it.
 2. Initial NOTIFY: Validates the initial NOTIFY body that Asterisk sends when 
 subscribing to a resource list.
 3. Full State: Establishes a subscription to a resource list and then changes 
 the state of a resource. Ensures that Asterisk sends a NOTIFY with full state 
 of the list.
 4. Partial State: Establishes a subscription to a resource list and then 
 changes the state of a resource. Ensures that Asterisk sends a NOTIFY with 
 partial state, with only the state of the resource whose state was changed.
 5. Resubscription Full State: Establishes a subscription and then 
 resubscribes. Ensures that even though partial state is configured, the 
 NOTIFY that Asterisk sends in response to the resubscription has full state 
 of the list.
 6. Termination Full State: Establishes a subscription and then terminates the 
 subscription. Ensures that even though partial state is configured, the 
 NOTIFY that Asterisk sends in response to the termination has full state of 
 the list.
 
 
 Diffs
 -
 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/tests.yaml 
 5168 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/tests.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/rls_integrity.py
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/extensions.conf
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/sipp/list_subscribe.xml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/pjsip.conf
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/extensions.conf
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/test-config.yaml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/sipp/resubscribe.xml
  PRE-CREATION 
   
 /asterisk/team/group/rls-tests/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/resubscribe.py
  PRE-CREATION

Re: [asterisk-dev] [Code Review] 3813: codec_speex: Fix trashing normal static frame for AST_FRAME_CNG.

2014-07-21 Thread rmudgett


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


Ping for attention.

- rmudgett


On July 16, 2014, 4:37 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3813/
 ---
 
 (Updated July 16, 2014, 4:37 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Made use a local static frame to generate the AST_FRAME_CNG frame when 
 silence starts.
 
 I don't think the handling of the AST_FRAME_CNG has ever really worked 
 because there doesn't seem to be any consumers of it.
 
 
 Diffs
 -
 
   /team/group/media_formats-reviewed-trunk/codecs/codec_speex.c 418785 
 
 Diff: https://reviewboard.asterisk.org/r/3813/diff/
 
 
 Testing
 ---
 
 It compiles.  I don't have anything that will consume/produce speex.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3813: codec_speex: Fix trashing normal static frame for AST_FRAME_CNG.


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

Ship it!


Ship It!

- Joshua Colp


On July 16, 2014, 9:37 p.m., rmudgett wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3813/
 ---
 
 (Updated July 16, 2014, 9:37 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Made use a local static frame to generate the AST_FRAME_CNG frame when 
 silence starts.
 
 I don't think the handling of the AST_FRAME_CNG has ever really worked 
 because there doesn't seem to be any consumers of it.
 
 
 Diffs
 -
 
   /team/group/media_formats-reviewed-trunk/codecs/codec_speex.c 418785 
 
 Diff: https://reviewboard.asterisk.org/r/3813/diff/
 
 
 Testing
 ---
 
 It compiles.  I don't have anything that will consume/produce speex.
 
 
 Thanks,
 
 rmudgett
 


-- 
_
-- 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

Re: [asterisk-dev] [Code Review] 3759: chan_sip: upgrade registry and mwi object to ao2



 On July 21, 2014, 1:14 p.m., Mark Michelson wrote:
  /trunk/channels/chan_sip.c, lines 3522-3534
  https://reviewboard.asterisk.org/r/3759/diff/3/?file=64859#file64859line3522
 
  Heh, it's a bit late for this now, but it would have probably saved you 
  a bunch of searching and replacing to just change the definition of 
  registry_unref() and registry_addref() to use ao2 in their implementations.

I knew this, but decided to put the work in to use the best ao2 function for 
each use.  I do not like the use of intermediary functions or macro's that 
simply link to ao2 operations.


 On July 21, 2014, 1:14 p.m., Mark Michelson wrote:
  /trunk/channels/chan_sip.c, lines 6543-6547
  https://reviewboard.asterisk.org/r/3759/diff/3/?file=64859#file64859line6543
 
  I agree with this comment. Since mwi-call was created via sip_alloc(), 
  destruction of it should be accomplished with dialog_unref()

I've removed this comment and will post a separate review to fix this in 1.8+.

OTOH I think this code is actually unreachable, we have a circular link - 
mwi-call holds a reference to mwi-call-mwi.  I ran testsuite - 
tests/channels/SIP/subscribe before and after this patch, and mwi leaks.


 On July 21, 2014, 1:14 p.m., Mark Michelson wrote:
  /trunk/channels/chan_sip.c, lines 33324-6
  https://reviewboard.asterisk.org/r/3759/diff/3/?file=64859#file64859line33324
 
  You sometimes search for sip_registrys using OBJ_KEY, but your hash and 
  cmp callbacks do not account for a search key being passed to the callback 
  instead of a sip_registry.
  
  Use the templates on this wiki page 
  https://wiki.asterisk.org/wiki/pages/viewpage.action?pageId=25919686 to 
  define your hash and comparison callbacks and you should be good to go.

I actually only ever used OBJ_KEY.  I've fixed the hash/cmp callbacks to follow 
the template, supporting OBJ_SEARCH_OBJECT in case it's used in the future.  
I've also replaced the use deprecated OBJ_KEY with OBJ_SEARCH_KEY.

When I originally ran the testsuite this was testing for reg1-name, which was 
a char[80] defined as the first field.  Since reg1-name == reg1, it didn't 
cause a problem.  Now that reg1-configvalue is a stringfield, I think this 
would have caused a segfault, though I didn't get one with the testsuite.


- Corey


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


On July 21, 2014, 11:52 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/3759/
 ---
 
 (Updated July 21, 2014, 11:52 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24067
 https://issues.asterisk.org/jira/browse/ASTERISK-24067
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Upgrade all ASTOBJ objects in chan_sip to ao2.
 
 
 Diffs
 -
 
   /trunk/channels/sip/include/sip.h 419127 
   /trunk/channels/chan_sip.c 419127 
 
 Diff: https://reviewboard.asterisk.org/r/3759/diff/
 
 
 Testing
 ---
 
 Full testsuite run.
 
 
 Thanks,
 
 Corey Farrell
 


-- 
_
-- 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

[asterisk-dev] [Code Review] 3834: chan_sip: sip_subscribe_mwi_destroy should not call sip_destroy