Re: [asterisk-dev] [Code Review] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes

2014-09-18 Thread Joshua Colp

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



/branches/13/channels/chan_iax2.c


Despite it not changing behavior I'd still use 20 here to match 12.


- Joshua Colp


On Sept. 16, 2014, 9:28 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3999/
> ---
> 
> (Updated Sept. 16, 2014, 9:28 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24265
> https://issues.asterisk.org/jira/browse/ASTERISK-24265
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This only occurs when the chan_iax jitterbuffer options are set and no when 
> setting jitterbuffers via diaplan or anything like that.
> 
> The first time __get_from_jb is called, voiceformat has not been set on the 
> IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. 
> This worked previously because Asterisk 12 and prior simply modified an 
> ast_format on the stack, so when it used ast_codec_interp_len on that format 
> pointer there was no possibility for it to be a NULL pointer... just one that 
> doesn't have a real format associated with it.
> 
> One thing I might not be doing right here is that I'm using an interpolation 
> value of 0 for a NULL format. Previously Asterisk would just check to see if 
> the format was ILBC and if it was, return 30 and otherwise return 20... so it 
> might be more appropriate to use 20 instead of 0.  It doesn't appear to make 
> a difference for the sake of behavior.
> 
> 
> Diffs
> -
> 
>   /branches/13/channels/chan_iax2.c 423149 
> 
> Diff: https://reviewboard.asterisk.org/r/3999/diff/
> 
> 
> Testing
> ---
> 
> Ran basic call from a PJSIP peer to an IAX peer with the following:
> 
> [general]
> 
> ; The important parts
> jitterbuffer=yes
> forcejitterbuffer=yes
> 
> 
> [deskbox]
> type=friend
> requirecalltoken = no
> username = deskbox
> secret = secret
> host = dynamic
> transfer = no
> dtmfmode = auto
> encryption = no
> qualify = 300
> context = default
> disallow=all
> allow=ulaw
> allow=alaw
> ; Most of this is probably unnecessary for reproduction
> 
> 
> Without the patch this would crash in 13 but work fine in 12.
> With the patch, behavior strongly resembles 12 with the initial call into 
> __get_from_jb attempting to jb_get and getting JB_OK back and then later, 
> when the call was actually answered, the voice format would change and the 
> function would call again with the proper format and the jitterbuffer would 
> get started properly.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Matt Jordan

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



/branches/12/main/stasis_channels.c


You can actually do this without needing to do a safe traverse:

while ((cur = AST_LIST_REMOVE_HEAD(&ds->dialed_peers))) {
   /* Clean up cur */
}



/branches/12/main/stasis_channels.c


Swap these two: Finish the old dial first, then publish the new dial.


- Matt Jordan


On Sept. 17, 2014, 12:57 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 17, 2014, 12:57 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3993: config: bug: Fix SEGV in ast_category_insert when a matching category isn't found

2014-09-18 Thread Matt Jordan

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

Ship it!


Ship It!

- Matt Jordan


On Sept. 15, 2014, 10:03 a.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3993/
> ---
> 
> (Updated Sept. 15, 2014, 10:03 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> If you call ast_category_insert with a match category that doesn't exist, the 
> list traverse runs out of 'next' categories and you get a SEGV.  This patch 
> adds check for the end-of-list condition and changes the signature to return 
> an int for success/failure indication instead of a void.
> 
> The only consumer of this function is manager and it was also changed to use 
> the return value.
> 
> 
> Diffs
> -
> 
>   branches/1.8/main/manager.c 423127 
>   branches/1.8/main/config.c 423127 
>   branches/1.8/include/asterisk/config.h 423127 
> 
> Diff: https://reviewboard.asterisk.org/r/3993/diff/
> 
> 
> Testing
> ---
> 
> Tested with AMI UpdateConfig/newcat to make sure the proper value is returned 
> in both positive and negative cases.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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] 3988: res_pjsip: Don't require a password when doing userpass authentication

2014-09-18 Thread Joshua Colp

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

Ship it!


Ship It!

- Joshua Colp


On Sept. 15, 2014, 7:27 p.m., Sean Bright wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3988/
> ---
> 
> (Updated Sept. 15, 2014, 7:27 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> An empty password is valid for username/password authentication so we 
> shouldn't barf on it.
> 
> 
> Diffs
> -
> 
>   /branches/12/res/res_pjsip/config_auth.c 422963 
> 
> Diff: https://reviewboard.asterisk.org/r/3988/diff/
> 
> 
> Testing
> ---
> 
> Compiles.  Registered a device with auth_type=userpass and no password set.  
> Tested registration with a password which failed, and again without a 
> password (an empty password) and it succeeds.
> 
> 
> 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] 3994: Bridges User Documentation Update

2014-09-18 Thread Matt Jordan

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

Ship it!


Ship It!

- Matt Jordan


On Sept. 15, 2014, 10:46 a.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3994/
> ---
> 
> (Updated Sept. 15, 2014, 10:46 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-23469
> None
> 
> 
> Description
> ---
> 
> I have updated the Bridges documentation for Asterisk 12/13. I'd love any 
> feedback on the structure, completeness, and correctness of the information. 
> You can find the documentation here:
> https://wiki.asterisk.org/wiki/display/AST/Bridges
> 
> Note that this is user documentation, not developer documentation.
> 
> 
> Diffs
> -
> 
> 
> Diff: https://reviewboard.asterisk.org/r/3994/diff/
> 
> 
> Testing
> ---
> 
> N/A
> 
> 
> 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] 3994: Bridges User Documentation Update

2014-09-18 Thread opticron

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

(Updated Sept. 18, 2014, 8:20 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Bugs: ASTERISK-23469
None


Description
---

I have updated the Bridges documentation for Asterisk 12/13. I'd love any 
feedback on the structure, completeness, and correctness of the information. 
You can find the documentation here:
https://wiki.asterisk.org/wiki/display/AST/Bridges

Note that this is user documentation, not developer documentation.


Diffs
-


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


Testing
---

N/A


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] Git Migration

2014-09-18 Thread Matthew Jordan
On Wed, Sep 17, 2014 at 11:21 AM, Paul Belanger <
paul.belan...@polybeacon.com> wrote:

> On Tue, Sep 16, 2014 at 6:01 PM, Russell Bryant
>  wrote:
> > On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan 
> wrote:
> >>
>
>



> Just to echo everything Russell typed, I also recommend above.  While
> complicated and full of moving parts, its extremely good at what it
> does.  I have a system running both public and private for different
> projects I am doing.  One thing that is great about it, developers can
> develop faster without knowing or understanding the release processes
> of a project.
>
> I think the issue that you'll run into the is amount of time and
> effort to learn the system and understand the workings. I don't know
> the official timelines, however if we are still talking about this at
> Astricon, I don't mind sitting down with people and hashing it out.
>
>
I'd like to move everything over prior to starting the major work on
Asterisk 14. That wouldn't occur until after AstriCon at the earliest, so
we can certainly sit down and talk about this some more then.


> Additionally, I'd offer up my public infrastructure for a demo or POC
> of the asterisk testsuite.  Infact, once the project was converted
> into git, it would only take a few minutes to import it into the
> process.  The, people could do test commits / see how the system
> works.
>

That's awesome Paul - thanks!

I know there's a few small issues we'd have to work through in just the
initial conversion process, but once we've done that we can test it out.

I do think the next step is to see how gerritt can interact with crowd and
the rest of the existing infrastructure. We can do that in parallel with
other things however.

Matt

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com & http://asterisk.org
-- 
_
-- 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] 3993: config: bug: Fix SEGV in ast_category_insert when a matching category isn't found

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 9:37 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 423276


Repository: Asterisk


Description
---

If you call ast_category_insert with a match category that doesn't exist, the 
list traverse runs out of 'next' categories and you get a SEGV.  This patch 
adds check for the end-of-list condition and changes the signature to return an 
int for success/failure indication instead of a void.

The only consumer of this function is manager and it was also changed to use 
the return value.


Diffs
-

  branches/1.8/main/manager.c 423127 
  branches/1.8/main/config.c 423127 
  branches/1.8/include/asterisk/config.h 423127 

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


Testing
---

Tested with AMI UpdateConfig/newcat to make sure the proper value is returned 
in both positive and negative cases.


Thanks,

George Joseph

-- 
_
-- 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] 3972: Only install dahdi_span_config_hook if DAHDI is enabled

2014-09-18 Thread David Lee

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

(Updated Sept. 18, 2014, 10 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 423281


Repository: Asterisk


Description
---

I'm a bit weird, and I configure asterisk with --prefix=/opt/asterisk,
so that I can install it without running as root. The install script
for the DAHDI hook scripts were hard coded to install into /usr/share,
which foils my weirdness.

This patch changes the install to only install the hook script if
DAHDI is enabled. It also adds the script to the uninstall task, and
moves the DAHDI_UDEV_HOOK_DIR variable so that it's not between the
_MAKEOPTS variables and their comment.


Diffs
-

  /branches/13/makeopts.in 422582 
  /branches/13/Makefile 422582 

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


Testing
---

Ran ./configure && make all install both with and without DAHDI,
confirming that it worked as expected.


Thanks,

David Lee

-- 
_
-- 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] 3998: res_pjsip: ami: Fix error in AMI output when no transport is associated to an endpoint

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 10:01 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and Kevin Harwell.


Changes
---

Committed in revision 423282


Bugs: 24161 and 24331
https://issues.asterisk.org/jira/browse/24161
https://issues.asterisk.org/jira/browse/24331


Repository: Asterisk


Description
---

When no transport is associated to an endpoint, the AMI output for 
PJSIPShowEndpoint indicates an error instead of silently ignoring the missing 
transport.

This patch causes the error to appear only if a transport was specified on the 
endpoint and the transport doesn't exist.  It also fixes an issue with counting 
the objects that were actually found.


Diffs
-

  branches/12/res/res_pjsip_endpoint_identifier_ip.c 423274 
  branches/12/res/res_pjsip/pjsip_configuration.c 423274 
  branches/12/res/res_pjsip/location.c 423274 
  branches/12/res/res_pjsip/config_transport.c 423274 
  branches/12/res/res_pjsip/config_auth.c 423274 
  branches/12/include/asterisk/res_pjsip.h 423274 

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


Testing
---

Checked the output of PJSIPShowEndpoint to make sure the error appears (or 
doesn't) correctly and that ListItems is set correctly.


Thanks,

George Joseph

-- 
_
-- 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] Git Migration

2014-09-18 Thread Samuel Galarneau
On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant 
wrote:

> On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan 
> wrote:
>
>> "And there was much rejoicing"
>>
>
> \o/
>
>
>> But seriously, we all know that a lot of people have wanted to move to
>> Git for some time. For the record, everyone at Digium has wanted to move
>> the project to Git for some time. I swore to myself that we wouldn't do
>> another Standard release on Subversion - after we spent at least six weeks
>> mucking around with merge conflicts during Asterisk 12 - and with Asterisk
>> 14 looming ever closer, the time is now to start getting something done on
>> this.
>>
> ...
> -- Team repos
>
> I'd recommend just using your own account on github or whatever.
>
> ...
>
> -- Process Recommendation
>
> I discussed this a good bit above, but I'm happy to answer questions.
>
> --
> Russell Bryant
>

Russell,

how does Gerrit deal with submitting reviews? Are all reviews simply topic
branches on the repository that Gerrit hosts?

What about a pull request workflow where the repository is forked during
development, can Gerrit support this in some way? Just trying to understand
how team repos on Github or some other platform could be used for
development purposes.

-- 

Samuel Fortier-Galarneau
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at:  www.digium.com  & www.asterisk.org
-- 
_
-- 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose

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

(Updated Sept. 18, 2014, 10:59 a.m.)


Review request for Asterisk Developers, Matt Jordan and rmudgett.


Changes
---

Hit mjordan's suggestions.


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


Repository: Asterisk


Description
---

Reproduction: 
pj123 calls 1601
pj123 does a SIP blonde transfer to 1603
1603 answers
FRACK occurs
all are PJSIP endpoints.

Basically what happens is there is a second outbound dial from another pj123 
channel. Before the dial is answered, the pj123 gets masqueraded out of the 
picture and replaced with a channel that isn't in the dial state.

This patch fixes that by advancing the incoming channel to the dial state in 
the channel breakdown function of a datastore on the pj123 channel. Honestly, 
I'm not sure this is entirely adequate, but it seems to work in all of the 
conditions I've tried so far and it doesn't impede normal attended transfers. I 
might need to try and figure out what happens if I masquerade in a channel that 
is already dialing though. I'm not sure if that's something we can really 
expect to happen under normal conditions, but that seems like something that 
would screw up this approach.

Note that I think this patch is the right idea, I just don't know if I need to 
account for the possibility that the channel that masquerades into pj123's 
dialing channel might not be in the neutral state.


Diffs (updated)
-

  /branches/12/main/stasis_channels.c 422882 

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


Testing
---

Ran against reproduction of the above scenario. Assertion was gone and the CDR 
results were as follows:

"","123","1601","default"," 
<123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
 21:48:51","2014-09-11 21:48:53","2014-09-11 
21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
"","123","","default"," 
<123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
 21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
ANSWER","DOCUMENTATION","1410472135.6",""
"","1601","1603","default"," 
<1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""

And dial events reported by AMI:
http://pastebin.com/tWuwL7xa
(note that there is a mismatch between the number of dial end and dial begin 
events... not sure if this is a problem, but I might be able to fix it just by 
updating the old chan, not sure what status code to use though).

Ran against normal attended transfer, feature attended transfers, and blind 
transfers with no noticeable effect.

Ran against testsuite blonde transfer tests, some attended transfer tests, some 
blind transfer tests, and all pjsip transfer tests (at least ones that will run 
on my box... a few won't due to sipp version requirements that I really need to 
get around to fixing eventually) for comparison purposes. All passed exhibiting 
the same behavior as before as far as warning messages and such go.


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] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes

2014-09-18 Thread Jonathan Rose


> On Sept. 18, 2014, 8:12 a.m., Joshua Colp wrote:
> > /branches/13/channels/chan_iax2.c, line 4126
> > 
> >
> > Despite it not changing behavior I'd still use 20 here to match 12.

Alright, fixed that. The only difference in the code is that 20 is there 
instead of 0, so I think I'll hold off on actually posting another review for 
now.


- Jonathan


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


On Sept. 16, 2014, 4:28 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3999/
> ---
> 
> (Updated Sept. 16, 2014, 4:28 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24265
> https://issues.asterisk.org/jira/browse/ASTERISK-24265
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This only occurs when the chan_iax jitterbuffer options are set and no when 
> setting jitterbuffers via diaplan or anything like that.
> 
> The first time __get_from_jb is called, voiceformat has not been set on the 
> IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. 
> This worked previously because Asterisk 12 and prior simply modified an 
> ast_format on the stack, so when it used ast_codec_interp_len on that format 
> pointer there was no possibility for it to be a NULL pointer... just one that 
> doesn't have a real format associated with it.
> 
> One thing I might not be doing right here is that I'm using an interpolation 
> value of 0 for a NULL format. Previously Asterisk would just check to see if 
> the format was ILBC and if it was, return 30 and otherwise return 20... so it 
> might be more appropriate to use 20 instead of 0.  It doesn't appear to make 
> a difference for the sake of behavior.
> 
> 
> Diffs
> -
> 
>   /branches/13/channels/chan_iax2.c 423149 
> 
> Diff: https://reviewboard.asterisk.org/r/3999/diff/
> 
> 
> Testing
> ---
> 
> Ran basic call from a PJSIP peer to an IAX peer with the following:
> 
> [general]
> 
> ; The important parts
> jitterbuffer=yes
> forcejitterbuffer=yes
> 
> 
> [deskbox]
> type=friend
> requirecalltoken = no
> username = deskbox
> secret = secret
> host = dynamic
> transfer = no
> dtmfmode = auto
> encryption = no
> qualify = 300
> context = default
> disallow=all
> allow=ulaw
> allow=alaw
> ; Most of this is probably unnecessary for reproduction
> 
> 
> Without the patch this would crash in 13 but work fine in 12.
> With the patch, behavior strongly resembles 12 with the initial call into 
> __get_from_jb attempting to jb_get and getting JB_OK back and then later, 
> when the call was actually answered, the voice format would change and the 
> function would call again with the proper format and the jitterbuffer would 
> get started properly.
> 
> 
> 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] 3999: chan_iax2: Jitterbuffer causes crash in Asterisk 13 on account of format changes

2014-09-18 Thread Joshua Colp

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

Ship it!


Ship It!

- Joshua Colp


On Sept. 16, 2014, 9:28 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3999/
> ---
> 
> (Updated Sept. 16, 2014, 9:28 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24265
> https://issues.asterisk.org/jira/browse/ASTERISK-24265
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This only occurs when the chan_iax jitterbuffer options are set and no when 
> setting jitterbuffers via diaplan or anything like that.
> 
> The first time __get_from_jb is called, voiceformat has not been set on the 
> IAX pvt. Trying to call ast_format_get_default_ms on a NULL pointer fails. 
> This worked previously because Asterisk 12 and prior simply modified an 
> ast_format on the stack, so when it used ast_codec_interp_len on that format 
> pointer there was no possibility for it to be a NULL pointer... just one that 
> doesn't have a real format associated with it.
> 
> One thing I might not be doing right here is that I'm using an interpolation 
> value of 0 for a NULL format. Previously Asterisk would just check to see if 
> the format was ILBC and if it was, return 30 and otherwise return 20... so it 
> might be more appropriate to use 20 instead of 0.  It doesn't appear to make 
> a difference for the sake of behavior.
> 
> 
> Diffs
> -
> 
>   /branches/13/channels/chan_iax2.c 423149 
> 
> Diff: https://reviewboard.asterisk.org/r/3999/diff/
> 
> 
> Testing
> ---
> 
> Ran basic call from a PJSIP peer to an IAX peer with the following:
> 
> [general]
> 
> ; The important parts
> jitterbuffer=yes
> forcejitterbuffer=yes
> 
> 
> [deskbox]
> type=friend
> requirecalltoken = no
> username = deskbox
> secret = secret
> host = dynamic
> transfer = no
> dtmfmode = auto
> encryption = no
> qualify = 300
> context = default
> disallow=all
> allow=ulaw
> allow=alaw
> ; Most of this is probably unnecessary for reproduction
> 
> 
> Without the patch this would crash in 13 but work fine in 12.
> With the patch, behavior strongly resembles 12 with the initial call into 
> __get_from_jb attempting to jb_get and getting JB_OK back and then later, 
> when the call was actually answered, the voice format would change and the 
> function would call again with the proper format and the jitterbuffer would 
> get started properly.
> 
> 
> 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] 3971: astobj2.c/refcounter.py: Fix to deal with invalid object refs.

2014-09-18 Thread rmudgett

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

(Updated Sept. 18, 2014, 11:08 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 423349


Bugs: AFS-155 and ASTERISK-24295
https://issues.asterisk.org/jira/browse/AFS-155
https://issues.asterisk.org/jira/browse/ASTERISK-24295


Repository: Asterisk


Description
---

* Make astob2 REF_DEBUG output an invalid object line when an invalid ao2 
object ref/unref is attempted.  This is similar to the constructor/destructor 
lines.

* Fixed refcounter.py to handle skewed objects that have constructor/destructor 
states.

* Made refcounter.py highlight the invalid ao2 object refs by putting them in 
their own section of the processed output file.

* Made refcounter.py highlight unreffing an object by more than one that 
results in a negative ref count and the object being destroyed.  The abnormally 
destroyed object is reported in the invalid and finalized object sections of 
the output.


Diffs
-

  /branches/1.8/main/astobj2.c 423191 
  /branches/1.8/contrib/scripts/refcounter.py 423191 

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


Testing
---

Instigated a REF_DEBUG log that reffed destroyed objects to get an invalid line 
in the ref debug log file.
The patched script doesn't have an excpetion on a previous invalid ref debug 
log that had code unreffing a destroyed object.
The patched script now highlights the invalid objects in the log.

Tested unreffing a newly created object by two and then unreffing the object 
again.  The abnormally destroyed object is reported in the invalid and 
finalized sections with an additional log line indicating that the object was 
destroyed abnormally.  The next unref is reported as an unref of an invalid 
object.


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] Git Migration

2014-09-18 Thread Russell Bryant
On Thu, Sep 18, 2014 at 11:31 AM, Samuel Galarneau 
wrote:

>
>
> On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant  > wrote:
>
>> On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan 
>> wrote:
>>
>>> "And there was much rejoicing"
>>>
>>
>> \o/
>>
>>
>>> But seriously, we all know that a lot of people have wanted to move to
>>> Git for some time. For the record, everyone at Digium has wanted to move
>>> the project to Git for some time. I swore to myself that we wouldn't do
>>> another Standard release on Subversion - after we spent at least six weeks
>>> mucking around with merge conflicts during Asterisk 12 - and with Asterisk
>>> 14 looming ever closer, the time is now to start getting something done on
>>> this.
>>>
>> ...
>> -- Team repos
>>
>> I'd recommend just using your own account on github or whatever.
>>
>> ...
>>
>> -- Process Recommendation
>>
>> I discussed this a good bit above, but I'm happy to answer questions.
>>
>> --
>> Russell Bryant
>>
>
> Russell,
>
> how does Gerrit deal with submitting reviews? Are all reviews simply topic
> branches on the repository that Gerrit hosts?
>

Perhaps a real demonstration of workflow would help.  I'll use a recent
trivial fix that I did.  This is a one-liner patch that needed to go into
master as well as two stable branches.

I headed over to my local git tree and created a branch to do the fix.

$ cd openstack/nova
$ git checkout -b bug/1370191 origin/master

... Hack code and commit the fix ...

$ git commit -a

Now I have a single patch on top of upstream master that I want to submit
for review.

$ git review

This created https://review.openstack.org/#/c/121940/

What's actually happening is a git push to gerrit.  A git repo in gerrit
maintains all revisions of all patches.  You can actually fetch the patch
and look at it locally in your tree.

$ git fetch https://review.openstack.org/openstack/nova
refs/changes/40/121940/1 && git checkout FETCH_HEAD

or as a shortcut:

$ git review -d 121940

>From there a few CI systems ran tests against the patch automatically and
posted their results to the review.  Also, some people reviewed the code
and it was approved and automatically merged.

Let's say that someone found a bug in my patch and I needed to update it.
 That process would be:

... Hack the code and then amend the commit ...

$ git commit -a --amend
$ git review

That git-review would push a second revision to the existing code review.

Once my patch against master was merged, I wanted to propose it for merging
into the stable branches.  You can do this manually (make a local branch
based on upstream stable, cherry-pick the commit, and git-review).  In this
case, the patch was trivial and had no conflicts, so I was able to just
press the "Cherry-pick to branch" button in gerrit.  The result is 3 code
reviews:

https://review.openstack.org/#/q/I93b370d6457d2e85493be01a62a76404d228a6fa,n,z

My stable backports are still under review at the time of writing.

What about a pull request workflow where the repository is forked during
> development, can Gerrit support this in some way? Just trying to understand
> how team repos on Github or some other platform could be used for
> development purposes.
>

Sure.  This case would be a bit of an expansion on the above.  Let's say
you're working on something a bit more involved.  Your feature may be a
series of 15 commits. Since you're doing so much work, you don't want it to
only exist on your hard drive.  So, you periodically push a branch to
github for both visibility and backup purposes.  When you're ready for
review, you just run git-review as usual.

When you go to post the review, git-review will warn you and say something
 like "you're about to push more than 1 patch up for review, are you sure
that's what you want?".  Yes, it is indeed what you want.

At that point, you would have 15 new code reviews created.  Gerrit
understands that these are all related and notes the dependencies between
them.

One example would be
https://review.openstack.org/#/q/status:merged+project:openstack/nova+branch:master+topic:bp/convert-ec2-api-to-use-nova-objects,n,z

Take a look at a specific patch like this one:
https://review.openstack.org/#/c/113389/

Find the "Dependencies" section on the review and expand it.  You'll see
that this patch was one in the middle of a patch series.  It depended on
one patch and another patch depended on this one.

Hope that helps,

-- 
Russell Bryant
-- 
_
-- 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] 3924: Testsuite: Add modular event action framework

2014-09-18 Thread Scott Griepentrog

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

Ship it!


Ship It!

- Scott Griepentrog


On Sept. 11, 2014, 8:04 a.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3924/
> ---
> 
> (Updated Sept. 11, 2014, 8:04 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: testsuite
> 
> 
> Description
> ---
> 
> This adds a modular event/action system that can handle routing from 
> arbitrary events to arbitrary actions allowing creation of more flexible 
> tests. This includes event modules for AMI and ARI actions as well as action 
> modules for AMI, ARI, callbacks, and more.
> 
> This also converts three tests as a proof of concept.
> 
> 
> Diffs
> -
> 
>   asterisk/trunk/tests/rest_api/bridges/bridge_by_id/test-config.yaml 5568 
>   asterisk/trunk/tests/rest_api/bridges/attended_transfer/test-config.yaml 
> 5568 
>   
> asterisk/trunk/tests/rest_api/bridges/attended_transfer/attended_transfer.py 
> 5568 
>   asterisk/trunk/tests/asyncagi/asyncagi_break/test-config.yaml 5568 
>   asterisk/trunk/tests/asyncagi/asyncagi_break/asyncagi_break.py 5568 
>   asterisk/trunk/lib/python/asterisk/pluggable_registry.py PRE-CREATION 
>   asterisk/trunk/lib/python/asterisk/pluggable_modules.py 5568 
>   asterisk/trunk/lib/python/asterisk/ari.py 5568 
>   asterisk/trunk/lib/python/asterisk/ami.py 5568 
> 
> Diff: https://reviewboard.asterisk.org/r/3924/diff/
> 
> 
> Testing
> ---
> 
> Ran the tests and verified that events and actions were triggering properly.
> 
> 
> 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] 3995: res_pjsip_endpoint_identifier_ip: Can't parse identify with match value containing CIDR

2014-09-18 Thread Jonathan Rose

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

(Updated Sept. 18, 2014, 11:44 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and Joshua Colp.


Changes
---

Committed in revision 423417


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


Repository: Asterisk


Description
---

Using a value such as '10.24.0.0/16' would fail to match because 
ast_sockaddr_resolve can only parse the following formats:

 * hostname:port
 * host.example.com:port
 * a.b.c.d
 * a.b.c.d:port
 * a:b:c:...:d
 * [a:b:c:...:d]
 * [a:b:c:...:d]:port

When the format doesn't match one of these, the function fails and we bail.

To get around this, I simply checked for the presence of a '/' in the identify 
string and used ast_append_ha directly with the address if it was present.


Diffs
-

  /branches/12/res/res_pjsip_endpoint_identifier_ip.c 423062 

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


Testing
---

Used CLI command 'pjsip show endpoint 1603' with an endpoint that had the 
following identifier:

[1603]
type=identify
match=10.24.18.13/16
endpoint=1603


Before, the address would fail to parse and the command would show no identifier
After, the address would parse correctly and show '10.24.0.0/16' for the 
identifier as seen in:

 Endpoint:  1603/1603Not in use
0 of inf
Aor:  1603   5
  Contact:  1603/sip:1603@10.24.18.13:5060;obUnknown
   nan
   Identify:  10.24.0.0/16

I tried a few other things, such as not using a CIDR and using a hostname to 
verify that there wasn't any obvious deviation in behavior introduced by the 
patch.


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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Matt Jordan

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



/branches/12/main/stasis_channels.c


This will terminate the dial masquerade datastore on the first dial 
completion message.

Say I'm in a parallel dial. I've dialled Alice, Bob, and Charlie. I 
complete the dial to Alice, and publish my message. That removes the datastore.

Can I be masqueraded out before I also complete the dial with Bob and 
Charlie?


- Matt Jordan


On Sept. 18, 2014, 10:59 a.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 10:59 a.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose


> On Sept. 18, 2014, 12:22 p.m., Matt Jordan wrote:
> > /branches/12/main/stasis_channels.c, line 383
> > 
> >
> > This will terminate the dial masquerade datastore on the first dial 
> > completion message.
> > 
> > Say I'm in a parallel dial. I've dialled Alice, Bob, and Charlie. I 
> > complete the dial to Alice, and publish my message. That removes the 
> > datastore.
> > 
> > Can I be masqueraded out before I also complete the dial with Bob and 
> > Charlie?

Good point.  Perhaps what I should be doing here is removing individual entries 
from the existing dialstore rather than removing it wholesale.


- Jonathan


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


On Sept. 18, 2014, 10:59 a.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 10:59 a.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose


> On Sept. 18, 2014, 12:22 p.m., Matt Jordan wrote:
> > /branches/12/main/stasis_channels.c, line 383
> > 
> >
> > This will terminate the dial masquerade datastore on the first dial 
> > completion message.
> > 
> > Say I'm in a parallel dial. I've dialled Alice, Bob, and Charlie. I 
> > complete the dial to Alice, and publish my message. That removes the 
> > datastore.
> > 
> > Can I be masqueraded out before I also complete the dial with Bob and 
> > Charlie?
> 
> Jonathan Rose wrote:
> Good point.  Perhaps what I should be doing here is removing individual 
> entries from the existing dialstore rather than removing it wholesale.

I've checked through your suggested scenario with some added sleeps and it 
looks like what you've described is technically possible.


- Jonathan


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


On Sept. 18, 2014, 10:59 a.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 10:59 a.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose

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

(Updated Sept. 18, 2014, 1:21 p.m.)


Review request for Asterisk Developers, Matt Jordan and rmudgett.


Changes
---

Address mjordan's masquerade during parallel dial between events that happen 
with the canceled dial and the events that happen on the dial that stays alive. 
 It's kind of weird.

I addressed it by changing the function that was purging the datastore entirely 
to remove only items belonging to the same channel. Hopefully I didn't miss 
anything important.


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


Repository: Asterisk


Description
---

Reproduction: 
pj123 calls 1601
pj123 does a SIP blonde transfer to 1603
1603 answers
FRACK occurs
all are PJSIP endpoints.

Basically what happens is there is a second outbound dial from another pj123 
channel. Before the dial is answered, the pj123 gets masqueraded out of the 
picture and replaced with a channel that isn't in the dial state.

This patch fixes that by advancing the incoming channel to the dial state in 
the channel breakdown function of a datastore on the pj123 channel. Honestly, 
I'm not sure this is entirely adequate, but it seems to work in all of the 
conditions I've tried so far and it doesn't impede normal attended transfers. I 
might need to try and figure out what happens if I masquerade in a channel that 
is already dialing though. I'm not sure if that's something we can really 
expect to happen under normal conditions, but that seems like something that 
would screw up this approach.

Note that I think this patch is the right idea, I just don't know if I need to 
account for the possibility that the channel that masquerades into pj123's 
dialing channel might not be in the neutral state.


Diffs (updated)
-

  /branches/12/main/stasis_channels.c 422882 

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


Testing
---

Ran against reproduction of the above scenario. Assertion was gone and the CDR 
results were as follows:

"","123","1601","default"," 
<123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
 21:48:51","2014-09-11 21:48:53","2014-09-11 
21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
"","123","","default"," 
<123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
 21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
ANSWER","DOCUMENTATION","1410472135.6",""
"","1601","1603","default"," 
<1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""

And dial events reported by AMI:
http://pastebin.com/tWuwL7xa
(note that there is a mismatch between the number of dial end and dial begin 
events... not sure if this is a problem, but I might be able to fix it just by 
updating the old chan, not sure what status code to use though).

Ran against normal attended transfer, feature attended transfers, and blind 
transfers with no noticeable effect.

Ran against testsuite blonde transfer tests, some attended transfer tests, some 
blind transfer tests, and all pjsip transfer tests (at least ones that will run 
on my box... a few won't due to sipp version requirements that I really need to 
get around to fixing eventually) for comparison purposes. All passed exhibiting 
the same behavior as before as far as warning messages and such go.


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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose

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



/branches/12/main/stasis_channels.c


Oops, I need a break here.


- Jonathan Rose


On Sept. 18, 2014, 1:21 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 1:21 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose

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

(Updated Sept. 18, 2014, 1:25 p.m.)


Review request for Asterisk Developers, Matt Jordan and rmudgett.


Changes
---

Add a break in the function to remove an entry from the datastore.  I don't 
think it's possible to have the same channel in there for two separate events, 
but there's no sense in continuing to traverse past the found entry anyway.


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


Repository: Asterisk


Description
---

Reproduction: 
pj123 calls 1601
pj123 does a SIP blonde transfer to 1603
1603 answers
FRACK occurs
all are PJSIP endpoints.

Basically what happens is there is a second outbound dial from another pj123 
channel. Before the dial is answered, the pj123 gets masqueraded out of the 
picture and replaced with a channel that isn't in the dial state.

This patch fixes that by advancing the incoming channel to the dial state in 
the channel breakdown function of a datastore on the pj123 channel. Honestly, 
I'm not sure this is entirely adequate, but it seems to work in all of the 
conditions I've tried so far and it doesn't impede normal attended transfers. I 
might need to try and figure out what happens if I masquerade in a channel that 
is already dialing though. I'm not sure if that's something we can really 
expect to happen under normal conditions, but that seems like something that 
would screw up this approach.

Note that I think this patch is the right idea, I just don't know if I need to 
account for the possibility that the channel that masquerades into pj123's 
dialing channel might not be in the neutral state.


Diffs (updated)
-

  /branches/12/main/stasis_channels.c 422882 

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


Testing
---

Ran against reproduction of the above scenario. Assertion was gone and the CDR 
results were as follows:

"","123","1601","default"," 
<123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
 21:48:51","2014-09-11 21:48:53","2014-09-11 
21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
"","123","","default"," 
<123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
 21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
ANSWER","DOCUMENTATION","1410472135.6",""
"","1601","1603","default"," 
<1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""

And dial events reported by AMI:
http://pastebin.com/tWuwL7xa
(note that there is a mismatch between the number of dial end and dial begin 
events... not sure if this is a problem, but I might be able to fix it just by 
updating the old chan, not sure what status code to use though).

Ran against normal attended transfer, feature attended transfers, and blind 
transfers with no noticeable effect.

Ran against testsuite blonde transfer tests, some attended transfer tests, some 
blind transfer tests, and all pjsip transfer tests (at least ones that will run 
on my box... a few won't due to sipp version requirements that I really need to 
get around to fixing eventually) for comparison purposes. All passed exhibiting 
the same behavior as before as far as warning messages and such go.


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

[asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread rmudgett

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

Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

Outgoing PJSIP calls can result in non-negotiated formats listed in the 
channel's native formats if video formats are listed in the endpoint's 
configuration.  The resulting call could then use a non-negotiated format 
resulting in one way audio.

* Simplified the update of session->req_caps in set_caps().  Why do something 
in five steps when only one is needed?


Diffs
-

  /branches/13/res/res_pjsip_sdp_rtp.c 423446 
  /branches/13/channels/chan_pjsip.c 423446 

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


Testing
---

Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
Called from D40 with g722 among other formats enabled to a Polycom that 
negotiates ulaw.
Before the patch, Asterisk would send g722 frames to the Polycom.  The 
resulting call had one way audio because the Polycom does not understand g722.
After the patch, Asterisk sends ulaw frames to the Polycom.


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] 3989: utils: Create ast_strsep function that ignores separators inside quotes.

2014-09-18 Thread opticron

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

Ship it!


Ship It!

- opticron


On Sept. 15, 2014, 2:43 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3989/
> ---
> 
> (Updated Sept. 15, 2014, 2:43 p.m.)
> 
> 
> Review request for Asterisk Developers and rmudgett.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> I'm going to need this for my imminent manager and config
> enhancements but I thought I'd post this separately.
> 
> /*!
>   \brief Act like strsep but ignore separators inside quotes.
>   \param s Pointer to address of the the string to be processed.
>   Will be modified and can't be constant.
>   \param sep A single character delimiter.
>   \param flags Controls post-processing of the result.
>   AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
>   AST_STRSEP_STRIP does a trim then strips the outermost quotes.  You may want
>   to trim again after the strip.  Just OR both the TRIM and STRIP flags.
>   AST_STRSEP_UNESCAPE unescapes '\' sequences.
>   AST_STRSEP_ALL does all of the above processing.
>   \return The next token or NULL if done or if there are more than 8 levels of
>   nested quotes.
> 
>   This function acts like strsep with three exceptions...
>   The separator is a single character instead of a string.
>   Separators inside quotes are treated literally instead of like separators.
>   You can elect to have leading and trailing whitespace and quotes
>   stripped from the result and have '\' sequences unescaped.
> 
>   Like strsep, ast_strsep maintains no internal state and you can call it
>   recursively using different separators on the same storage.
> 
>   Also like strsep, for consistent results, consecutive separators are not
>   collapsed so you may get an empty string as a valid result.
> 
>   Examples:
>   \code
>   char *mystr = ast_strdupa("abc=def,ghi='zzz=yyy,456',jkl");
>   char *token, *token2, *token3;
> 
>   while((token = ast_strsep(&mystr, ',', AST_SEP_STRIP))) {
>   // 1st token will be aaa=def
>   // 2nd token will be ghi='zzz=yyy,456'
>   while((token2 = ast_strsep(&token, '=', AST_SEP_STRIP))) {
>   // 1st token2 will be ghi
>   // 2nd token2 will be zzz=yyy,456
>   while((token3 = ast_strsep(&token2, ',', 
> AST_SEP_STRIP))) {
>   // 1st token3 will be zzz=yyy
>   // 2nd token3 will be 456
>   // and so on
>   }
>   }
>   // 3rd token will be jkl
>   }
> 
>   \endcode
>  */
> char *ast_strsep(char **s, const char sep, uint32_t flags);
> 
> 
> Diffs
> -
> 
>   branches/12/tests/test_strings.c 422963 
>   branches/12/main/utils.c 422963 
>   branches/12/include/asterisk/strings.h 422963 
> 
> Diff: https://reviewboard.asterisk.org/r/3989/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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] 3967: Subscriptoin state test events for review 3966

2014-09-18 Thread Mark Michelson

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

(Updated Sept. 18, 2014, 1:31 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 423462


Repository: Asterisk


Description
---

These are some simple test events that can be used by the testsuite to know the 
current state of a subscription in Asterisk. Since these are meant for simple 
tests, not much information is given except that "a subscription has changed to 
state X". It does not attempt to indicate which subscription has changed in any 
way.

Without these events, the tests on /r/3966 cannot run.


Diffs
-

  /branches/13/res/res_pjsip_pubsub.c 422536 

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


Testing
---

/r/3966 exercises these events and has tested that the events are sent when 
expected.


Thanks,

Mark Michelson

-- 
_
-- 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] Git Migration

2014-09-18 Thread Russell Bryant
On Thu, Sep 18, 2014 at 12:29 PM, Russell Bryant 
wrote:

> On Thu, Sep 18, 2014 at 11:31 AM, Samuel Galarneau 
> wrote:
>
>>
>>
>> On Tue, Sep 16, 2014 at 5:01 PM, Russell Bryant <
>> russ...@russellbryant.net> wrote:
>>
>>> On Tue, Sep 16, 2014 at 3:48 PM, Matthew Jordan 
>>> wrote:
>>>
 "And there was much rejoicing"

>>>
>>> \o/
>>>
>>>
 But seriously, we all know that a lot of people have wanted to move to
 Git for some time. For the record, everyone at Digium has wanted to move
 the project to Git for some time. I swore to myself that we wouldn't do
 another Standard release on Subversion - after we spent at least six weeks
 mucking around with merge conflicts during Asterisk 12 - and with Asterisk
 14 looming ever closer, the time is now to start getting something done on
 this.

>>> ...
>>> -- Team repos
>>>
>>> I'd recommend just using your own account on github or whatever.
>>>
>>> ...
>>>
>>> -- Process Recommendation
>>>
>>> I discussed this a good bit above, but I'm happy to answer questions.
>>>
>>> --
>>> Russell Bryant
>>>
>>
>> Russell,
>>
>> how does Gerrit deal with submitting reviews? Are all reviews simply
>> topic branches on the repository that Gerrit hosts?
>>
>
> Perhaps a real demonstration of workflow would help.  I'll use a recent
> trivial fix that I did.  This is a one-liner patch that needed to go into
> master as well as two stable branches.
>
> I headed over to my local git tree and created a branch to do the fix.
>
> $ cd openstack/nova
> $ git checkout -b bug/1370191 origin/master
>
> ... Hack code and commit the fix ...
>
> $ git commit -a
>
> Now I have a single patch on top of upstream master that I want to submit
> for review.
>
> $ git review
>
> This created https://review.openstack.org/#/c/121940/
>
> What's actually happening is a git push to gerrit.  A git repo in gerrit
> maintains all revisions of all patches.  You can actually fetch the patch
> and look at it locally in your tree.
>
>
A couple more comments about the magic happening here ...

First, "git review" knows where to push based on a file checked in to the
repo:

 $ cat .gitreview
[gerrit]
host=review.openstack.org
port=29418
project=openstack/nova.git

"git review" also sets up a local commit hook that adds a "Change-Id"
header to your commit message.  That Change-Id is what links multiple
revisions of the same change together.  So, if you edit your change and
push it again, as long as the Change-Id remains the same, gerrit treats it
as the same review request and not a new one.

-- 
Russell Bryant
-- 
_
-- 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Matt Jordan

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

Ship it!


Ship It!

- Matt Jordan


On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 1:25 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] Git Migration

2014-09-18 Thread Samuel Galarneau
>
> A couple more comments about the magic happening here ...
>
> First, "git review" knows where to push based on a file checked in to the
> repo:
>
>  $ cat .gitreview
> [gerrit]
> host=review.openstack.org
> port=29418
> project=openstack/nova.git
>
> "git review" also sets up a local commit hook that adds a "Change-Id"
> header to your commit message.  That Change-Id is what links multiple
> revisions of the same change together.  So, if you edit your change and
> push it again, as long as the Change-Id remains the same, gerrit treats it
> as the same review request and not a new one.
>

Sounds good to me. So it doesn't really matter from which repo you post a
review so long as it's a clone of the original with that .gitreview file.

I have another question unrelated to reviews. Does your setup make it easy
to mirror a repo? In a more complicated scenario, what if someone had a
private fork but they wanted to get public commits to master mirrored to
their repo? Would they have to treat the original repo as upstream and
manually pull changes and rebase their private branch off of it?


-- 

Samuel Fortier-Galarneau
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at:  www.digium.com  & www.asterisk.org
-- 
_
-- 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] Asterisk 11.6-cert6, 11.12.1, 12.5.1 Now Available (Security Release)

2014-09-18 Thread Asterisk Development Team
The Asterisk Development Team has announced security releases for Certified
Asterisk 11.6 and Asterisk 11 and 12. The available security releases are
released as versions 11.6-cert6, 11.12.1, and 12.5.1.

These releases are available for immediate download at
http://downloads.asterisk.org/pub/telephony/asterisk/releases

Please note that the release of these versions resolves the following security
vulnerability:

* AST-2014-010: Remote Crash when Handling Out of Call Message in Certain
Dialplan Configurations

Additionally, the release of Asterisk 12.5.1 resolves the following security
vulnerability:

* AST-2014-009: Remote Crash Based on Malformed SIP Subscription Requests 

Note that the crash described in AST-2014-010 can be worked around through
dialplan configuration. Given the likelihood of the issue, an advisory was
deemed to be warranted.

For more information about the details of these vulnerabilities, please read
security advisories AST-2014-009 and AST-2014-010, which were released at the
same time as this announcement.

For a full list of changes in the current releases, please see the ChangeLogs:

http://downloads.asterisk.org/pub/telephony/certified-asterisk/releases/ChangeLog-11.6-cert6
http://downloads.asterisk.org/pub/telephony/asterisk/releases/ChangeLog-11.12.1
http://downloads.asterisk.org/pub/telephony/asterisk/releases/ChangeLog-12.5.1

The security advisories are available at:

 * http://downloads.asterisk.org/pub/security/AST-2014-009.pdf
 * http://downloads.asterisk.org/pub/security/AST-2014-010.pdf

Thank you for your continued support of Asterisk!


-- 
_
-- 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] AST-2014-009: Remote crash based on malformed SIP subscription requests

2014-09-18 Thread Asterisk Security Team
   Asterisk Project Security Advisory - AST-2014-009

 ProductAsterisk  
 SummaryRemote crash based on malformed SIP subscription  
requests  
Nature of Advisory  Remotely triggered crash of Asterisk  
  SusceptibilityRemote authenticated sessions 
 Severity   Major 
  Exploits KnownNo
   Reported On  30 July, 2014 
   Reported By  Mark Michelson
Posted On   18 September, 2014
 Last Updated OnSeptember 18, 2014
 Advisory Contact   Mark Michelson  
 CVE Name   Pending   

Description  It is possible to trigger a crash in Asterisk by sending a   
 SIP SUBSCRIBE request with unexpected mixes of headers for   
 a given event package. The crash occurs because Asterisk 
 allocates data of one type at one layer and then interprets  
 the data as a separate type at a different layer. The crash  
 requires that the SUBSCRIBE be sent from a configured
 endpoint, and the SUBSCRIBE must pass any authentication 
 that has been configured.
  
 Note that this crash is Asterisk's PJSIP-based   
 res_pjsip_pubsub module and not in the old chan_sip module.  

Resolution  Type-safety has been built into the pubsub API where it   
previously was absent. A test has been added to the   
testsuite that previously would have triggered the crash. 

   Affected Versions  
Product   Release  
  Series   
  Asterisk Open Source 1.8.x   Unaffected 
  Asterisk Open Source 11.xUnaffected 
  Asterisk Open Source 12.x12.1.0 and up  
   Certified Asterisk 1.8.15   Unaffected 
   Certified Asterisk  11.6Unaffected 

  Corrected In 
 Product  Release 
  Asterisk Open Source12.5.1  

Patches  
SVN URL  Revision 
   http://downloads.asterisk.org/pub/security/AST-2014-009-12.diff   Asterisk 
 12   

Links  https://issues.asterisk.org/jira/browse/ASTERISK-24136 

Asterisk Project Security Advisories are posted at
http://www.asterisk.org/security  
  
This document may be superseded by later versions; if so, the latest  
version will be posted at 
http://downloads.digium.com/pub/security/AST-2014-009.pdf and 
http://downloads.digium.com/pub/security/AST-2014-009.html

Revision History
 DateEditor  Revisions Made   
19 August, 2014  Mark Michelson  Initial version of document  

   Asterisk Project Security Advisory - AST-2014-009
  Copyright (c) 2014 Digium, Inc. All Rights Reserved.
  Permission is hereby granted to distribute and publish this advisory in its
   original, unaltered form.


-- 
_
-- 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] AST-2014-010: Remote crash when handling out of call message in certain dialplan configurations

2014-09-18 Thread Asterisk Security Team
   Asterisk Project Security Advisory - AST-2014-010

 ProductAsterisk  
 SummaryRemote crash when handling out of call message in 
certain dialplan configurations   
Nature of Advisory  Remotely triggered crash of Asterisk  
  SusceptibilityRemote authenticated sessions 
 Severity   Minor 
  Exploits KnownNo
   Reported On  05 September 2014 
   Reported By  Philippe Lindheimer   
Posted On   18 September 2014 
 Last Updated OnSeptember 18, 2014
 Advisory Contact   Matt Jordan
 CVE Name   Pending   

Description  When an out of call message - delivered by either the SIP
 or PJSIP channel driver or the XMPP stack - is handled in
 Asterisk, a crash can occur if the channel servicing the 
 message is sent into the ReceiveFax dialplan application 
 while using the res_fax_spandsp module.  
  
 Note that this crash does not occur when using the   
 res_fax_digium module.   
  
 While this crash technically occurs due to a configuration   
 issue, as attempting to receive a fax from a channel driver  
 that only contains textual information will never succeed,   
 the likelihood of having it occur is sufficiently high as
 to warrant this advisory.

Resolution  The fax family of applications have been updated to handle
the Message channel driver correctly. Users using the fax 
family of applications along with the out of call text
messaging features are encouraged to upgrade their versions   
of Asterisk to the versions specified in this security
advisory. 
  
Additionally, users of Asterisk are encouraged to use a   
separate dialplan context to process text messages. This  
avoids issues where the Message channel driver is passed to   
dialplan applications that assume a media stream is   
available. Note that the various channel drivers and stacks   
provide such an option; an example being the SIP channel  
driver's outofcall_message_context option.

   Affected Versions   
 Product   Release  
   Series   
  Asterisk Open Source  11.xAll versions  
  Asterisk Open Source  12.xAll versions  
   Certified Asterisk   11.6All versions  

  Corrected In   
Product  Release  
 Asterisk Open Source11.12.1, 12.5.1  
  Certified Asterisk   11.6-cert6 

 Patches 
SVN URL  Revision  
   http://downloads.asterisk.org/pub/security/AST-2014-010-11.diff   Asterisk  
 11
   http://downloads.asterisk.org/pub/security/AST-2014-010-12.diff   Asterisk  
 12
   http://downloads.asterisk.org/pub/security/AST-2014-010-11.6.diff Certified 
 Asterisk  
 11.6  

Links  https://issues.asterisk.org/jira/browse/ASTERISK-24301 

Asterisk Project Security Advisories are posted at
http://www.asterisk.org/security  
 

Re: [asterisk-dev] [Code Review] 3989: utils: Create ast_strsep function that ignores separators inside quotes.

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 2:21 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and rmudgett.


Changes
---

Committed in revision 423476


Repository: Asterisk


Description
---

I'm going to need this for my imminent manager and config
enhancements but I thought I'd post this separately.

/*!
  \brief Act like strsep but ignore separators inside quotes.
  \param s Pointer to address of the the string to be processed.
  Will be modified and can't be constant.
  \param sep A single character delimiter.
  \param flags Controls post-processing of the result.
  AST_STRSEP_TRIM trims all leading and trailing whitespace from the result.
  AST_STRSEP_STRIP does a trim then strips the outermost quotes.  You may want
  to trim again after the strip.  Just OR both the TRIM and STRIP flags.
  AST_STRSEP_UNESCAPE unescapes '\' sequences.
  AST_STRSEP_ALL does all of the above processing.
  \return The next token or NULL if done or if there are more than 8 levels of
  nested quotes.

  This function acts like strsep with three exceptions...
  The separator is a single character instead of a string.
  Separators inside quotes are treated literally instead of like separators.
  You can elect to have leading and trailing whitespace and quotes
  stripped from the result and have '\' sequences unescaped.

  Like strsep, ast_strsep maintains no internal state and you can call it
  recursively using different separators on the same storage.

  Also like strsep, for consistent results, consecutive separators are not
  collapsed so you may get an empty string as a valid result.

  Examples:
  \code
char *mystr = ast_strdupa("abc=def,ghi='zzz=yyy,456',jkl");
char *token, *token2, *token3;

while((token = ast_strsep(&mystr, ',', AST_SEP_STRIP))) {
// 1st token will be aaa=def
// 2nd token will be ghi='zzz=yyy,456'
while((token2 = ast_strsep(&token, '=', AST_SEP_STRIP))) {
// 1st token2 will be ghi
// 2nd token2 will be zzz=yyy,456
while((token3 = ast_strsep(&token2, ',', 
AST_SEP_STRIP))) {
// 1st token3 will be zzz=yyy
// 2nd token3 will be 456
// and so on
}
}
// 3rd token will be jkl
}

  \endcode
 */
char *ast_strsep(char **s, const char sep, uint32_t flags);


Diffs
-

  branches/12/tests/test_strings.c 422963 
  branches/12/main/utils.c 422963 
  branches/12/include/asterisk/strings.h 422963 

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


Testing
---


Thanks,

George Joseph

-- 
_
-- 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] 3988: res_pjsip: Don't require a password when doing userpass authentication

2014-09-18 Thread Sean Bright

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

(Updated Sept. 18, 2014, 2:29 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 423481


Repository: Asterisk


Description
---

An empty password is valid for username/password authentication so we shouldn't 
barf on it.


Diffs
-

  /branches/12/res/res_pjsip/config_auth.c 422963 

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


Testing
---

Compiles.  Registered a device with auth_type=userpass and no password set.  
Tested registration with a password which failed, and again without a password 
(an empty password) and it succeeds.


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] 3966: Testsuite: RLS batched notification tests

2014-09-18 Thread Mark Michelson

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

(Updated Sept. 18, 2014, 2:31 p.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers.


Changes
---

Committed in revision 5603


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


Repository: testsuite


Description
---

This is a suite of tests for resource list subscriptions with batched 
notification. Batched notification is a process by which a time interval may be 
configured on a resource list. When a state change occurs for one of the 
resources in the list, a timer is started. While the timer is running, any 
further state changes to any of the resources in the list are cached. Once the 
timer has expired, all state changes that have occurred for resources in the 
list are sent in a single NOTIFY. This set of tests means to ensure that the 
feature works as expected. The tests are as follows:

* basic: A baseline test, this creates a subscription to a resource list with 
two resources. A change is made to one of the resources, and we ensure that the 
notification of the state change occurs after the configured interval. This 
test also includes a resubscription and termination, ensuring that the NOTIFYs 
that we send in response to SUBSCRIBEs are sent immediately and do not use the 
configured interval.
* single_resource_multiple_changes: In this test, a single resource has two 
rapid state changes made on it. We ensure that even though we have changed the 
state twice, we only receive a single NOTIFY from Asterisk with the most recent 
state change.
* multiple_resources_single_change: In this test, two resources each have a 
single state change made on them. We ensure that even though we have changed 
two different resources' states, we receive only a single NOTIFY from Asterisk 
containing both resources' state changes.
* resubscription_interruption: In this test, we make a state change on a 
resource, and then immediately resubscribe to the list. Since we must send a 
NOTIFY immediately in response to the SUBSCRIBE, and since this NOTIFY will 
reflect the latest state of the resource, we test that the batched notification 
that we created when the state change occurred has been canceled. We do this by 
waiting for several seconds after the conclusion of the test to be certain that 
Asterisk does not send any errant NOTIFY requests.
* termination_interruption: This test is nearly identical to 
resubscription_interruption, except that instead of sending a resubscription to 
interrupt the batched notification, we send a subscription termination. Like 
with resubscription_interruption, we wait around after the scenario has 
completed in case Asterisk sends any further NOTIFY requests.
* list_of_lists/batched: This is the only test for lists containing sublists. 
In this scenario, the outer list has batched notifications disabled, but the 
inner sublist has batched notifications enabled. The test ensures that the 
configuration for notification batching on the inner list is ignored and that 
notifications are not batched at all. We do this by sending two rapid state 
changes and ensuring that each results in a NOTIFY request being sent by 
Asterisk.

In order to get these tests to work properly, I made a couple of changes to 
rls_test.py:
* I added a "stop_after_notifys" option, True by default. For the 
resubscription_interruption and termination_interruption tests, I needed 
rls_test.py not to stop the test after the final expected NOTIFY was received. 
This option allows those tests to keep the test alive until they deem it okay 
to stop the test.
* I made the call to register_scenario_started_callback() conditional. I did 
this initially because I was not going to use the SIPpTestCase for 
resubscription_interruption and termination_interruption. When not using the 
SIPpTestCase, an exception is thrown when trying to call 
register_scenario_started_callback() since it does not exist. In the end, I did 
end up using the SIPpTestCase for those tests, but I decided to leave the 
change to rls_test.py intact since it shouldn't necessarily presume that a 
SIPpTestCase is being used.

There are two versions of each of the tests, one for presence and one for MWI. 
For the most part, the tests are identical except for the obvious bits 
(Updating MWI instead of device state in test drivers, configuring mailboxes 
instead of hints, etc.). However, the single_resource_multiple_state_changes 
test is a bit different between the presence and MWI versions. This is because 
while testing with presence, I discovered issue 
https://issues.asterisk.org/jira/browse/ASTERISK-24286 . For presence tests, I 
could not reliably send two rapid state changes and expect consistent results. 
So instead, I have to se

Re: [asterisk-dev] [Code Review] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett

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



/branches/12/main/stasis_channels.c


Why is the caller channel lock held for the 
ast_channel_publish_dial_forward() call?

A dead lock can happen holding the caller lock because 
ast_channel_publish_dial_forward() locks caller and peer in turn.



/branches/12/main/stasis_channels.c


Maybe this should be ast_strlen_zero(dialstatus) instead.  There are 
several calls in app_queue() that pass something for both dialstring and 
dialstatus.



/branches/12/main/stasis_channels.c


dial_masquerade() is called with several locks held: the global channels 
lock, old_chan, and new_chan.

The calls to ast_channel_publish_dial_forward() will then try to lock 
cur->peer.

I'm just noting this situation because locking more than one channel at a 
time normally has a deadlock potential.  However, since the global channels 
container lock is held it should be safe enough.



/branches/12/main/stasis_channels.c


target is leaked here.



/branches/12/main/stasis_channels.c


Allocation failure is not checked here.  Though it seems like it might not 
cause a problem anyway.


- rmudgett


On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 1:25 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need 

Re: [asterisk-dev] [Code Review] 3992: res_pjsip_sdp_rtp: Add optimistic SRTP support.

2014-09-18 Thread Mark Michelson


> On Sept. 15, 2014, 4:14 p.m., opticron wrote:
> > This change appears to render the media_use_received_transport 
> > configuration option non-functional since it removes all checks relating to 
> > it.
> 
> Joshua Colp wrote:
> It'll still work, the difference is on received it is always done and the 
> outgoing check has been relaxed to not rely on the option.

I don't really understand your response here, Josh. As opticron stated, all 
references to the option media_use_received_transport have been removed, 
therefore the option has no effect any longer.


- Mark


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


On Sept. 14, 2014, 12:25 a.m., Joshua Colp wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3992/
> ---
> 
> (Updated Sept. 14, 2014, 12:25 a.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> When enabling SRTP support in PJSIP it is either forced on or disabled. This 
> means that if you specify SRTP but the client does not support it the session 
> will fail. For situations where this guarantee is not required this new 
> functionality can be used to optimistically use SRTP if possible. This has 
> the added benefit of encrypting the media when possible but does not 
> guarantee it. This also fixes an issue where a client may offer SRTP using 
> the normal transport but we reject it.
> 
> 
> Diffs
> -
> 
>   /trunk/res/res_pjsip_sdp_rtp.c 423064 
>   /trunk/res/res_pjsip/pjsip_configuration.c 423064 
>   /trunk/res/res_pjsip.c 423064 
>   /trunk/include/asterisk/res_pjsip.h 423064 
>   /trunk/configs/samples/pjsip.conf.sample 423064 
> 
> Diff: https://reviewboard.asterisk.org/r/3992/diff/
> 
> 
> Testing
> ---
> 
> Used Blink to place calls with optimistic enabled and disabled on the PJSIP 
> side.
> In Blink I alternated between disabled/mandatory/optional.
> Confirmed that for each scenario the expected outcome occurred.
> 
> Blink  Asterisk   Result
> Disabled   Optimistic Off Failed
> Disabled   Optimistic On  Success (Not encrypted)
> Mandatory  Optimistic Off Success (Encrypted)
> Mandatory  Optimistic On  Success (Encrypted)
> Optional   Optimistic Off Success (Encrypted)
> Optional   Optimistic On  Success (Encrypted)
> 
> 
> 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] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 2:42 p.m.)


Review request for Asterisk Developers.


Changes
---

Updated summary


Repository: Asterisk


Description (updated)
---

The big piece missing for me to finally transition to pjsip was the ability to 
mirror the auto provisioning features of res_phoneprov.  The first step (this 
patch) is to make res_phoneprov more modular so other modules (like pjsip) can 
provide configuration information instead of res_phoneprov relying solely on 
users.conf and sip.conf.  To accomplish this a new ast_phoneprov public API is 
now exposed which allows config providers to register themselves, set defaults 
(server profile, etc) and add user extensions.

ast_phoneprov_provider_register registers the provider and provides callbacks 
for loading default settings and loading users.
ast_phoneprov_provider_unregister clears the defaults and users.
ast_phoneprov_add_extension should be called once for each user/extension by 
the provider's load_users callback to add them.
ast_phoneprov_delete_extension deletes one extension.
ast_phoneprov_delete_extensions deletes all extensions for the provider.

res_phoneprov actually registers itself as the provider for sip/users and is 
always available and is the default.

Writing a new provider...
Since res_phoneprov is also it's own provider, examples of what a new provider 
would have to do are in load_users() in res_phoneprov.c.  Those functions 
gather the information from users.conf and sip.conf and call the 
ast_provider_register and ast_phoneprov_add_extension apis.

So...
The provider creates a callback function which calls the 
ast_phoneprov_add_extension api for each user.  
It then calls ast_phoneprov_provider_register with the callback.
res_phoneprov then calls the callback to cause the actual load.
During normal http server ops, all work is done by res_phoneprov and the 
provider is never called again unless a reload is needed.
If the provider wants to reload it can simply unregister and reregister or it 
can call its own load_users callback.
If res_phoneprov wants to reload, it iterates over its registry and calls the 
providers callback.

NOTE:  If res_phoneprov is actually unloaded, it has no way to know what 
providers were registered (other than itself) so a subsequent load will have 
nothing but it's own users.  

Additional changes...
I added a few convenience functions to chanvars for creating lists and finding 
and deleting entries.  No existing code was touched.

Next steps...
A provider for res_pjsip.


Diffs
-

  branches/12/res/res_phoneprov.exports.in PRE-CREATION 
  branches/12/res/res_phoneprov.c 422737 
  branches/12/main/chanvars.c 422737 
  branches/12/include/asterisk/phoneprov.h PRE-CREATION 
  branches/12/include/asterisk/chanvars.h 422737 
  branches/12/configs/phoneprov.conf.sample 422737 

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


Testing
---

I ran through several scenarios including the use of PP_EACH_USER and 
PP_EACH_EXTENSION to make sure that all existing functionality was preserved.  
I actually use it with Grandstream phones and everything worked exactly as 
expected.


Thanks,

George Joseph

-- 
_
-- 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose


> On Sept. 18, 2014, 3:10 p.m., rmudgett wrote:
> > /branches/12/main/stasis_channels.c, line 1257
> > 
> >
> > dial_masquerade() is called with several locks held: the global 
> > channels lock, old_chan, and new_chan.
> > 
> > The calls to ast_channel_publish_dial_forward() will then try to lock 
> > cur->peer.
> > 
> > I'm just noting this situation because locking more than one channel at 
> > a time normally has a deadlock potential.  However, since the global 
> > channels container lock is held it should be safe enough.


> On Sept. 18, 2014, 3:10 p.m., rmudgett wrote:
> > /branches/12/main/stasis_channels.c, lines 381-394
> > 
> >
> > Why is the caller channel lock held for the 
> > ast_channel_publish_dial_forward() call?
> > 
> > A dead lock can happen holding the caller lock because 
> > ast_channel_publish_dial_forward() locks caller and peer in turn.

Well, I want to keep the lock held in case a masquerade happens before I do 
ast_channel_publish_dial_forward... not sure how realistic such a scenario is, 
probably not very, but perhaps I should just do a lock_both here instead?


- Jonathan


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


On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 1:25 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior

Re: [asterisk-dev] Git Migration

2014-09-18 Thread Russell Bryant
On Thu, Sep 18, 2014 at 3:01 PM, Samuel Galarneau 
wrote:

>
>> A couple more comments about the magic happening here ...
>>
>> First, "git review" knows where to push based on a file checked in to the
>> repo:
>>
>>  $ cat .gitreview
>> [gerrit]
>> host=review.openstack.org
>> port=29418
>> project=openstack/nova.git
>>
>> "git review" also sets up a local commit hook that adds a "Change-Id"
>> header to your commit message.  That Change-Id is what links multiple
>> revisions of the same change together.  So, if you edit your change and
>> push it again, as long as the Change-Id remains the same, gerrit treats it
>> as the same review request and not a new one.
>>
>
> Sounds good to me. So it doesn't really matter from which repo you post a
> review so long as it's a clone of the original with that .gitreview file.
>
> I have another question unrelated to reviews. Does your setup make it easy
> to mirror a repo? In a more complicated scenario, what if someone had a
> private fork but they wanted to get public commits to master mirrored to
> their repo? Would they have to treat the original repo as upstream and
> manually pull changes and rebase their private branch off of it?
>

Mirroring, sure.  I don't remember exactly how we do it, but all OpenStack
repos are mirrored to github for convenience, for example.

Regarding private branches, git generally makes that kind of thing
***MUCH*** easier than svn.  You can easily track the exact commits that
are applied on top of upstream.  You could either periodically rebase your
changes on top of upstream (re-applying the commits, rewriting history but
a cleaner history), or periodically merge from upstream.  In either case, I
personally wouldn't automate it.  You want some sanity checking around that
stuff.  Conflicts happen.  Really, I think this is going to be MUCH better
no matter what specific infrastructure you go with.

-- 
Russell Bryant
-- 
_
-- 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett


> On Sept. 18, 2014, 3:10 p.m., rmudgett wrote:
> > /branches/12/main/stasis_channels.c, lines 381-394
> > 
> >
> > Why is the caller channel lock held for the 
> > ast_channel_publish_dial_forward() call?
> > 
> > A dead lock can happen holding the caller lock because 
> > ast_channel_publish_dial_forward() locks caller and peer in turn.
> 
> Jonathan Rose wrote:
> Well, I want to keep the lock held in case a masquerade happens before I 
> do ast_channel_publish_dial_forward... not sure how realistic such a scenario 
> is, probably not very, but perhaps I should just do a lock_both here instead?

Masquerades are pure evil as far as stasis messages that are part of a series 
is concerned.  Surprise!  I'm a different channel now.
Dial Start/Stop
Bridge Enter/Leave

Probably have to lock both channels.  Note that you may or may not have caller, 
but you will always have peer.


- rmudgett


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


On Sept. 18, 2014, 1:25 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 1:25 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> Thanks,
> 
> Jonathan Rose
> 
>

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

asterisk-dev mailing list
To UNSUBSCRIBE or update options visit:
   http:/

[asterisk-dev] [Code Review] 4001: PJSIP: Prevent T38 framehook being put on wrong channel

2014-09-18 Thread opticron

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

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

This changes gives framehooks a reverse-direction masquerade callback in 
addition to chan_fixup_cb similar to the callback added to datastores to handle 
the same situation. The new callback provides the same parameters as the fixup 
callback, but is called on the new channel's framehooks before moving 
framehooks from the old channel to the new channel. This gives the framehooks 
an oppurtunity to decide whether they should remain on the new channel or be 
removed.

This new callback is used to prevent the PJSIP T.38 framehook from remaining on 
a masqueraded channel if the new channel is not also a PJSIP channel. This was 
causing a crash when a local channel was masqueraded into a PJSIP channel and 
the framehook was executed on the local channel since the channel's tech 
private data was not structured as expected.


Diffs
-

  branches/12/res/res_pjsip_t38.c 423230 
  branches/12/main/framehook.c 423230 
  branches/12/include/asterisk/framehook.h 423230 

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


Testing
---

This corrected my reproduction of the crash and fixed the crash for the 
original reporter.


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] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 3:21 p.m.)


Review request for Asterisk Developers.


Changes
---

Updated description and cleaned up default processing.


Repository: Asterisk


Description (updated)
---

This module allows res_pjsip to integrate with res_phoneprov and depends on the 
res_phoneprov refactor (r3970).

A new pjsip.conf object is defined by this module...

;EXAMPLE PHONEPROV CONFIGURATION

; Before configuring provisioning here, see the documentation for res_phoneprov
; and configure phoneprov.conf appropriately.

; For each user to be autoprovisioned, a [phoneprov] configuration section
; must be created.  At a minimum, the 'type', 'PROFILE' and 'MAC' variables must
; be set.  All other variables are optional.
; Example:

;[1000]
;type=phoneprov   ; must be specified as 'phoneprov'
;endpoint=1000; Required only if automatic setting of
  ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
  ; are needed.
;PROFILE=digium   ; required
;MAC=deadbeef4dad ; required
;SERVER=myserver.example.com  ; A standard variable
;TIMEZONE=America/Denver  ; A standard variable
;MYVAR=somevalue  ; A user confdigured variable

; If the phoneprov sections have common variables, it is best to create a
; phoneprov template.  The example below will produce the same configuration
; as the one specified above except that MYVAR will be overridden for
; the specific user.
; Example:

;[phoneprov_defaults](!)
;type=phoneprov   ; must be specified as 'phoneprov'
;PROFILE=digium   ; required
;SERVER=myserver.example.com  ; A standard variable
;TIMEZONE=America/Denver  ; A standard variable
;MYVAR=somevalue  ; A user configured variable

;[1000](phoneprov_defaults)
;endpoint=1000; Required only if automatic setting of
  ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
  ; are needed.
;MAC=deadbeef4dad ; required
;MYVAR=someOTHERvalue ; A user confdigured variable

; To have USERNAME and SECRET automatically set, the endpoint
; specified here must in turn have an outbound_auth section defined.

; Fuller example:

;[1000]
;type=endpoint
;outbound_auth=1000-auth
;callerid=My Name <8005551212>
;transport=transport-udp-nat

;[1000-auth]
;type=auth
;auth_type=userpass
;username=myname
;password=mysecret

;[phoneprov_defaults](!)
;type=phoneprov   ; must be specified as 'phoneprov'
;PROFILE=someprofile  ; required
;SERVER=myserver.example.com  ; A standard variable
;TIMEZONE=America/Denver  ; A standard variable
;MYVAR=somevalue  ; A user configured variable

;[1000](phoneprov_defaults)
;endpoint=1000; Required only if automatic setting of
  ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
  ; are needed.
;MAC=deadbeef4dad ; required
;MYVAR=someUSERvalue  ; A user confdigured variable
;LABEL=1000   ; A standard variable
 
; The previous sections would produce a template substitution map as follows:

;MAC=deadbeef4dad   ;added by pp1000
;USERNAME=myname;automatically added by 1000-auth username
;SECRET=mysecret;automatically added by 1000-auth password
;PROFILE=someprofile;added by defaults
;SERVER=myserver.example.com;added by defaults
;SERVER_PORT=5060   ;added by defaults
;MYVAR=someUSERvalue;added by defaults but overdidden by user
;CALLERID=8005551212;automatically added by 1000 callerid
;DISPLAY_NAME=My Name   ;automatically added by 1000 callerid
;TIMEZONE=America/Denver;added by defaults
;TZOFFSET=252100;automatically calculated by res_phoneprov
;DST_ENABLE=1   ;automatically calculated by res_phoneprov
;DST_START_MONTH=3  ;automatically calculated by res_phoneprov
;DST_START_MDAY=9   ;automatically calculated by res_phoneprov
;DST_START_HOUR=3   ;automatically calculated by res_phoneprov
;DST_END_MONTH=11   ;automatically calculated by res_phoneprov
;DST_END_MDAY=2 ;automatically calculated by res_phoneprov
;DST_END_HOUR=1 ;automatically calculated by res_phoneprov
;ENDPOINT_ID=1000   ;automatically added by this module
;AUTH_ID=1000-auth  ;automatically added by this module
;TRANSPORT_ID=transport-udp-nat ;automatically added by this module
;LABEL=1000 ;added by user


Diffs (updated)
-

  branches/12/res/res_

Re: [asterisk-dev] [Code Review] 4001: PJSIP: Prevent T38 framehook being put on wrong channel

2014-09-18 Thread Matt Jordan

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

Ship it!


Ship It!

- Matt Jordan


On Sept. 18, 2014, 4:19 p.m., opticron wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4001/
> ---
> 
> (Updated Sept. 18, 2014, 4:19 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This changes gives framehooks a reverse-direction masquerade callback in 
> addition to chan_fixup_cb similar to the callback added to datastores to 
> handle the same situation. The new callback provides the same parameters as 
> the fixup callback, but is called on the new channel's framehooks before 
> moving framehooks from the old channel to the new channel. This gives the 
> framehooks an oppurtunity to decide whether they should remain on the new 
> channel or be removed.
> 
> This new callback is used to prevent the PJSIP T.38 framehook from remaining 
> on a masqueraded channel if the new channel is not also a PJSIP channel. This 
> was causing a crash when a local channel was masqueraded into a PJSIP channel 
> and the framehook was executed on the local channel since the channel's tech 
> private data was not structured as expected.
> 
> 
> Diffs
> -
> 
>   branches/12/res/res_pjsip_t38.c 423230 
>   branches/12/main/framehook.c 423230 
>   branches/12/include/asterisk/framehook.h 423230 
> 
> Diff: https://reviewboard.asterisk.org/r/4001/diff/
> 
> 
> Testing
> ---
> 
> This corrected my reproduction of the crash and fixed the crash for the 
> original reporter.
> 
> 
> 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] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread Mark Michelson

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



/branches/13/res/res_pjsip_sdp_rtp.c


Since joint only has formats of type media_type, would specifying 
media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?


- Mark Michelson


On Sept. 18, 2014, 6:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose

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

(Updated Sept. 18, 2014, 4:48 p.m.)


Review request for Asterisk Developers, Matt Jordan and rmudgett.


Changes
---

Hit rmudgett's findings.


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


Repository: Asterisk


Description
---

Reproduction: 
pj123 calls 1601
pj123 does a SIP blonde transfer to 1603
1603 answers
FRACK occurs
all are PJSIP endpoints.

Basically what happens is there is a second outbound dial from another pj123 
channel. Before the dial is answered, the pj123 gets masqueraded out of the 
picture and replaced with a channel that isn't in the dial state.

This patch fixes that by advancing the incoming channel to the dial state in 
the channel breakdown function of a datastore on the pj123 channel. Honestly, 
I'm not sure this is entirely adequate, but it seems to work in all of the 
conditions I've tried so far and it doesn't impede normal attended transfers. I 
might need to try and figure out what happens if I masquerade in a channel that 
is already dialing though. I'm not sure if that's something we can really 
expect to happen under normal conditions, but that seems like something that 
would screw up this approach.

Note that I think this patch is the right idea, I just don't know if I need to 
account for the possibility that the channel that masquerades into pj123's 
dialing channel might not be in the neutral state.


Diffs (updated)
-

  /branches/12/main/stasis_channels.c 422882 

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


Testing
---

Ran against reproduction of the above scenario. Assertion was gone and the CDR 
results were as follows:

"","123","1601","default"," 
<123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
 21:48:51","2014-09-11 21:48:53","2014-09-11 
21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
"","123","","default"," 
<123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
 21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
ANSWER","DOCUMENTATION","1410472135.6",""
"","1601","1603","default"," 
<1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""

And dial events reported by AMI:
http://pastebin.com/tWuwL7xa
(note that there is a mismatch between the number of dial end and dial begin 
events... not sure if this is a problem, but I might be able to fix it just by 
updating the old chan, not sure what status code to use though).

Ran against normal attended transfer, feature attended transfers, and blind 
transfers with no noticeable effect.

Ran against testsuite blonde transfer tests, some attended transfer tests, some 
blind transfer tests, and all pjsip transfer tests (at least ones that will run 
on my box... a few won't due to sipp version requirements that I really need to 
get around to fixing eventually) for comparison purposes. All passed exhibiting 
the same behavior as before as far as warning messages and such go.


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] 3954: pjsip_options.c: Fix race condition stopping periodic out of dialog OPTIONS request.

2014-09-18 Thread Mark Michelson


> On Sept. 11, 2014, 3:16 p.m., Joshua Colp wrote:
> > /branches/13/res/res_pjsip.c, lines 2488-2501
> > 
> >
> > Looking at the pjproject code why wouldn't the following return values 
> > cover these cases for when the callback is not called:
> > 
> > PJ_EINVAL
> > PJ_EINVALIDOP
> > PJ_ENOMEM
> > 
> > Handling those as special would reduce the memory leak possibility.
> 
> rmudgett wrote:
> Almost any value could be returned because user supplied callbacks could 
> fail with any error code they think is reasonable.  Most of those transport 
> callback routines can return PJ_EINVAL when they sanity check the parameters. 
>  If it was going out over a websocket transport the PJ_ENOMEM value could be 
> returned.  The safeset thing that can be done is to not free anything.

I'm going to suggest something that I think should ensure that memory gets 
freed. The issue is that the callback may or may not be called when an error 
occurs. Similarly, the callback may be called synchronously or asynchronously.

By process of elimination, if the callback is called asynchronously, then that 
means that the initial call to pjsip_endpt_send_request() MUST have returned 
success. There's no way for pjsip_endpt_send_request() to return an error if 
the error happens at a separate time. Any in the async case, the callback 
SHOULD be called every single time.

The problem can be narrowed down to the case of when the callback is called 
synchronously when an error occurs. Since the callback is called synchronously, 
it can be made possible to detect if the callback has been called when 
pjsip_endpt_send_request() returns. There's only one piece of data that is 
common to both send_out_of_dialog_request() and send_request_cb(), and that's 
the token. In send_out_of_dialog_request(), you can wrap token inside another 
structure that has a flag that indicates if the callback has been called or not.

With all this information at hand, you do the following:
* In send_request_cb, always free the data pased in, and be sure to mark the 
token's wrapper so that it indicates that the callback was called.
* In send_out_of_dialog_request(), if pjsip_endpt_send_request() returns an 
error, then check the token wrapper to see if the callback was called. If so, 
do nothing. If not, then free the data. In both cases, you can return an error, 
and the caller of send_out_of_dialog_request() can safely free their token.


- Mark


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


On Sept. 10, 2014, 10:18 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3954/
> ---
> 
> (Updated Sept. 10, 2014, 10:18 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-155 and ASTERISK-24295
> https://issues.asterisk.org/jira/browse/AFS-155
> https://issues.asterisk.org/jira/browse/ASTERISK-24295
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The crash on the issues is a result of an invalid transport configuration 
> change when asterisk is restarted.  The attempt to send the qualify request 
> fails and we cleaned up.  However, the callback is also called which results 
> in a double unref of the objects involved.
> 
> * Fixed send_request_cb() and qualify_contact_cb() to not cleanup the token 
> when the PJSIP event is PJSIP_EVENT_TRANSPORT_ERROR since the initial 
> function call will do the clean up.
> 
> * Made send_request_cb() able to handle repeated challenges (Up to 10).
> 
> * Fix periodic endpoint qualify OPTIONS sched deletion race by avoiding it.  
> The sched entry will no longer self stop and must be externally stopped.
> 
> * Added REF_DEBUG description tags to struct sched_data in pjsip_options.c.
> 
> * Fix some off-nominal ref leaks in schedule_qualify(), 
> qualify_and_schedule().
> 
> * Reordered pjsip_options.c module start/stop code to cleanup better on error.
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip/pjsip_options.c 422963 
>   /branches/13/res/res_pjsip.c 422963 
> 
> Diff: https://reviewboard.asterisk.org/r/3954/diff/
> 
> 
> Testing
> ---
> 
> * With the qualify_frequency option enabled, added and removed a "local_net=" 
> line in the transport section and restarted asterisk via "core restart now".  
> Before the latest patch version, asterisk would crash.  With the new patch, 
> it keeps on going.
> 
> * Set the qualify_frequency option to different values and reloaded res_pjsip 
> each time.  The OPTIONS poll frequency changed, started, and stopped 
> according to the new qualify_frequency

Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-18 Thread rmudgett

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


A once over pass thourgh of the patch.

You should go through and fixup any coding guidelines problems in the changes 
you have made.


branches/12/include/asterisk/chanvars.h


Protect these macros with the "do {} while (0)" trick.

I'm not sure of the usefulness of these AST_VAR_LIST_xxx macros.  You are 
simply hiding the if test which is for the most part already in the code.



branches/12/include/asterisk/phoneprov.h


You should extern this declaration in the header and initialize it in a c 
file.  As it is, every file that includes this header has an initialized copy.



branches/12/main/chanvars.c


Rather than a relatively expensive safe traversal do this:

while ((var = AST_LIST_REMOVE_HEAD()) {
  ast_var_delete(var);
}



branches/12/res/res_phoneprov.c


This is an interesting way of doing the standard ao2 container callback 
functions.

Did you fix the formatting from the wiki page.  Directly cutting and 
pasting the template function from the wiki gives you spaces instead of tabs.



branches/12/res/res_phoneprov.c


curly on its own line



branches/12/res/res_phoneprov.c


No need for the backslash on this line as it ends the macro.



branches/12/res/res_phoneprov.c


idem



branches/12/res/res_phoneprov.c


if test is redundant because you defined AST_VAR_LIST_INSERT_TAIL to have 
the if test inside it.

The next 7 changes are the same as here.

I looks like just about everywhere you used the new macros that there is 
now a redundant if test.



branches/12/res/res_phoneprov.c


curlies



branches/12/res/res_phoneprov.c


Is the cast necessary?



branches/12/res/res_phoneprov.c


privider is ref leaked

Use:
for (; (provider = ao2_it_next()); ao2_ref(provider, -1))


- rmudgett


On Sept. 18, 2014, 3:42 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3970/
> ---
> 
> (Updated Sept. 18, 2014, 3:42 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The big piece missing for me to finally transition to pjsip was the ability 
> to mirror the auto provisioning features of res_phoneprov.  The first step 
> (this patch) is to make res_phoneprov more modular so other modules (like 
> pjsip) can provide configuration information instead of res_phoneprov relying 
> solely on users.conf and sip.conf.  To accomplish this a new ast_phoneprov 
> public API is now exposed which allows config providers to register 
> themselves, set defaults (server profile, etc) and add user extensions.
> 
> ast_phoneprov_provider_register registers the provider and provides callbacks 
> for loading default settings and loading users.
> ast_phoneprov_provider_unregister clears the defaults and users.
> ast_phoneprov_add_extension should be called once for each user/extension by 
> the provider's load_users callback to add them.
> ast_phoneprov_delete_extension deletes one extension.
> ast_phoneprov_delete_extensions deletes all extensions for the provider.
> 
> res_phoneprov actually registers itself as the provider for sip/users and is 
> always available and is the default.
> 
> Writing a new provider...
> Since res_phoneprov is also it's own provider, examples of what a new 
> provider would have to do are in load_users() in res_phoneprov.c.  Those 
> functions gather the information from users.conf and sip.conf and call the 
> ast_provider_register and ast_phoneprov_add_extension apis.
> 
> So...
> The provider creates a callback function which calls the 
> ast_phoneprov_add_extension api for each user.  
> It then calls ast_phoneprov_provider_register with the callback.
> res_phoneprov then calls the callback to cause the actual load.
> During normal http server ops, all work is done by res_phoneprov and the 
> provider is never called again unless a reload is needed.
> If the provider wants to reload it can simply unregist

Re: [asterisk-dev] [Code Review] 3980: cel_odbc: Add microseconds precision in the eventtime column

2014-09-18 Thread Mark Michelson

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

Ship it!


Ship It!

- Mark Michelson


On Sept. 5, 2014, 7:54 p.m., Etienne Lessard wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3980/
> ---
> 
> (Updated Sept. 5, 2014, 7:54 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: ASTERISK-24283
> https://issues.asterisk.org/jira/browse/ASTERISK-24283
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This patch adds microsecond precision when inserting a CEL record into a 
> table with an "eventtime" column of type timestamp, instead of second 
> precision. The documentation (configs/cel_odbc.conf.sample) was already 
> saying that the eventtime column included microseconds precision, but that 
> was not the case.
> 
> Also, without this patch, if you had a table with an "eventtime" column of 
> type varchar, you had millisecond precision. With this patch, you also get 
> microsecond precision in this case.
> 
> 
> Diffs
> -
> 
>   /branches/11/cel/cel_odbc.c 422682 
> 
> Diff: https://reviewboard.asterisk.org/r/3980/diff/
> 
> 
> Testing
> ---
> 
> Tested with postgres 9.1 and mysql 5.5.
> 
> With postgres, with a CEL table with an "eventtime" column of type timestamp, 
> you get microsecond precision. Same for a CEL table with an "eventtime" 
> column of type varchar.
> 
> With mysql, with a CEL table with an "eventtime" column of type timestamp, 
> you still get only second precision, because mysql 5.5 don't store it ( 
> http://dev.mysql.com/doc/refman/5.5/en/fractional-seconds.html ). That said, 
> it's not causing any problem. For a CEL table with an "eventtime" column of 
> type varchar, you do get microsecond precision.
> 
> 
> Thanks,
> 
> Etienne Lessard
> 
>

-- 
_
-- 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-09-18 Thread Mark Michelson

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

(Updated Sept. 18, 2014, 10:32 p.m.)


Status
--

This change has been marked as submitted.


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/trunk/tests/channels/pjsip/subscriptions/tests.yaml 5168 
  /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/tests.yaml 
PRE-CREATION 
  /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/rls_integrity.py 
PRE-CREATION 
  /asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/tests.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/tests.yaml 
PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/termination.py
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/sipp/termination.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/termination_full_state/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/sipp/list_subscribe.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/subscription_establishment/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/sipp/resubscribe.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/resubscribe.py
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/resubscribe_full_state/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/test-config.yaml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/sipp/list_subscribe.xml
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/partial_state.py
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/configs/ast1/pjsip.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/presence/partial_state/configs/ast1/extensions.conf
 PRE-CREATION 
  
/asterisk/trunk/tests/channels/pjsip/subscriptions/rls/lists/nominal/pre

Re: [asterisk-dev] [Code Review] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread rmudgett


> On Sept. 18, 2014, 4:35 p.m., Mark Michelson wrote:
> > /branches/13/res/res_pjsip_sdp_rtp.c, line 259
> > 
> >
> > Since joint only has formats of type media_type, would specifying 
> > media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?

It's a case of six of one and half a dozen of another.  It won't make any 
difference in this case since all formats will be appended anyway.  It's a 
little more efficient to use the constant since the function has to test if it 
isn't UNKNOWN and then test to see if it is the specified type.

I'll leave it as is unless there is a stronger argument for changing it.


- rmudgett


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


On Sept. 18, 2014, 1:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 1:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> 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] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread Mark Michelson


> On Sept. 18, 2014, 9:35 p.m., Mark Michelson wrote:
> > /branches/13/res/res_pjsip_sdp_rtp.c, line 259
> > 
> >
> > Since joint only has formats of type media_type, would specifying 
> > media_type instead of AST_MEDIA_TYPE_UNKNOWN make more sense here?
> 
> rmudgett wrote:
> It's a case of six of one and half a dozen of another.  It won't make any 
> difference in this case since all formats will be appended anyway.  It's a 
> little more efficient to use the constant since the function has to test if 
> it isn't UNKNOWN and then test to see if it is the specified type.
> 
> I'll leave it as is unless there is a stronger argument for changing it.

Okay, that's good enough for me.


- Mark


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


On Sept. 18, 2014, 6:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> 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] 4000: res_pjsip_sdp_rtp.c: Fix native formats containing formats that were not negotiated.

2014-09-18 Thread Mark Michelson

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

Ship it!


Ship It!

- Mark Michelson


On Sept. 18, 2014, 6:26 p.m., rmudgett wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/4000/
> ---
> 
> (Updated Sept. 18, 2014, 6:26 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Bugs: AFS-162
> https://issues.asterisk.org/jira/browse/AFS-162
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Outgoing PJSIP calls can result in non-negotiated formats listed in the 
> channel's native formats if video formats are listed in the endpoint's 
> configuration.  The resulting call could then use a non-negotiated format 
> resulting in one way audio.
> 
> * Simplified the update of session->req_caps in set_caps().  Why do something 
> in five steps when only one is needed?
> 
> 
> Diffs
> -
> 
>   /branches/13/res/res_pjsip_sdp_rtp.c 423446 
>   /branches/13/channels/chan_pjsip.c 423446 
> 
> Diff: https://reviewboard.asterisk.org/r/4000/diff/
> 
> 
> Testing
> ---
> 
> Configured PJSIP endpoints with: allow=!all,h264,g722,h263,ulaw,h263p,alaw
> Called from D40 with g722 among other formats enabled to a Polycom that 
> negotiates ulaw.
> Before the patch, Asterisk would send g722 frames to the Polycom.  The 
> resulting call had one way audio because the Polycom does not understand g722.
> After the patch, Asterisk sends ulaw frames to the Polycom.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett

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



/branches/12/main/stasis_channels.c


Swap setting up target->peer with setting up target->dialstring.  As it is 
you will leak a ref to target->peer if ast_strdup allocaton fails.


- rmudgett


On Sept. 18, 2014, 4:48 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 4:48 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread Jonathan Rose

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

(Updated Sept. 18, 2014, 5:50 p.m.)


Review request for Asterisk Developers, Matt Jordan and rmudgett.


Changes
---

Fix reference leak rmudgett pointed out.


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


Repository: Asterisk


Description
---

Reproduction: 
pj123 calls 1601
pj123 does a SIP blonde transfer to 1603
1603 answers
FRACK occurs
all are PJSIP endpoints.

Basically what happens is there is a second outbound dial from another pj123 
channel. Before the dial is answered, the pj123 gets masqueraded out of the 
picture and replaced with a channel that isn't in the dial state.

This patch fixes that by advancing the incoming channel to the dial state in 
the channel breakdown function of a datastore on the pj123 channel. Honestly, 
I'm not sure this is entirely adequate, but it seems to work in all of the 
conditions I've tried so far and it doesn't impede normal attended transfers. I 
might need to try and figure out what happens if I masquerade in a channel that 
is already dialing though. I'm not sure if that's something we can really 
expect to happen under normal conditions, but that seems like something that 
would screw up this approach.

Note that I think this patch is the right idea, I just don't know if I need to 
account for the possibility that the channel that masquerades into pj123's 
dialing channel might not be in the neutral state.


Diffs (updated)
-

  /branches/12/main/stasis_channels.c 422882 

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


Testing
---

Ran against reproduction of the above scenario. Assertion was gone and the CDR 
results were as follows:

"","123","1601","default"," 
<123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
 21:48:51","2014-09-11 21:48:53","2014-09-11 
21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
"","123","","default"," 
<123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
 21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
ANSWER","DOCUMENTATION","1410472135.6",""
"","1601","1603","default"," 
<1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""

And dial events reported by AMI:
http://pastebin.com/tWuwL7xa
(note that there is a mismatch between the number of dial end and dial begin 
events... not sure if this is a problem, but I might be able to fix it just by 
updating the old chan, not sure what status code to use though).

Ran against normal attended transfer, feature attended transfers, and blind 
transfers with no noticeable effect.

Ran against testsuite blonde transfer tests, some attended transfer tests, some 
blind transfer tests, and all pjsip transfer tests (at least ones that will run 
on my box... a few won't due to sipp version requirements that I really need to 
get around to fixing eventually) for comparison purposes. All passed exhibiting 
the same behavior as before as far as warning messages and such go.


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] 3990: CDRs/Dial: Fix an assertion caused by advancing a neutral state channel straight into dial pending without going through dial

2014-09-18 Thread rmudgett

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

Ship it!


Ship It!

- rmudgett


On Sept. 18, 2014, 5:50 p.m., Jonathan Rose wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3990/
> ---
> 
> (Updated Sept. 18, 2014, 5:50 p.m.)
> 
> 
> Review request for Asterisk Developers, Matt Jordan and rmudgett.
> 
> 
> Bugs: ASTERISK-24237
> https://issues.asterisk.org/jira/browse/ASTERISK-24237
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> Reproduction: 
> pj123 calls 1601
> pj123 does a SIP blonde transfer to 1603
> 1603 answers
> FRACK occurs
> all are PJSIP endpoints.
> 
> Basically what happens is there is a second outbound dial from another pj123 
> channel. Before the dial is answered, the pj123 gets masqueraded out of the 
> picture and replaced with a channel that isn't in the dial state.
> 
> This patch fixes that by advancing the incoming channel to the dial state in 
> the channel breakdown function of a datastore on the pj123 channel. Honestly, 
> I'm not sure this is entirely adequate, but it seems to work in all of the 
> conditions I've tried so far and it doesn't impede normal attended transfers. 
> I might need to try and figure out what happens if I masquerade in a channel 
> that is already dialing though. I'm not sure if that's something we can 
> really expect to happen under normal conditions, but that seems like 
> something that would screw up this approach.
> 
> Note that I think this patch is the right idea, I just don't know if I need 
> to account for the possibility that the channel that masquerades into pj123's 
> dialing channel might not be in the neutral state.
> 
> 
> Diffs
> -
> 
>   /branches/12/main/stasis_channels.c 422882 
> 
> Diff: https://reviewboard.asterisk.org/r/3990/diff/
> 
> 
> Testing
> ---
> 
> Ran against reproduction of the above scenario. Assertion was gone and the 
> CDR results were as follows:
> 
> "","123","1601","default"," 
> <123>","PJSIP/pj123-","PJSIP/1601-0001","Dial","PJSIP/1601,,tT","2014-09-11
>  21:48:51","2014-09-11 21:48:53","2014-09-11 
> 21:48:57",5,4,"ANSWERED","DOCUMENTATION","1410472131.0",""
> "","123","","default"," 
> <123>","PJSIP/pj123-0002","PJSIP/1603-0003","Dial","PJSIP/1603","2014-09-11
>  21:48:55",,"2014-09-11 21:48:57",2,0,"NO 
> ANSWER","DOCUMENTATION","1410472135.6",""
> "","1601","1603","default"," 
> <1601>","PJSIP/1601-0001","PJSIP/1603-0003","AppDial","(Outgoing 
> Line)","2014-09-11 21:48:57","2014-09-11 21:48:57","2014-09-11 
> 21:49:04",6,6,"ANSWERED","DOCUMENTATION","1410472131.1",""
> 
> And dial events reported by AMI:
> http://pastebin.com/tWuwL7xa
> (note that there is a mismatch between the number of dial end and dial begin 
> events... not sure if this is a problem, but I might be able to fix it just 
> by updating the old chan, not sure what status code to use though).
> 
> Ran against normal attended transfer, feature attended transfers, and blind 
> transfers with no noticeable effect.
> 
> Ran against testsuite blonde transfer tests, some attended transfer tests, 
> some blind transfer tests, and all pjsip transfer tests (at least ones that 
> will run on my box... a few won't due to sipp version requirements that I 
> really need to get around to fixing eventually) for comparison purposes. All 
> passed exhibiting the same behavior as before as far as warning messages and 
> such go.
> 
> 
> 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] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-18 Thread rmudgett

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



branches/12/res/res_pjsip_phoneprov_provider.c


Probably should be marked as extended since res_phoneprov is extended.



branches/12/res/res_pjsip_phoneprov_provider.c


Concerned about the parameter names chosen.  All uppercase parameter names 
is a bit unusual.

I think there is a requirement for pjsip parameters to be snake_case for 
ARI.



branches/12/res/res_pjsip_phoneprov_provider.c


Thge ??



branches/12/res/res_pjsip_phoneprov_provider.c


Another case where the if test burried in AST_VAR_LIST_INSERT_TAIL() is 
redundant.



branches/12/res/res_pjsip_phoneprov_provider.c


Use:
for (; (pp = ao2_iter_next()); ao2_ref(pp, -1))

This way you could use continue instead of goto cleanup.


Probably should break up the while loop body into functions.  It is rather 
large.



branches/12/res/res_pjsip_phoneprov_provider.c


Check for failure.


- rmudgett


On Sept. 18, 2014, 4:21 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3976/
> ---
> 
> (Updated Sept. 18, 2014, 4:21 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This module allows res_pjsip to integrate with res_phoneprov and depends on 
> the res_phoneprov refactor (r3970).
> 
> A new pjsip.conf object is defined by this module...
> 
> ;EXAMPLE PHONEPROV CONFIGURATION
> 
> ; Before configuring provisioning here, see the documentation for 
> res_phoneprov
> ; and configure phoneprov.conf appropriately.
> 
> ; For each user to be autoprovisioned, a [phoneprov] configuration section
> ; must be created.  At a minimum, the 'type', 'PROFILE' and 'MAC' variables 
> must
> ; be set.  All other variables are optional.
> ; Example:
> 
> ;[1000]
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;PROFILE=digium   ; required
> ;MAC=deadbeef4dad ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user confdigured variable
> 
> ; If the phoneprov sections have common variables, it is best to create a
> ; phoneprov template.  The example below will produce the same configuration
> ; as the one specified above except that MYVAR will be overridden for
> ; the specific user.
> ; Example:
> 
> ;[phoneprov_defaults](!)
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;PROFILE=digium   ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user configured variable
> 
> ;[1000](phoneprov_defaults)
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;MAC=deadbeef4dad ; required
> ;MYVAR=someOTHERvalue ; A user confdigured variable
> 
> ; To have USERNAME and SECRET automatically set, the endpoint
> ; specified here must in turn have an outbound_auth section defined.
> 
> ; Fuller example:
> 
> ;[1000]
> ;type=endpoint
> ;outbound_auth=1000-auth
> ;callerid=My Name <8005551212>
> ;transport=transport-udp-nat
> 
> ;[1000-auth]
> ;type=auth
> ;auth_type=userpass
> ;username=myname
> ;password=mysecret
> 
> ;[phoneprov_defaults](!)
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;PROFILE=someprofile  ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user configured variable
> 
> ;[1000](phoneprov_defaults)
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;MAC=deadbeef4dad ; required
> ;MYVAR=someUSERvalue  ; A user confdigur

Re: [asterisk-dev] [Code Review] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-18 Thread George Joseph


> On Sept. 18, 2014, 4:11 p.m., rmudgett wrote:
> > branches/12/res/res_phoneprov.c, lines 128-129
> > 
> >
> > This is an interesting way of doing the standard ao2 container callback 
> > functions.
> > 
> > Did you fix the formatting from the wiki page.  Directly cutting and 
> > pasting the template function from the wiki gives you spaces instead of 
> > tabs.

Yep, I reformatted it after pasting it.


> On Sept. 18, 2014, 4:11 p.m., rmudgett wrote:
> > branches/12/res/res_phoneprov.c, line 983
> > 
> >
> > Is the cast necessary?

Yes.  The compiler complains 
note: expected ‘void *’ but argument is of type ‘ast_string_field’


- George


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


On Sept. 18, 2014, 6:01 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3970/
> ---
> 
> (Updated Sept. 18, 2014, 6:01 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> The big piece missing for me to finally transition to pjsip was the ability 
> to mirror the auto provisioning features of res_phoneprov.  The first step 
> (this patch) is to make res_phoneprov more modular so other modules (like 
> pjsip) can provide configuration information instead of res_phoneprov relying 
> solely on users.conf and sip.conf.  To accomplish this a new ast_phoneprov 
> public API is now exposed which allows config providers to register 
> themselves, set defaults (server profile, etc) and add user extensions.
> 
> ast_phoneprov_provider_register registers the provider and provides callbacks 
> for loading default settings and loading users.
> ast_phoneprov_provider_unregister clears the defaults and users.
> ast_phoneprov_add_extension should be called once for each user/extension by 
> the provider's load_users callback to add them.
> ast_phoneprov_delete_extension deletes one extension.
> ast_phoneprov_delete_extensions deletes all extensions for the provider.
> 
> res_phoneprov actually registers itself as the provider for sip/users and is 
> always available and is the default.
> 
> Writing a new provider...
> Since res_phoneprov is also it's own provider, examples of what a new 
> provider would have to do are in load_users() in res_phoneprov.c.  Those 
> functions gather the information from users.conf and sip.conf and call the 
> ast_provider_register and ast_phoneprov_add_extension apis.
> 
> So...
> The provider creates a callback function which calls the 
> ast_phoneprov_add_extension api for each user.  
> It then calls ast_phoneprov_provider_register with the callback.
> res_phoneprov then calls the callback to cause the actual load.
> During normal http server ops, all work is done by res_phoneprov and the 
> provider is never called again unless a reload is needed.
> If the provider wants to reload it can simply unregister and reregister or it 
> can call its own load_users callback.
> If res_phoneprov wants to reload, it iterates over its registry and calls the 
> providers callback.
> 
> NOTE:  If res_phoneprov is actually unloaded, it has no way to know what 
> providers were registered (other than itself) so a subsequent load will have 
> nothing but it's own users.  
> 
> Additional changes...
> I added a few convenience functions to chanvars for creating lists and 
> finding and deleting entries.  No existing code was touched.
> 
> Next steps...
> A provider for res_pjsip.
> 
> 
> Diffs
> -
> 
>   branches/12/res/res_phoneprov.exports.in PRE-CREATION 
>   branches/12/res/res_phoneprov.c 423501 
>   branches/12/main/chanvars.c 423501 
>   branches/12/include/asterisk/phoneprov.h PRE-CREATION 
>   branches/12/include/asterisk/chanvars.h 423501 
>   branches/12/configs/phoneprov.conf.sample 423501 
> 
> Diff: https://reviewboard.asterisk.org/r/3970/diff/
> 
> 
> Testing
> ---
> 
> I ran through several scenarios including the use of PP_EACH_USER and 
> PP_EACH_EXTENSION to make sure that all existing functionality was preserved. 
>  I actually use it with Grandstream phones and everything worked exactly as 
> expected.
> 
> 
> Thanks,
> 
> George Joseph
> 
>

-- 
_
-- 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] 3970: res_phoneprov: Refactor phoneprov to allow pluggable config providers.

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 6:01 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Richard's comments.


Repository: Asterisk


Description
---

The big piece missing for me to finally transition to pjsip was the ability to 
mirror the auto provisioning features of res_phoneprov.  The first step (this 
patch) is to make res_phoneprov more modular so other modules (like pjsip) can 
provide configuration information instead of res_phoneprov relying solely on 
users.conf and sip.conf.  To accomplish this a new ast_phoneprov public API is 
now exposed which allows config providers to register themselves, set defaults 
(server profile, etc) and add user extensions.

ast_phoneprov_provider_register registers the provider and provides callbacks 
for loading default settings and loading users.
ast_phoneprov_provider_unregister clears the defaults and users.
ast_phoneprov_add_extension should be called once for each user/extension by 
the provider's load_users callback to add them.
ast_phoneprov_delete_extension deletes one extension.
ast_phoneprov_delete_extensions deletes all extensions for the provider.

res_phoneprov actually registers itself as the provider for sip/users and is 
always available and is the default.

Writing a new provider...
Since res_phoneprov is also it's own provider, examples of what a new provider 
would have to do are in load_users() in res_phoneprov.c.  Those functions 
gather the information from users.conf and sip.conf and call the 
ast_provider_register and ast_phoneprov_add_extension apis.

So...
The provider creates a callback function which calls the 
ast_phoneprov_add_extension api for each user.  
It then calls ast_phoneprov_provider_register with the callback.
res_phoneprov then calls the callback to cause the actual load.
During normal http server ops, all work is done by res_phoneprov and the 
provider is never called again unless a reload is needed.
If the provider wants to reload it can simply unregister and reregister or it 
can call its own load_users callback.
If res_phoneprov wants to reload, it iterates over its registry and calls the 
providers callback.

NOTE:  If res_phoneprov is actually unloaded, it has no way to know what 
providers were registered (other than itself) so a subsequent load will have 
nothing but it's own users.  

Additional changes...
I added a few convenience functions to chanvars for creating lists and finding 
and deleting entries.  No existing code was touched.

Next steps...
A provider for res_pjsip.


Diffs (updated)
-

  branches/12/res/res_phoneprov.exports.in PRE-CREATION 
  branches/12/res/res_phoneprov.c 423501 
  branches/12/main/chanvars.c 423501 
  branches/12/include/asterisk/phoneprov.h PRE-CREATION 
  branches/12/include/asterisk/chanvars.h 423501 
  branches/12/configs/phoneprov.conf.sample 423501 

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


Testing
---

I ran through several scenarios including the use of PP_EACH_USER and 
PP_EACH_EXTENSION to make sure that all existing functionality was preserved.  
I actually use it with Grandstream phones and everything worked exactly as 
expected.


Thanks,

George Joseph

-- 
_
-- 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] Git Migration

2014-09-18 Thread Paul Belanger
On Thu, Sep 18, 2014 at 5:12 PM, Russell Bryant
 wrote:
> On Thu, Sep 18, 2014 at 3:01 PM, Samuel Galarneau 
> wrote:
>>>
>>>
>>> A couple more comments about the magic happening here ...
>>>
>>> First, "git review" knows where to push based on a file checked in to the
>>> repo:
>>>
>>>  $ cat .gitreview
>>> [gerrit]
>>> host=review.openstack.org
>>> port=29418
>>> project=openstack/nova.git
>>>
>>> "git review" also sets up a local commit hook that adds a "Change-Id"
>>> header to your commit message.  That Change-Id is what links multiple
>>> revisions of the same change together.  So, if you edit your change and push
>>> it again, as long as the Change-Id remains the same, gerrit treats it as the
>>> same review request and not a new one.
>>
>>
>> Sounds good to me. So it doesn't really matter from which repo you post a
>> review so long as it's a clone of the original with that .gitreview file.
>>
>> I have another question unrelated to reviews. Does your setup make it easy
>> to mirror a repo? In a more complicated scenario, what if someone had a
>> private fork but they wanted to get public commits to master mirrored to
>> their repo? Would they have to treat the original repo as upstream and
>> manually pull changes and rebase their private branch off of it?
>
>
> Mirroring, sure.  I don't remember exactly how we do it, but all OpenStack
> repos are mirrored to github for convenience, for example.
>
Technically, the repos on github for Openstack are the mirrors of a
mirror, I know :)

The basic idea is the upstream repo lives either in gerrit or zuul,
then can mirror the repos to any system. Thing to remember, everything
goes through gerrit and zuul.  So, if you mirrored to github, PR are
useless since it is only read-only.

For internal projects, I host a cgit server and public I just mirror to github.

> Regarding private branches, git generally makes that kind of thing
> ***MUCH*** easier than svn.  You can easily track the exact commits that are
> applied on top of upstream.  You could either periodically rebase your
> changes on top of upstream (re-applying the commits, rewriting history but a
> cleaner history), or periodically merge from upstream.  In either case, I
> personally wouldn't automate it.  You want some sanity checking around that
> stuff.  Conflicts happen.  Really, I think this is going to be MUCH better
> no matter what specific infrastructure you go with.
>
> --
> Russell Bryant
>
> --
> _
> -- 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



-- 
Paul Belanger | PolyBeacon, Inc.
Jabber: paul.belan...@polybeacon.com | IRC: pabelanger (Freenode)
Github: https://github.com/pabelanger | Twitter: https://twitter.com/pabelanger

-- 
_
-- 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] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-18 Thread George Joseph


> On Sept. 18, 2014, 5:32 p.m., rmudgett wrote:
> > branches/12/res/res_pjsip_phoneprov_provider.c, lines 71-75
> > 
> >
> > Concerned about the parameter names chosen.  All uppercase parameter 
> > names is a bit unusual.
> > 
> > I think there is a requirement for pjsip parameters to be snake_case 
> > for ARI.

phoneprov variable have historically been upper case and they are 
case-sensitive.  Having some upper and some lower would be not only confusing 
but it would make it impossible to use the same template file for both sip and 
pjsip providers.


- George


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


On Sept. 18, 2014, 3:21 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3976/
> ---
> 
> (Updated Sept. 18, 2014, 3:21 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This module allows res_pjsip to integrate with res_phoneprov and depends on 
> the res_phoneprov refactor (r3970).
> 
> A new pjsip.conf object is defined by this module...
> 
> ;EXAMPLE PHONEPROV CONFIGURATION
> 
> ; Before configuring provisioning here, see the documentation for 
> res_phoneprov
> ; and configure phoneprov.conf appropriately.
> 
> ; For each user to be autoprovisioned, a [phoneprov] configuration section
> ; must be created.  At a minimum, the 'type', 'PROFILE' and 'MAC' variables 
> must
> ; be set.  All other variables are optional.
> ; Example:
> 
> ;[1000]
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;PROFILE=digium   ; required
> ;MAC=deadbeef4dad ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user confdigured variable
> 
> ; If the phoneprov sections have common variables, it is best to create a
> ; phoneprov template.  The example below will produce the same configuration
> ; as the one specified above except that MYVAR will be overridden for
> ; the specific user.
> ; Example:
> 
> ;[phoneprov_defaults](!)
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;PROFILE=digium   ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user configured variable
> 
> ;[1000](phoneprov_defaults)
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;MAC=deadbeef4dad ; required
> ;MYVAR=someOTHERvalue ; A user confdigured variable
> 
> ; To have USERNAME and SECRET automatically set, the endpoint
> ; specified here must in turn have an outbound_auth section defined.
> 
> ; Fuller example:
> 
> ;[1000]
> ;type=endpoint
> ;outbound_auth=1000-auth
> ;callerid=My Name <8005551212>
> ;transport=transport-udp-nat
> 
> ;[1000-auth]
> ;type=auth
> ;auth_type=userpass
> ;username=myname
> ;password=mysecret
> 
> ;[phoneprov_defaults](!)
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;PROFILE=someprofile  ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user configured variable
> 
> ;[1000](phoneprov_defaults)
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;MAC=deadbeef4dad ; required
> ;MYVAR=someUSERvalue  ; A user confdigured variable
> ;LABEL=1000   ; A standard variable
>  
> ; The previous sections would produce a template substitution map as follows:
> 
> ;MAC=deadbeef4dad   ;added by pp1000
> ;USERNAME=myname;automatically added by 1000-auth username
> ;SECRET=mysecret;automatically added by 1000-auth password
> ;PROFILE=someprofile;added by defaults
> ;SERVER=myserver.example.com;added by defaults
> ;SERVER_PORT=5060   ;added by defaults
> ;MYVAR=someUSERvalue;added by defaults but overdidden by user
> ;CALLER

Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-18 Thread George Joseph


> On Sept. 18, 2014, 5:32 p.m., rmudgett wrote:
> > branches/12/res/res_pjsip_phoneprov_provider.c, line 174
> > 
> >
> > Another case where the if test burried in AST_VAR_LIST_INSERT_TAIL() is 
> > redundant.

true but here I'm actually checking the return for the allocation failure and 
bailing.  res_phoneprov always silently ignored the allocation failure and just 
didn't insert the variable into the list.

Silently ignoring non-required variables can't hurt anything and if the system 
is so low on memory that it can't allocate a dozen bytes then you'll never see 
the error messages anyway.  So, I guess I'll remove the check here and silently 
ignore.


- George


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


On Sept. 18, 2014, 3:21 p.m., George Joseph wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviewboard.asterisk.org/r/3976/
> ---
> 
> (Updated Sept. 18, 2014, 3:21 p.m.)
> 
> 
> Review request for Asterisk Developers.
> 
> 
> Repository: Asterisk
> 
> 
> Description
> ---
> 
> This module allows res_pjsip to integrate with res_phoneprov and depends on 
> the res_phoneprov refactor (r3970).
> 
> A new pjsip.conf object is defined by this module...
> 
> ;EXAMPLE PHONEPROV CONFIGURATION
> 
> ; Before configuring provisioning here, see the documentation for 
> res_phoneprov
> ; and configure phoneprov.conf appropriately.
> 
> ; For each user to be autoprovisioned, a [phoneprov] configuration section
> ; must be created.  At a minimum, the 'type', 'PROFILE' and 'MAC' variables 
> must
> ; be set.  All other variables are optional.
> ; Example:
> 
> ;[1000]
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;PROFILE=digium   ; required
> ;MAC=deadbeef4dad ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user confdigured variable
> 
> ; If the phoneprov sections have common variables, it is best to create a
> ; phoneprov template.  The example below will produce the same configuration
> ; as the one specified above except that MYVAR will be overridden for
> ; the specific user.
> ; Example:
> 
> ;[phoneprov_defaults](!)
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;PROFILE=digium   ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user configured variable
> 
> ;[1000](phoneprov_defaults)
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;MAC=deadbeef4dad ; required
> ;MYVAR=someOTHERvalue ; A user confdigured variable
> 
> ; To have USERNAME and SECRET automatically set, the endpoint
> ; specified here must in turn have an outbound_auth section defined.
> 
> ; Fuller example:
> 
> ;[1000]
> ;type=endpoint
> ;outbound_auth=1000-auth
> ;callerid=My Name <8005551212>
> ;transport=transport-udp-nat
> 
> ;[1000-auth]
> ;type=auth
> ;auth_type=userpass
> ;username=myname
> ;password=mysecret
> 
> ;[phoneprov_defaults](!)
> ;type=phoneprov   ; must be specified as 'phoneprov'
> ;PROFILE=someprofile  ; required
> ;SERVER=myserver.example.com  ; A standard variable
> ;TIMEZONE=America/Denver  ; A standard variable
> ;MYVAR=somevalue  ; A user configured variable
> 
> ;[1000](phoneprov_defaults)
> ;endpoint=1000; Required only if automatic setting of
>   ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
>   ; are needed.
> ;MAC=deadbeef4dad ; required
> ;MYVAR=someUSERvalue  ; A user confdigured variable
> ;LABEL=1000   ; A standard variable
>  
> ; The previous sections would produce a template substitution map as follows:
> 
> ;MAC=deadbeef4dad   ;added by pp1000
> ;USERNAME=myname;automatically added by 1000-auth username
> ;SECRET=mysecret;automatically added by 1000-auth password
> ;PROFILE=someprofile;added by defaults
> ;SERVER=myserver.example.com;added by defaults
> ;SERVER_PORT=5060   ;added 

Re: [asterisk-dev] [Code Review] 3976: New module: res_pjsip_phoneprov_provider provides the integration between res_pjsip and res_phoneprov.

2014-09-18 Thread George Joseph

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

(Updated Sept. 18, 2014, 6:55 p.m.)


Review request for Asterisk Developers.


Changes
---

Addressed Richard's comments.


Repository: Asterisk


Description
---

This module allows res_pjsip to integrate with res_phoneprov and depends on the 
res_phoneprov refactor (r3970).

A new pjsip.conf object is defined by this module...

;EXAMPLE PHONEPROV CONFIGURATION

; Before configuring provisioning here, see the documentation for res_phoneprov
; and configure phoneprov.conf appropriately.

; For each user to be autoprovisioned, a [phoneprov] configuration section
; must be created.  At a minimum, the 'type', 'PROFILE' and 'MAC' variables must
; be set.  All other variables are optional.
; Example:

;[1000]
;type=phoneprov   ; must be specified as 'phoneprov'
;endpoint=1000; Required only if automatic setting of
  ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
  ; are needed.
;PROFILE=digium   ; required
;MAC=deadbeef4dad ; required
;SERVER=myserver.example.com  ; A standard variable
;TIMEZONE=America/Denver  ; A standard variable
;MYVAR=somevalue  ; A user confdigured variable

; If the phoneprov sections have common variables, it is best to create a
; phoneprov template.  The example below will produce the same configuration
; as the one specified above except that MYVAR will be overridden for
; the specific user.
; Example:

;[phoneprov_defaults](!)
;type=phoneprov   ; must be specified as 'phoneprov'
;PROFILE=digium   ; required
;SERVER=myserver.example.com  ; A standard variable
;TIMEZONE=America/Denver  ; A standard variable
;MYVAR=somevalue  ; A user configured variable

;[1000](phoneprov_defaults)
;endpoint=1000; Required only if automatic setting of
  ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
  ; are needed.
;MAC=deadbeef4dad ; required
;MYVAR=someOTHERvalue ; A user confdigured variable

; To have USERNAME and SECRET automatically set, the endpoint
; specified here must in turn have an outbound_auth section defined.

; Fuller example:

;[1000]
;type=endpoint
;outbound_auth=1000-auth
;callerid=My Name <8005551212>
;transport=transport-udp-nat

;[1000-auth]
;type=auth
;auth_type=userpass
;username=myname
;password=mysecret

;[phoneprov_defaults](!)
;type=phoneprov   ; must be specified as 'phoneprov'
;PROFILE=someprofile  ; required
;SERVER=myserver.example.com  ; A standard variable
;TIMEZONE=America/Denver  ; A standard variable
;MYVAR=somevalue  ; A user configured variable

;[1000](phoneprov_defaults)
;endpoint=1000; Required only if automatic setting of
  ; USERNAME, SECRET, DISPLAY_NAME and CALLERID
  ; are needed.
;MAC=deadbeef4dad ; required
;MYVAR=someUSERvalue  ; A user confdigured variable
;LABEL=1000   ; A standard variable
 
; The previous sections would produce a template substitution map as follows:

;MAC=deadbeef4dad   ;added by pp1000
;USERNAME=myname;automatically added by 1000-auth username
;SECRET=mysecret;automatically added by 1000-auth password
;PROFILE=someprofile;added by defaults
;SERVER=myserver.example.com;added by defaults
;SERVER_PORT=5060   ;added by defaults
;MYVAR=someUSERvalue;added by defaults but overdidden by user
;CALLERID=8005551212;automatically added by 1000 callerid
;DISPLAY_NAME=My Name   ;automatically added by 1000 callerid
;TIMEZONE=America/Denver;added by defaults
;TZOFFSET=252100;automatically calculated by res_phoneprov
;DST_ENABLE=1   ;automatically calculated by res_phoneprov
;DST_START_MONTH=3  ;automatically calculated by res_phoneprov
;DST_START_MDAY=9   ;automatically calculated by res_phoneprov
;DST_START_HOUR=3   ;automatically calculated by res_phoneprov
;DST_END_MONTH=11   ;automatically calculated by res_phoneprov
;DST_END_MDAY=2 ;automatically calculated by res_phoneprov
;DST_END_HOUR=1 ;automatically calculated by res_phoneprov
;ENDPOINT_ID=1000   ;automatically added by this module
;AUTH_ID=1000-auth  ;automatically added by this module
;TRANSPORT_ID=transport-udp-nat ;automatically added by this module
;LABEL=1000 ;added by user


Diffs (updated)
-

  branches/12/res/res_pjsip_phoneprov_provider.c PRE-CREA

[asterisk-dev] [Code Review] 4003: 503 incorrectly sent to rentransmitted invites

2014-09-18 Thread Torrey Searle

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

Review request for Asterisk Developers.


Repository: Asterisk


Description
---

Scenario
INVITE arrives to asterisk, asterisk responds Busy()
If the INVITE is retransmitted, asterisk will generate a 503 in addition to the 
486.

Patch ensures a 2nd response isn't generated to an already responded INVITE


Diffs
-

  http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 423250 

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


Testing
---

Asterisk testsuite test has been created to reproduce this error


Thanks,

Torrey Searle

-- 
_
-- 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] 4003: 503 incorrectly sent to rentransmitted invites

2014-09-18 Thread Torrey Searle

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

(Updated Sept. 19, 2014, 6:52 a.m.)


Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

Scenario
INVITE arrives to asterisk, asterisk responds Busy()
If the INVITE is retransmitted, asterisk will generate a 503 in addition to the 
486.

Patch ensures a 2nd response isn't generated to an already responded INVITE


Diffs
-

  http://svn.asterisk.org/svn/asterisk/branches/11/channels/chan_sip.c 423250 

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


Testing
---

Asterisk testsuite test has been created to reproduce this error


Thanks,

Torrey Searle

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