Re: [asterisk-dev] Module Deprecation, Default Not Building, and Removal

[Corey Farrell]

Yes, or "core, deprecated in future versions" in the minors.  I don't 
have strong feelings on the exact language just that we should indicate 
the long term future of a module has been decided.


Also sorry I accidentally didn't send my last reply to the list.

On 10/1/20 1:20 PM, Joshua C. Colp wrote:
On Thu, Oct 1, 2020 at 2:18 PM Corey Farrell <mailto:g...@cfware.com>> wrote:


Yes I'm suggesting documenting that it is deprecated in minor
releases.  Ending support in a minor seems bad unless support
already doesn't exist.  Could we add
AST_MODULE_SUPPORT_CORE_DEPRECATED and
AST_MODULE_SUPPORT_EXTENDED_DEPRECATED to `enum
ast_module_support_level`?  Then a module would get one of those
support levels in a minor, AST_MODULE_SUPPORT_DEPRECATED in master.


We could, but just so I'm clear - it would be stated as "to be 
deprecated in future" essentially in minor releases and then marked as 
deprecated in master?


--
Joshua C. Colp
Asterisk Technical Lead
Sangoma Technologies
Check us out at www.sangoma.com <http://www.sangoma.com> and 
www.asterisk.org <http://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] Module Deprecation, Default Not Building, and Removal

[Corey Farrell]

Any reason we can't "documentation deprecate" things in minor releases?  
No runtime warnings and keep building by default but if we deprecate a 
module in master right now it seems like the next minor release of all 
active branches should document the status of the module.  The fact that 
a module will still be supported on 16.14.0 doesn't stop us from telling 
users what will happen.


On 10/1/20 12:25 PM, Joshua C. Colp wrote:
On Thu, Oct 1, 2020 at 1:15 PM Dan Jenkins > wrote:


Firstly, thank you Josh for trying to outline the process so that
it's something that can be followed and while I agree mostly with
the steps... the fact that its going to take 4 years for a module
to be moved from "deprecated" to being removed just feels too long
but understandable if we're talking about "this module has GONE
from the source code"

I'm not really sure if my thought process here is tainted by the
chan_sip process that seems to have gone on for an eternity already.

I personally don't see a problem with deprecating a module in non
LTS and removing it from being default enabled in the LTS - the
code is still there, and can still be enabled and packagers could
still enable it. I don't know what choices packagers make as to
what gets built and what doesn't as I don't use packages - each
install f Asterisk has different needs and so I always end up
building from source. And then we could remove it in the next
standard release cutting the time by half.

Would the above be ok if there was a "simple" way to say bring in
external modules at build time? IE movethe deprecated module to a
separate repo, and have the option to be able to bring it in
dynamically during build "at your own risk" - just like what
happens with pjproject, codecs etc

Or is that not really achieving the goal because there would still
need to be some sort of maintenance for these deprecated modules.


That still incurs the maintenance burden in the first place. Even with 
an "at your own risk" perspective if you make it an easy ability to 
bring it in people will still have an expectation that it work, and 
when it doesn't then you need to deal with it in some way.



I don't know... but essentially 4 years feels like forever and LTS
and Standard releases exist for a reason - I don't see why we need
two LTS releases before somethings removed.


The reason I opted for the way I did is that a portion of the user 
base don't run standard releases. They keep on LTS, so if you only do 
something in a standard release then they'll miss it. With my proposal 
standard release acts as an initial gauge of things before being 
released to the wider community. While I can understand the appeal of 
wanting to remove things as quickly as possible, it's not something I 
want to force as quick as possible upon the user base. It's a delicate 
balance to have so as to not drive people away, to give them time to 
transition and move, and to plan.


--
Joshua C. Colp
Asterisk Technical Lead
Sangoma Technologies
Check us out at www.sangoma.com  and 
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] Proposal for New Major Version Process Change

[Corey Farrell]

With the current process new features can be added with tests can target 
18 as soon as the branch is cut but would not go into 18.0.  What would 
happen with new features in this scheme, would they just get held open 
in gerrit until 18.0 is branched or be accepted as normal since 18.0.0 
release process is not yet started?  I agree a month is plenty of time 
for the release candidate cycle but think it would be good to decide how 
new features will be dealt with before 18 is branched.


On 7/8/20 8:20 AM, Joshua C. Colp wrote:

Greetings all,

I've given this some thought in the past and thought with the 
impending branching of Asterisk 18 I'd get some input on a change to 
the process. The new major version process has evolved over time but 
hasn't really been changed lately. Let's look at what it is like in 
practice today:


On July 15th under current process both the 18 branch and 18.0 
branches will be cut. The 18 branch will continue to receive all bug 
fixes, but the 18.0 branch will only receive changes as a result of 
issues found through testing 18.0 or through big fixes to new 
functionality in it. This means that when 18.0.0 is actually released 
it is generally a few months behind. In the past this was to give it 
time to stabilize as it were. This presents the following issues:


1. It leaves a confusing area for developers where we have to ask 
"should this go into 18.0?"
2. It confuses users because if they upgrade to 18.0.0 then it is 
likely the other current releases have bug fixes they don't have, 
which has caused issues for users in the past.


I don't think this is the best situation for either.

I'd like to propose that instead of cutting the 18.0 branch on July 
15th we simply cut the 18 branch, and that it continues to receive all 
bug fixes. Approximately a month before a target release of 18.0.0 we 
create the first release candidate, 18.0.0-rc1. At this time we also 
create release candidates of the other branches. All release 
candidates will then be left available for a prolonged period of time 
to give people ample time to test. On the release date all will be 
released, ensuring that all branches including 18 have the same set of 
bug fixes as appropriate to their version.


This removes the confusion for developers over whether to include a 
fix, since the 18.0 branch won't exist until rc1 at which point normal 
cherry pick rules apply. This also eliminates the confusion 
experienced by users since all releases will be on the same page at 
the same time at release.


What do people think? Do we believe that a month out is ample enough? 
The 18 branch itself will exist, so that can be used for early testing 
(and likely will be). If a month isn't enough it could be moved out 
further. Really I think thanks to the testing that happens and the 
code review I don't think we need as long of a stabilization period as 
has been needed in the past, so this helps shrink it.


Cheers,

--
Joshua C. Colp
Asterisk Technical Lead
Sangoma Technologies
Check us out at www.sangoma.com  and 
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] [BOUNTY] X-Header on SIP BYE

[Corey Farrell]

chan_sip is fragile and has no active maintainers.  Personally I think 
new features do not belong in chan_sip, should instead be implemented 
for chan_pjsip.


On 11/19/19 8:06 AM, bala murugan wrote:

Hi Ernst ,

   I can develop this for you in chan_sip driver  on the version you 
are using 15.7.2 . If you could provide more details like


     - How are you planning to pass the value for the X-Header ?
     - Is this needed for all BYE initiated by asterisk ?
     - Is this required for Bridged call ?

regards,
bala
On Tue, Nov 19, 2019 at 6:27 PM Ernst Nusterer > wrote:


Joshua,

thank you very much for your message!

We are using PJSIP and Asterisk 15.7.2 under Ubuntu 18.04, but we
are quite flexible to use another Asterisk version if needed (or
the other sip driver).

Kind regards

Ernst

On 19.11.19 13:52, Joshua C. Colp wrote:

I would suggest specifying which version of Asterisk and also
which SIP channel driver. There are two: chan_pjsip and chan_sip,
and anyone interested in this bounty would likely want to know which.

On Tue, Nov 19, 2019 at 8:49 AM Ernst Nusterer mailto:ernst@gmx.at>> wrote:

Hello, we would need X-Headers to be sent with the SIP BYE
request and can not implement it ourselves in Asterisk.
Currently this is not possible. Thus, I would like to offer a
bounty of USD 2000 for the developer who develops this for
us. The code needed for this can go in the Asterisk
distribution of course and be made open source. Please let me
know if the amount offered is not enough for the complexity
of this development, I can not estimate how much effort it
is. Money will be sent via Pay Pal when the solution is ready
(or on another desired channel) Thank you very much for your
help! Please mail me for questions! Thanks a lot & kind
regards Ernst NB: I have sent this message already once, but
it seems to have been rejected. If that is not the case,
please apologize the duplication. Anyway, I have increased
the bounty :-)

-- 
_

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



-- 
Joshua C. Colp

Digium - A Sangoma Company | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.sangoma.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


-- 
_
-- 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] Asterisk and CentOS 8

[Corey Farrell]

iksemel-devel: To my knowledge library is not maintained and has been 
removed from Fedora because it no longer builds from source. Honestly I 
thought we had deprecated res_xmpp and chan_motif for this very reason.


libedit-devel: This is mandatory since Asterisk 16.  The devel package 
is found the in PowerTools repository which is not enabled by default.


Other stuff install_prereq installs from the PowerTools repo:

bluez-libs-devel
doxygen
gsm-devel
libogg-devel
libsndfile-devel
libsrtp-devel
libvorbis-devel
lua-devel
neon-devel
speex-devel
speexdsp-devel

Why RedHat decided to hide these devel packages in a repository that is 
disabled by default it is beyond me.



On 10/17/19 2:24 PM, George Joseph wrote:



On Thu, Oct 17, 2019 at 12:03 PM James Finstrom > wrote:


I started writing
https://github.com/jfinstrom/just-a-wiki/wiki/FreePBX-on-Centos-8
when Centos 8 launched. It actually 90% works.
FreePBX itself has some weird stuff happening and I haven't had
time to revisit it. That said the asterisk portion built just fine.


Here are the missing packages.  I guess as long as you don't need any 
of their functionality you could be OK but libsrtp is a big one.


libedit-devel
speex-devel
speexdsp-devel
libogg-devel
libvorbis-devel
xmlstarlet
neon-devel
gmime-devel
lua-devel
uriparser-devel
bluez-libs-devel
freetds-devel
iksemel-devel
corosynclib-devel
spandsp-devel
libresample-devel
libsrtp-devel
gsm-devel
doxygen
hoard
codec2-devel
libsndfile-devel


On Thu, Oct 17, 2019 at 9:19 AM George Joseph mailto:gjos...@digium.com>> wrote:

At the current time, we do not recommend attempting to build
Asterisk on CentOS 8.  Many packages Asterisk uses are not yet
available and would require building from their sources.  The
Asterisk packages are also not available in the EPEL 8 or
CentOS 8 repositories yet for the same reason.

We'll update you when we think it's safe.

-- 
*George Joseph*

Digium - A Sangoma Company | Software Developer | Software
Engineering
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
direct/fax: +1 256 428 6012
Check us out at: https://digium.com  ·
https://sangoma.com 

-- 
_

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



-- 
*James Finstrom*

Guy who does stuff that is sometimes cool

gpg: https://github.com/jfinstrom.gpg

This email was sent from a personal email account. The content of
this email is not endorsed by my employer or any project I may be
a part of. The contents of this email should be considered my
opinion and not taken as any form of official response. Please
keep your hands and feet in the ride while in motion. Please be
sure to tip the wait staff.
-- 
_

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



--
*George Joseph*
Digium - A Sangoma Company | Software Developer | Software Engineering
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
direct/fax: +1 256 428 6012
Check us out at: https://digium.com  · 
https://sangoma.com 



-- 
_
-- 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] Memory leak since Asterisk 16.5.x / pjsip

[Corey Farrell]

Updating pjproject does take less time/effort than maintaining a fork 
for the many years of an LTS release.  That reduced effort isn't just 
free time to developers, the time is instead spent working on 
enhancements and bug fixes.  Maintaining a fork would prevent users from 
getting important upstream bug fixes and would introduce risk of 
regressions caused by the fork itself.  So my vote is that pjsip version 
bumps should not be held back.  I'm also not in favor of creating 
separate releases for pjsip version bumps, this takes time that I feel 
can/should be spent on other tasks.


One thing I think Asterisk can improve is to always make sure that any 
pjsip version bump is prominently mentioned in release notes, probably a 
note in the CHANGES and/or UPGRADE files.  This would let people who use 
bundled pjproject of potentially major changes.  It would also tell 
users of non-bundled pjproject that they should upgrade their own 
libraries as the older version is no longer tested.


On 9/16/19 1:06 PM, Michael Maier wrote:

On 15.09.19 at 21:19 Joshua C. Colp wrote:

On Sun, Sep 15, 2019, at 2:21 AM, Michael Maier wrote:

BTW: I'm not really happy with the fact, that an existing LTS / stable version gets a new 
pjsip version "on the fly". From my point of view, this should have been
done during a normal development cycle and not during a stable phase.

Since support for bundled PJSIP we've actively tried to keep up to date, so 
that we don't end up managing a fork and backporting a lot of patches. This has 
worked well
for us and we haven't seen any problems - in fact we've gained some stability 
at times.

Chance - there's always a first time :-)
BTW: I like the bundling of pjsip!


If this is a problem in PJSIP this would be the first time we've encountered a
regression. If people feel that we should instead lock versions then this is 
certainly something we can discuss. What do others think?

 From a developers perspective, it's for sure better to do it as you do it like 
now. From a users / customers perspective, it's most probably the other way 
round. I don't
want to have any deep changes during a LTS version (that's exactly why I'm 
using LTS versions). The new pjsip release should have been put to a new 
asterisk release, too.
Asterisk 16.x was thoroughly tested and released on base of pjsip 4.8. Anybody 
who wants new pjsip 4.9 should consider using new Asterisk version, too.

At least, I would expect a severe distinction by using a dedicated minor 
version (without any own asterisk changes) to detect more easily potential 
pjsip regressions.


Thanks
Michael



--
_
-- 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] ARI, Stasis, and Dialplan

[Corey Farrell]

How will the ARI/dialplan integration handle specific extensions?  For 
example if I have a stasis app which registers itself to dialplan as 
'somecontext', how does this integration decide which extensions are 
handled by the app?  Does that app get calls for all extensions or only 
specific extensions?  Do we create a new type of ARI app which would 
respond to PBX switch callbacks where all calls go to the stasis router 
app which then accepts or rejects calls based on the ARI apps own 
extension list?  For example if we have a context:


[from-outside]
exten => 7002052000,1,Stasis(myapp)
exten => 7002052001,1,Stasis(myapp)

How do you envision replicating having these two extensions handled but 
all other extensions being invalid?


On 12/17/18 10:54 AM, Ben Ford wrote:
Thanks for all the feedback! I believe we have a strong enough 
consensus to move forward. If anyone has any other thoughts on the 
direction of this, feel free to post them here anytime, preferably 
sooner rather than later so that if there is a conflict or something 
really cool we'd love to add, development isn't too far along yet :)


On Thu, Dec 13, 2018 at 3:27 PM Seán C. McCord > wrote:


Sorry if I was unclear.  Yes, I would love to extend the
"Continue" function to add App and AppArgs as an option to use
instead of Context/Extension/Priority. "Originate" and "Create"
both allow these already.  Adding these to "Continue" would be
very helpful.

On Thu, Dec 13, 2018 at 2:45 PM Ben Ford mailto:bf...@digium.com>> wrote:

To elaborate on this further, would it be fine the way it is
currently, where you specify a context and extension, or is
there any interest in being able to alternatively specify an
application and arguments to pass to it?

On Thu, Dec 13, 2018 at 12:37 PM Seán C. McCord
mailto:ule...@gmail.com>> wrote:

Absolutely.  I really forgot about that, because I've just
molded my design approach to maintain a single ARI
application per node, but that extension to Continue
(similar to Originate's App/AppArgs properties), would be
enormously helpful.


On Thu, Dec 13, 2018 at 11:47 AM Ben Ford
mailto:bf...@digium.com>> wrote:

I think having a context that's created when the
application starts sounds like a solid approach.

Another thing to consider is how to move from one
application to the next. Any thoughts on the use of
/continue to tackle this? Is this something you'd like
to be able to make use of in your applications?

On Wed, Dec 12, 2018 at 11:42 PM Anil Gupta
mailto:anilgupt...@gmail.com>>
wrote:

+1 to the context idea

Something like /*context = stasis:app_name*/ would
be nice. I presume this could be achieved by
adding the functionality to the PBX module and
most (if not all) channel drivers would work
without modification.

Thanks,
Anil

On Thu, Dec 13, 2018 at 2:16 AM Dan Jenkins
mailto:d...@nimblea.pe>> wrote:

Oh I do remember the context idea  - which
seemed like a good idea at the time because of
not having to change much internally

On Wed, Dec 12, 2018 at 7:07 PM Seán C. McCord
mailto:ule...@gmail.com>>
wrote:

Correction: when I said the "latter," I
should have said the "third"
option.  The last option does not really
work well, since _channels_
map to _contexts_, not CELs.
On Wed, Dec 12, 2018 at 1:52 PM Seán C.
McCord mailto:ule...@gmail.com>> wrote:
>
> Neither of my previous emails appear to
be showing up third time's
> the charm?
>
> We had briefly discussed the idea of
creating virtual context/dialplan
> to handle this.  The idea would be that
a context and dialplan would
> be internally and automatically
generated which would direct calls
> sent to that context directly to ARI. 
Since the impetus for this
> feature was operational simplicity, not
 

Re: [asterisk-dev] Strange issue with permissions and a third party module

[Corey Farrell]

If using Fedora / similar systems it could be an SELinux policy issue.  
In Fedora /usr/sbin/asterisk is a confined process where python is not.  
On Fedora/CentOS SELinux will forbid Asterisk from taking certain 
actions even if allowed by the file system permissions set by chmod.  
It's possible chan_dongle is doing something that Asterisk / standard 
modules does not do so SELinux would block by default, in which case you 
could use audit2allow to create a custom SELinux module that would grant 
the additional permissions to the asterisk process.


On 12/10/18 12:53 PM, Joshua C. Colp wrote:

On Wed, Dec 5, 2018, at 12:40 PM, Hans-Peter Jansen wrote:

Hi *sters,

looking at a strange issue here. Experimenting with:

https://github.com/wdoekes/asterisk-chan-dongle

is all doing fine so far, but locking the device. While not essential, I would
like to understand, what's going on (wrong). Since chan-dongle handles serial
connections, it attempts to lock them in order to conform to usual Linux
standards. Simply put, it needs to create lock files like this:

/var/lock/LCK..ttyUSB1

In current Linux distributions, that requires it to be a member of the lock
group:

$ getent group lock
lock:x:54:uucp,asterisk

Now this code comes to action:

https://github.com/wdoekes/asterisk-chan-dongle/blob/master/chan_dongle.c#L123

but results in (strace excerpt):

openat(AT_FDCWD, "/var/lock/LCK..ttyUSB1", O_WRONLY|O_CREAT|O_TRUNC, 0444) =
-1 EACCES (Permission denied)

Simulating this call with sudo and a python script succeeds nevertheless:

$ sudo -u asterisk python3 /tmp/lckopen.py
$ l /var/lock/
total 0
drwxrwxr-x  3 root lock 100 Dec  5 17:13 ./
drwxr-xr-x 24 root root 720 Dec  3 20:18 ../
-r--r--r--  1 asterisk asterisk   0 Dec  5 17:13 LCK..ttyUSB1

$ cat /tmp/lckopen.py
import os
try:
 os.open('/var/lock/LCK..ttyUSB1', os.O_WRONLY|os.O_CREAT|os.O_TRUNC,
0o444)
except IOError as e:
 print('failed: %s' % e)

strace excerpt:

openat(AT_FDCWD, "/var/lock/LCK..ttyUSB1", O_WRONLY|O_CREAT|O_TRUNC|O_CLOEXEC,
0444) = 3

I patched chan_dongle to add the O_CLOEXEC flag, which python3 seems to add
behind the scenes, but no bonus. Added code to check for uid and euid on both
parties reveals the expected results: the systems asterisk uid is effectively
in use, so there's no reason to fail.

Why does the Asterisk module behaves differently permission-wise?

How is Asterisk actually run and executed? Is it being run as a systemd unit, 
could that be altering permissions and limiting things?


Does Asterisk use some special protection/capabilities for its modules?

Nope, we do nothing special and rely on the system itself. We can drop down to 
a different user and such, that's about it.



--
_
-- 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] Testsuite python3 compatibility

[Corey Farrell]

Sorry for the delayed reply I've been busy with other projects lately.


On 06/06/2018 04:38 AM, Alexander Traud wrote:

I love to help. However, before I apply your patch, I need a running
Test Suite first. In Ubuntu 18.04 LTS with branch Master, 11 test cases
fail for me, for example ASTERISK-27897. In that report you see my
concrete steps. Perhaps you spot the mistake. In Ubuntu 16.04 LTS with
branch 15.4, 30 test cases fail. 5 cases, which fail in Ubuntu 18.04 LTS
do not fail in Ubuntu 16.04 LTS. Consequently, I have still 6 cases
which fail. Is it OK, that 6 tests cannot be verified?
6 is less than I would face on my machine.  It would be very helpful to 
me if you could run the test again with my patch and just post your 
before / after asterisk-test-suite-report.xml to ASTERISK-26826 (please 
be sure to rename them before uploading so JIRA shows different filenames).




is a bit out of date.

Uhh, I hoped I had overlooked something. I knew that wiki page. The
Test Suite requires much more Python modules (see below). That Wiki
would help much more if it described the platform/version used. That
Wiki entry is Red Hat Fedora. However,
1) Which Fedora version is it exactly?
2) How are the Python modules installed (pip or package manager)?
Those two puzzle pieces would help much.
Sorry I don't know which version of Fedora the wiki is based on, I 
suspect it's been updated a few times.



asttest [is] not compatible with current versions of Lua.

Yes, it does not work with Lua 5.3 or 5.2 and still requires 5.1.
  

If you have to install [SIPp] from source please download the latest
version, not 3.4.1.

Because I want support for SIP-over-TLS in Ubuntu, I had to go for the
source. However, even the latest SIPp release, I had to patch for
OpenSSL 1.1.x. By the way, do you know why this 'greater 3.4.1'
requirement is not reflected by any dependency check:
$ ./runtests.py --list-tests
I'm not sure.  Really event the current 'master' unreleased version of 
sipp has at least one bug-fix that has impacted the asterisk.org jenkins 
server.  I'm not aware of any situation where running a newer version of 
sipp causes a problem.

Some additional python modules needed:
autobahn, lxml, construct, netifaces, pyxb.

... plus numpy, requests, twisted, and yaml. For construct, I had to go
for construct.legacy and patch four files: ASTERISK-27893.

Thanks for looking into these and for the patches you've posted recently.



skipped due to each dependency.

Puh. I was able to reduce everything to 3 missing dependencies: remote,
statsd, and rtpdump. Although I was able to install rtpdump, my variant
did not take a parameter 'd': ASTERISK-27080. Another 3 tests fail
and I have no idea why:
- tests/masquerade,
- tests/hep/rtcp-sender, and
- tests/hep/rtcp-receiver.
Consequently, 6 cases cannot be tested for regressions.


a full testsuite run is required to verify no errors. I do not have
adequate hardware

By the way, what exactly do you mean by this? Perhaps, I face the same
issue.

I'm working off a laptop which gets hotter than I'd like by running the 
testsuite.  I routinely face lock-up of the testsuite run where I'll 
have to kill a process before things will progress.  If you've got it 
down to only 6 tests that cannot pass it would be very helpful to see 
the list of regressions plus the list of 6 tests where regressions could 
not be checked.


--
_
-- 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] Testsuite python3 compatibility

[Corey Farrell]

An Asterisk WIKI page [1] exists.  It has the basics but it's a bit out 
of date.


Some notes:

 * For the purpose of this test you should use Linux.  It may work on
   *BSD but that would introduce a new mostly untested variable.  If
   you are doing this in a VM the options which have been tested most
   are Fedora 27 and Ubuntu 17 (they're used by jenkins build agents). 
   CentOS 7 is also used by the build agents but Fedora/Ubuntu are
   likely easier to get all the dependencies installed.
 * Don't worry about asttest if you can't build it.  It's not
   compatible with current versions of lua and has no effect on this
   review (lua tests do not use python).
 * For python libraries make sure you are installing the python2
   versions of everything.  As I said some tests do work in python3
   with my patch but the goal of this testing is to ensure no
   regressions are created in python2.  Once that's done the changes
   needed to get python3 working will be much smaller.
 * For starpy dependency please use the patch from
   https://gerrit.asterisk.org/8918.
 * You may be able to use the version of sipp packaged with the OS,
   just check 'sipp -v' to see if it was compiled with the required
   features.  If you have to install from source please download the
   latest version, not 3.4.1.
 * Some additional python modules needed: autobahn, lxml, construct,
   netifaces, pyxb.

Once you believe you have the dependencies please run `./runtests.py 
-l|grep -e 'Met: False'|sort|uniq -c`.  This will tell you if any 
dependencies are missing and how many tests will be skipped due to each 
dependency.  To actually run the testsuite you can use './runtests.py' 
without any arguments.  One argument that might be useful is 
`--timeout=300`.  This way if any test stops producing output for 5 
minutes the testsuite will assume it locked up and will terminate it / 
move on.  The full run should take at least a couple hours.


After you've run the testsuite please upload your 
asterisk-test-suite-report.xml to JIRA even if you had total success.  
This way we can see which tests (if any) were skipped due to 
dependencies and we'll be able to target those for testing.


Thank you, Corey

[1] 
https://wiki.asterisk.org/wiki/display/AST/Installing+the+Asterisk+Test+Suite



On 05/12/2018 09:32 AM, Alexander Traud wrote:

who has available hardware to run the complete testsuite
against Asterisk master

Would love to help. What is required exactly? I never got the Test Suite 
running because of its amount of dependencies. Is there an up-to-date guide 
anywhere?





-- 
_
-- 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] Testsuite python3 compatibility

[Corey Farrell]

I've posted python3 compatibility changes for the testsuite [1]. This 
has gotten some testing (which found issues that have been fixed), but 
really a full testsuite run is required to verify no errors.  I do not 
have adequate hardware to complete this task, I'm hoping someone in the 
community could assist with this.  I'm looking for anyone who has 
available hardware to run the complete testsuite against Asterisk master 
and report any regressions this patch causes for python2.  Although some 
tests may work with python3 many will not, once this patch is merged we 
can more easily fix issues in the individual tests.  Feel free to report 
regressions as findings on the review or you can just upload 
asterisk-test-suite-report.xml to the JIRA ticket [2].


Thanks,
Corey

[1] https://gerrit.asterisk.org/8854
[2] https://issues.asterisk.org/jira/browse/ASTERISK-26826

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

[Corey Farrell]

I'm working on some PR's to update the Asterisk testsuite to be 
compatible with Python3 (without breaking Python2).  An issue I've hit 
is starpy.  A PR to deal with compatibility was started but never 
finished.  Can we move starpy to gerrit.asterisk.org and convert the 
github repository to a mirror the same way Asterisk and the testsuite 
are done?  starpy is a critical dependency of the Asterisk testsuite and 
PR's on github.com do not get proper attention since most Asterisk 
developers do not follow it.  Using gerrit to perform active work would 
ensure important PR's do not go stale.



--
_
-- 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] Compiler feature requirements

[Corey Farrell]

This will definitely break any compilers older than gcc-4.1.2, but no 
major distribution (that I know of) has a supported release with this.  
CentOS 6 is the only major distro that will even use the __sync [1] 
methods. All other major distro's have gcc >= 4.7.0 which implements 
__atomic built-ins.


I tested OSX El Capitain, the configure script detected __atomic 
built-ins so this will not create any new issues for Mac.


[1] https://gist.github.com/coreyfarrell/c096dd335afee5502a6faee2a507b012

On 01/26/2018 03:20 PM, Matt Fredrickson wrote:

On Wed, Jan 24, 2018 at 3:50 PM, Corey Farrell <g...@cfware.com> wrote:

I've posted ASTERISK-27619 [1] proposing that we drop support for GCC
versions older than 4.1.2.  Specifically we'd be requiring that either
__sync or __atomic builtin functions be available (I'm unsure what this will
do to clang requirements).  gcc-4.1.2 was released in February 2007 and was
the version provided by CentOS 5.  I've posted a PR to the jansson project
[2] which will make reference counting thread safe, but I'm getting
push-back on the parts needed to provide a replacement function for old
compilers.  Since reference counting in jansson was never thread safe before
I think they'd rather just leave it as is for old compilers.

Obviously this proposal is for Asterisk 16+ only.  Does this matter to any
distributions that will be supported beyond this October?

[1] https://issues.asterisk.org/jira/browse/ASTERISK-27619
[2] https://github.com/akheron/jansson/pull/389

As long as we don't impact any major, currently supported
distributions that may use earlier versions than gcc-4.1.2, I'm ok
with moving things forward as suggested.  Especially since this is
isolated to master/proposed-16.

Anybody else have any thoughts?




--
_
-- 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] Which enviroments are supported, really?

[Corey Farrell]

If you are looking to work with FreeBSD I'd suggest starting at 
FreshPorts [1].  Someone has done the work to get Asterisk working on 
FreeBSD and they've got Asterisk 13.19.0 (they're keeping it current).


[1] https://www.freshports.org/net/asterisk13/


On 01/26/2018 06:41 AM, Alexander Traud wrote:

Testing FreeBSD poses other problems however. None of us really work
with BSD based distributions so it would take more time that we have
available to do any serious testing there.

Can you give an example of those anticipated problems?

Greater differences allow deeper learning. Especially, OpenBSD and FreeBSD look 
like promising candidates with their different default shells and compilers. If 
you have a look at my reported issues, those were just copy-and-paste mistakes, 
slipped through errors, and wrong assumptions; trivial things. But big enough 
to be a show-stopper for a novice user.

I am not about Continues Integration. I am about a manual test after a major 
release (for example before the xx.2 release) on platforms which should do, but 
which were not tested. I am not about running the whole Test Suite but just 
about installing Asterisk and running it once (to double-check there are no 
loader issues and no false errors). That took me seconds (after updating the 
dependencies lists for those platforms in the script install_prereq).

Then, one goes through the findings. As described above, often easy to fix, 
especially for the one who introduced that code change. Ideally the remaining 
issues are reported on Jira, with a note that those issues must be fixed by the 
community. Done.

Integrators and maintainers on those platforms can then use that information as 
starting point while creating their patches. That should even ease their 
contribution back upstream.






--
_
-- 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] Compiler feature requirements

[Corey Farrell]

I've posted a patch for this [1] so anyone who wants to test the 
proposed changes can.  This is really a question about non-GCC compilers 
since we already know which versions of GCC work.  Note that we cannot 
use __sync_fetch_and_nand as the original implementation in 4.1 was 
broken.  If we need __sync_fetch_and_nand or __sync_nand_and_fetch we 
would need to require gcc-4.4.  I've looked into the ast_flags macro's, 
implementing them with atomic operations would would not require nand 
functions.


[1] https://gerrit.asterisk.org/8049


On 01/24/2018 04:50 PM, Corey Farrell wrote:
I've posted ASTERISK-27619 [1] proposing that we drop support for GCC 
versions older than 4.1.2.  Specifically we'd be requiring that either 
__sync or __atomic builtin functions be available (I'm unsure what 
this will do to clang requirements).  gcc-4.1.2 was released in 
February 2007 and was the version provided by CentOS 5.  I've posted a 
PR to the jansson project [2] which will make reference counting 
thread safe, but I'm getting push-back on the parts needed to provide 
a replacement function for old compilers. Since reference counting in 
jansson was never thread safe before I think they'd rather just leave 
it as is for old compilers.


Obviously this proposal is for Asterisk 16+ only.  Does this matter to 
any distributions that will be supported beyond this October?


[1] https://issues.asterisk.org/jira/browse/ASTERISK-27619
[2] https://github.com/akheron/jansson/pull/389



--
_
-- 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] Compiler feature requirements

[Corey Farrell]

I've posted ASTERISK-27619 [1] proposing that we drop support for GCC 
versions older than 4.1.2.  Specifically we'd be requiring that either 
__sync or __atomic builtin functions be available (I'm unsure what this 
will do to clang requirements).  gcc-4.1.2 was released in February 2007 
and was the version provided by CentOS 5.  I've posted a PR to the 
jansson project [2] which will make reference counting thread safe, but 
I'm getting push-back on the parts needed to provide a replacement 
function for old compilers.  Since reference counting in jansson was 
never thread safe before I think they'd rather just leave it as is for 
old compilers.


Obviously this proposal is for Asterisk 16+ only.  Does this matter to 
any distributions that will be supported beyond this October?


[1] https://issues.asterisk.org/jira/browse/ASTERISK-27619
[2] https://github.com/akheron/jansson/pull/389

--
_
-- 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] Which enviroments are supported, really?

[Corey Farrell]

The only wiki page I found on non-Linux is [1].  Probably the clearest 
statement is in the README.md [2].  I think we should probably add a 
more negative spin to the comments about Other platforms, at minimum 
state that they are not actively tested and may break at any time.


As for jenkins2.asterisk.org, only Linux is tested.  The active build 
agents are:


 * CentOS 7 (x86 and x86_64)
 * Fedora 25 (x86_64)
 * Ubuntu 16 (x86_64)

Ubuntu 14 (x86_64 and x86) is listed in CI but they have been offline 
for a while so they aren't currently tested automatically.  I've 
recently made a change which caused CentOS 6 to stop building, I got a 
report as soon as the next RC was released.  I'm confused by your 
statement that things don't work in Fedora unless you're trying to 
compile with LLVM or clang (ASTERISK-26205).  All active versions of 
Fedora should work, I'm guessing many inactive (end of life) versions of 
Fedora will also work.  For OSX I've made some changes recently to get 
compile working, never went beyond that.  My goal was simply to be able 
to run 'make && git review' from OSX, I've never attempted to run the 
binaries.


GNU bash is required by some scripts (they have #!/bin/bash). Any 
scripts that have '#!/bin/sh' should not use bash only features but I'm 
not sure if this is ever tested.  I'm not aware of any continuous 
testing done with compilers other than GCC.


[1] https://wiki.asterisk.org/wiki/display/AST/Asterisk+on+%28Open%29Solaris
[2] https://github.com/asterisk/asterisk/#supported-operating-systems

On 01/21/2018 08:58 AM, Alexander Traud wrote:

Recently, I was hit by a missing dependency of an external library 
(ASTERISK-27475). Because I was not able to resolve the issue otherwise, I 
re-visited the first-time experience of Asterisk, thinking that should solve my 
issue for sure.

wget downloads.asterisk.org/pub/telephony/asterisk/asterisk-13-current.tar.gz
tar -zxf asterisk-*
cd asterisk-*
sudo ./contrib/scripts/install_prereq install
./configure --with-pjproject-bundled
make
sudo make install samples config
sudo asterisk -c

Those eight steps are the path for a novice user to get Asterisk going. 
Consequently, those steps have to succeed in a supported environment, without a 
single error and should not give warnings.

In OpenBSD, FreeBSD, and Fedora - with the current release (Asterisk 13.19 or 
15.2) - these steps lead into errors. Asterisk is not even built! Furthermore, 
external libraries are not found by the script configure, the wrong ones are 
installed, and (even essential) ones are missing. Make errors out, PJProject 
and Asterisk are not built. Even when all issues are fixed or circumvented, the 
command-line interface of Asterisk prints not only warnings but several errors 
in such a standard installation.

I contributed back many of my fixes to the current branches already. So the 
next releases should give a better out-of-the-box experience. However, all 
those issues could have been found by static testing, just executing the above 
commands on a virtual machine. Looking at the amount of issues, I asked myself 
whether I am doing something wrong. Especially as diagnosis revealed that some 
issues were 18 months old. Therefore: Which environments (platform, shell, and 
compiler) are really supported by the Asterisk team (has to work) and what is 
community contributed (should work)?

shell: GNU bash
compiler: GCC
platform: CentOS 7

That is used by the continuous integration machine(s) and FreePBX currently. 
What else has to work? I do not earn a penny because of Asterisk. I do not want 
to waste time with a so lala supported environment. There must be a wiki entry, 
blog post, video, or mail summarizing the environments. For some reason, I was 
not able to find it, yet.





-- 
_
-- 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] Help understand TCP/TLS server error

[Corey Farrell]

I'd suggest opening a ticket at https://issues.asterisk.org, include 
full debug logs and minimal test case for reproducing the issue. See 
https://wiki.asterisk.org/wiki/display/AST/Collecting+Debug+Information 
for details about what to post on the new ticket.


Just to ask have you tested chan_pjsip?  Not saying this issue shouldn't 
be fixed but the chan_sip doxygen and sample sip.conf state that 
chan_sip TCP/TLS support is experimental.  Given the support status of 
chan_sip combined with the declared state of the SIP over TCP feature I 
would not use it (but that's just my choice, not trying to tell you what 
to do).


One concern is if you are using TCP to communicate with an external 
service provider it's possible this could inflate billing.  When does 
the provider recognize the call as ending if you don't send a BYE?  
Probably not a big issue when the remote side hangs up too but you can't 
always count on that.



On 01/19/2018 04:03 PM, Chris Jones wrote:


Hello all,

Can I get a little help to understand why I am receiving this error? 
From a developer perspective, what Asterisk conditions would cause 
this error to trigger?


At exactly 120 seconds after an ongoing call is setup, this pops up in 
the console with heavy debugging enabled:


DEBUG[30015]: iostream.c:157 iostream_read: TLS clean shutdown alert 
reading data


DEBUG[30015]: chan_sip.c:2905 sip_tcptls_read: SIP TCP/TLS server has 
shut down


It doesn’t appear to have a negative effect on the ongoing call; 
however, it appears to be keeping Asterisk from sending a BYE message 
to the SIP provider at the conclusion of the call.


I have lots and lots of more details if you ask; however, I just need 
a nudge in the right direction. v15.1.3, though I have used a version 
in 13 and 14 train and had the same problem.


Cheers,

Chris





-- 
_
-- 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] QueueLog Linkedid

[Corey Farrell]

I'd be interested in what Loway would have to say about this (or any 
other reporting systems).  ATTENDEDTRANSFER already uses 4 data fields, 
so I don't have any issue if CONNECT sets the 4th data field to 
linkedid.  I don't see this as a format change, if we were talking about 
adding a 5th data field I'd feel be differently (since no verb has 5 
data fields).



On 01/12/2018 04:11 PM, Matt Fredrickson wrote:

On Thu, Jan 11, 2018 at 6:00 AM, marek cervenka  wrote:

hi,

because of some call analytics we need linkedid in queue_log

if we create patch for it (app_queue), will you accept it?

My preferences are that any changes to the queue log format would be
done in master only.  That being said, tests are always welcome
(master or not).  The queue applications is one of the areas of code
that are full of traps, partially due to the fact that we don't have
much testing around it.

Anybody else have any thoughts about this?




--
_
-- 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] tags on the main branches [was: Re: Minor Release Branches]

[Corey Farrell]

On 12/22/2017 12:36 PM, George Joseph wrote:



On Fri, Dec 22, 2017 at 9:50 AM, Corey Farrell <g...@cfware.com 
<mailto:g...@cfware.com>> wrote:


On 12/22/2017 10:22 AM, George Joseph wrote:



On Thu, Dec 21, 2017 at 1:44 PM, Corey Farrell <g...@cfware.com
<mailto:g...@cfware.com>> wrote:

George asked that I post some scenarios where this would be
useful.

1. You are about to create updated asterisk package and want
to quickly scan the changes to 13 since latest 13.x.0 release
to see if anything is a 'must patch' for your deployments. 
You can use 'tig' to review changes for critical fixes until
you reach the tag '13.x.0-rc1' (which you can see in the list
because the tag was merged).


I didn't even know about tig.  There's always something to learn
about the git ecosystem. :)

I know the end result can be accomplished by other means, but
not as easily.

2. You've created an automated test to try finding a
performance regression. The test runs asterisk under a
profiler and stores results.  Each revision you test needs a
new file to store results - 'git describe' would provide an
excellent filename that is unique per revision.

One thing I'm not sure about is if we should only merge rc1
tags or if we should merge all new release tags.  At first
all release tags seem reasonable, but the order of tags other
than rc1 would be off.  rc1 is special because it would be
merged back to mainline before anything else.  Even 13.x.0
does not get cut until after other commits are merged to 13,
so if we merged 13.19.0 to 13, the commits made since
13.19.0-rc1 would appear out of order (before 13.19.0). The
difference between rc1 and final release is always small, but
the number of new commits to mainline between that time can
be quite large.

So we would merge rc1's back to mainline but how about the point
releases?
13.19.0-rc1
13.19.1  ??
13.19.2  ??

13.18-cert1-rc1
13.18-cert2 ??
13.18-cert3 ??


Just to be clear certified branches would be untouched by this
proposal.  Look at 'git log certified/13.13-cert9' - you will see
the previous tagged releases in the certified/13.13 release series.

I think we should not do anything different with the point
releases (including 13.19.0) because of the commit order.  Think
about when 13.18.4 were released.  If we merged it to 13 it would
be in the wrong place on 'git log 13'.  Easily 200 non-merge
commits would appear after the 13.18.4 tag in the 13 branch, when
in fact they are not part of the 13.18.4 tag.  My hope is that we
can provide additional information, but only if the information is
accurate.


Sorry, I'm being thick...  So if it's only the rc1's then why won't an 
annotated tag do what you want without having to alter the mainline 
commit history with a merge?


The whole point is for an annotated tag to be in the mainline commit 
history.  This way when you do 'tig 15' it will show where each release 
was split from mainline (the ones we merge in the future that is).



One last detail I don't know exactly how we deal with new major
releases (ie 16.0.0).  If I remember correctly we will release
16.0.0-beta1, but I don't remember if that is the start of the
16.0 branch or if 16.0 starts with rc1.  However it works my
current proposal would be to merge the first commit of 16.0 back
to 16.


we manually cut the 16 branch from master before anything then the 
release process creates 16.0 when we release beta1.


In that case we would only merge 16.0.0-beta1 back to 16 (the split 
point), we would not merge 16.0.0-rc1 since it would be behind 16 at the 
time of creation.




Tzafrir I haven't heard from you since I joined your new thread. 
If we were to merge the rc1's back to mainline so that mainline
knew about the "split point", would this satisfy your request? 
Also was your intent to say "we should do this first before
deleting minor branches"?  Do you object if we proceed with
removal of minor branches or does your request need to be
completed first?





    On 12/21/2017 10:45 AM, Corey Farrell wrote:

I just read `git help merge` again and I think the solution
is 'git checkout 13 && git merge --strategy ours
13.19.0-rc1'.  This would effectively tell git that '13'
already contains 13.19.0-rc1, but without actually trying to
pull any changes to 13.  This merge would be the final step
of mkrelease.py.

No changes will be needed to our handling of '.lastclean',
please ignore those comments as I was wrong.

On 12/21/2017 08:19 AM, George Joseph wrote:



    On Wed, Dec 20, 201

Re: [asterisk-dev] Adding a Key/Value Store mechanism to Asterisk

[Corey Farrell]

I hope we consider creating a res_redis first, then base everything off 
that.  If a redis library can be used directly by any module that would 
fine but I'd like to see us avoid following the example of curl where 
everything uses a dialplan function to perform requests.  Dialplan 
functions should be for dialplan, in general I think they should not be 
used as internal API's.



On 12/22/2017 12:23 PM, Nir Simionovich wrote:


Well,

We can start with that implementation as a base for learning, and go 
from there. Looks like I have some homework for tonight.


Nir


On Fri, Dec 22, 2017, 18:44 Matt Fredrickson > wrote:


On Fri, Dec 22, 2017 at 10:23 AM, Ivan Poddubny
> wrote:

Hi,

There is an out-of-tree implementation of func_redis:
https://github.com/tic-ull/func_redis.
I don't use it in production, but it worked fine for me on a
test machine.
With slight modifications it works with Asterisk 13+.
Unfortunately, the project seems to be abandoned and the
author did not try to merge the code upstream.


Yeah, unless he submits it and goes through that process himself,
we can't do a lot with it in the mainline Asterisk codebase.

Matthew Fredrickson


On 22 December 2017 at 16:54, Matt Fredrickson
> wrote:



On Fri, Dec 22, 2017 at 6:58 AM, Nir Simionovich
> wrote:

Abhay,

Migrating astsb from SQLlite to redis isn't the topic
here. I'm talking adding a new type of storage engine.
For example, func_redis, that will implement the
relevant key/value operations that are commonly used
by people.


I think doing it as func_redis instead of a sorcery
backend is a good idea.

I'm guessing there are a lot of people that can use it. 
It seems like having access to a redis like backend is a
modern requirement for most big systems.

Matthew Fredrickson

Nir


On Fri, Dec 22, 2017, 14:33 Abhay Gupta
> wrote:

Hi All,

I had a program where I have implemented a project
using REDIS wherein the client is made using a
socket library and no other third party client
library in C .

This REDIS database has 400 million records and
performs extremely well though the memory
requirement for such a large dataset goes to 48GB
. So I strongly believe that for such key value
pair REDIS will be the right choice for ASTDB.

Regards,

Abhay


On 22-Dec-2017, at 5:52 PM, Nir Simionovich
> wrote:

Hi All,

Following a discussion on JIRA
[https://issues.asterisk.org/jira/browse/ASTERISK-27383],
I truly believe that
adding a scaleable, robust and most importantly -
accepted key/value store mechanism to the
Asterisk dialplan
is a worthwhile effort.

  Every, and I do mean every, Asterisk
application requires a key/value store of some
form. Most developers will
basically butcher (would have used stronger
words, but refraining from doing so) AstDB in the
process, which will
then result in a performance toll - specifically
when dealing with a high capacity systems.

Initially, I was under the impression this should
be done as a sorcery module, but I'm not sure
this is the
correct approach or the required use case.

  I would like to hear if others believe this is
a worth while effort? if others believe it is,
I'll be ecstatic to
work with others on this one (adding Redis
support isn't as simple as it sounds). However,
before I start
working on something, I'd like to see if others
believe this as strongly as I do.

  On the same note, I'll be in NYC second week of
January - so if any of you are 

Re: [asterisk-dev] tags on the main branches [was: Re: Minor Release Branches]

[Corey Farrell]

On 12/22/2017 10:22 AM, George Joseph wrote:



On Thu, Dec 21, 2017 at 1:44 PM, Corey Farrell <g...@cfware.com 
<mailto:g...@cfware.com>> wrote:


George asked that I post some scenarios where this would be useful.

1. You are about to create updated asterisk package and want to
quickly scan the changes to 13 since latest 13.x.0 release to see
if anything is a 'must patch' for your deployments.  You can use
'tig' to review changes for critical fixes until you reach the tag
'13.x.0-rc1' (which you can see in the list because the tag was
merged).


I didn't even know about tig.  There's always something to learn about 
the git ecosystem. :)


I know the end result can be accomplished by other means, but not
as easily.

2. You've created an automated test to try finding a performance
regression.  The test runs asterisk under a profiler and stores
results.  Each revision you test needs a new file to store results
- 'git describe' would provide an excellent filename that is
unique per revision.

One thing I'm not sure about is if we should only merge rc1 tags
or if we should merge all new release tags.  At first all release
tags seem reasonable, but the order of tags other than rc1 would
be off.  rc1 is special because it would be merged back to
mainline before anything else.  Even 13.x.0 does not get cut until
after other commits are merged to 13, so if we merged 13.19.0 to
13, the commits made since 13.19.0-rc1 would appear out of order
(before 13.19.0).  The difference between rc1 and final release is
always small, but the number of new commits to mainline between
that time can be quite large.

So we would merge rc1's back to mainline but how about the point releases?
13.19.0-rc1
13.19.1  ??
13.19.2  ??

13.18-cert1-rc1
13.18-cert2 ??
13.18-cert3 ??


Just to be clear certified branches would be untouched by this 
proposal.  Look at 'git log certified/13.13-cert9' - you will see the 
previous tagged releases in the certified/13.13 release series.


I think we should not do anything different with the point releases 
(including 13.19.0) because of the commit order.  Think about when 
13.18.4 were released.  If we merged it to 13 it would be in the wrong 
place on 'git log 13'.  Easily 200 non-merge commits would appear after 
the 13.18.4 tag in the 13 branch, when in fact they are not part of the 
13.18.4 tag.  My hope is that we can provide additional information, but 
only if the information is accurate.


One last detail I don't know exactly how we deal with new major releases 
(ie 16.0.0).  If I remember correctly we will release 16.0.0-beta1, but 
I don't remember if that is the start of the 16.0 branch or if 16.0 
starts with rc1.  However it works my current proposal would be to merge 
the first commit of 16.0 back to 16.


Tzafrir I haven't heard from you since I joined your new thread.  If we 
were to merge the rc1's back to mainline so that mainline knew about the 
"split point", would this satisfy your request?  Also was your intent to 
say "we should do this first before deleting minor branches"?  Do you 
object if we proceed with removal of minor branches or does your request 
need to be completed first?





    On 12/21/2017 10:45 AM, Corey Farrell wrote:

I just read `git help merge` again and I think the solution is
'git checkout 13 && git merge --strategy ours 13.19.0-rc1'.  This
would effectively tell git that '13' already contains
13.19.0-rc1, but without actually trying to pull any changes to
13.  This merge would be the final step of mkrelease.py.

No changes will be needed to our handling of '.lastclean', please
ignore those comments as I was wrong.

On 12/21/2017 08:19 AM, George Joseph wrote:



    On Wed, Dec 20, 2017 at 3:14 PM, Corey Farrell <g...@cfware.com
<mailto:g...@cfware.com>> wrote:

One thing that might improve this is if releases were merged
back to the major branch.  Currently the commit "Update for
13.19.0-rc1" is on the 13.19 branch and tagged as
13.19.0-rc1.  I believe that if we added 'git checkout 13 &&
git merge 13.19.0-rc1' we would get better information from
'git describe 13' and tags would appear in 'git log 13
--oneline'.  This would continue working even after we
delete the minor branches.

Sounds reasonable.

As a test I just ran 'git merge 13.18.4' from the current 13
branch.  The merge did have 2 conflicts but that's because
13.18 was branched so long ago and a couple files that were
modified in minor releases have since been modified again or
deleted.  Then I ran 'git describe 13', it said
'13.18.4-404-gd5d67bb1f4'.  This tells us that my local
branch had about 404 commits (including merges) that are not
part of 13.18

Re: [asterisk-dev] tags on the main branches [was: Re: Minor Release Branches]

[Corey Farrell]

George asked that I post some scenarios where this would be useful.

1. You are about to create updated asterisk package and want to quickly 
scan the changes to 13 since latest 13.x.0 release to see if anything is 
a 'must patch' for your deployments.  You can use 'tig' to review 
changes for critical fixes until you reach the tag '13.x.0-rc1' (which 
you can see in the list because the tag was merged).  I know the end 
result can be accomplished by other means, but not as easily.


2. You've created an automated test to try finding a performance 
regression.  The test runs asterisk under a profiler and stores 
results.  Each revision you test needs a new file to store results - 
'git describe' would provide an excellent filename that is unique per 
revision.


One thing I'm not sure about is if we should only merge rc1 tags or if 
we should merge all new release tags.  At first all release tags seem 
reasonable, but the order of tags other than rc1 would be off.  rc1 is 
special because it would be merged back to mainline before anything 
else.  Even 13.x.0 does not get cut until after other commits are merged 
to 13, so if we merged 13.19.0 to 13, the commits made since 13.19.0-rc1 
would appear out of order (before 13.19.0).  The difference between rc1 
and final release is always small, but the number of new commits to 
mainline between that time can be quite large.



On 12/21/2017 10:45 AM, Corey Farrell wrote:
I just read `git help merge` again and I think the solution is 'git 
checkout 13 && git merge --strategy ours 13.19.0-rc1'.  This would 
effectively tell git that '13' already contains 13.19.0-rc1, but 
without actually trying to pull any changes to 13.  This merge would 
be the final step of mkrelease.py.


No changes will be needed to our handling of '.lastclean', please 
ignore those comments as I was wrong.


On 12/21/2017 08:19 AM, George Joseph wrote:



On Wed, Dec 20, 2017 at 3:14 PM, Corey Farrell <g...@cfware.com 
<mailto:g...@cfware.com>> wrote:


One thing that might improve this is if releases were merged back
to the major branch.  Currently the commit "Update for
13.19.0-rc1" is on the 13.19 branch and tagged as 13.19.0-rc1.  I
believe that if we added 'git checkout 13 && git merge
13.19.0-rc1' we would get better information from 'git describe
13' and tags would appear in 'git log 13 --oneline'.  This would
continue working even after we delete the minor branches.

Sounds reasonable.

As a test I just ran 'git merge 13.18.4' from the current 13
branch.  The merge did have 2 conflicts but that's because 13.18
was branched so long ago and a couple files that were modified in
minor releases have since been modified again or deleted. Then I
ran 'git describe 13', it said '13.18.4-404-gd5d67bb1f4'.  This
tells us that my local branch had about 404 commits (including
merges) that are not part of 13.18.0-rc1 (the point where 13.18
diverged from 13 because 13.18.3 was not merged back).  Merging
each tag as soon as it's created would make the results more
accurate. and (almost always) eliminate merge conflicts.

"almost always" will be an issue since it's the scripts that do the 
work.  It's kinda frustrating already when you're trying to get 
releases out the door and something goes wrong with the script.  What 
conditions do you think might still cause merge conflicts?


The only wrinkle in this plan is that the '.lastclean' file is
created on the releases but it's listed in .gitignore.  I think
we might be able to just get rid of the .lastclean and
.cleancount files.  This Makefile hack predates the use of SVN
and I don't think it's necessary.  One thing it does do is try to
enable the astdb2sqlite3 utility, but Berkely DB was last used in
Asterisk 1.8.  The default is for that utility to be enabled,
that's enough.  In addition the mkrelease script actually copies
.cleancount to .lastclean, I think that means it's disabled for
releases.

These kind of things we can alter to suite our needs so there 
shouldn't be an issue.



On 12/20/2017 12:58 PM, George Joseph wrote:



On Wed, Dec 20, 2017 at 8:14 AM, Tzafrir Cohen
<tzafrir.co...@xorcom.com <mailto:tzafrir.co...@xorcom.com>> wrote:

On Wed, Dec 20, 2017 at 07:50:03AM -0700, George Joseph wrote:
> On Wed, Dec 20, 2017 at 2:44 AM, Tzafrir Cohen
<tzafrir.co...@xorcom.com <mailto:tzafrir.co...@xorcom.com>>
> wrote:
>
> > Off-topic:
> >
> > On Tue, Dec 19, 2017 at 01:50:03PM -0700, George Joseph
wrote:
> >
> > > Thankfully we tag EVERYTHING! :)
> >
> > asterisk(13)$ git describe
> > 13.15.0-rc1-908-ge31e3b581b
> >
> > asterisk(14)$ git de

Re: [asterisk-dev] tags on the main branches [was: Re: Minor Release Branches]

[Corey Farrell]

I just read `git help merge` again and I think the solution is 'git 
checkout 13 && git merge --strategy ours 13.19.0-rc1'.  This would 
effectively tell git that '13' already contains 13.19.0-rc1, but without 
actually trying to pull any changes to 13.  This merge would be the 
final step of mkrelease.py.


No changes will be needed to our handling of '.lastclean', please ignore 
those comments as I was wrong.


On 12/21/2017 08:19 AM, George Joseph wrote:



On Wed, Dec 20, 2017 at 3:14 PM, Corey Farrell <g...@cfware.com 
<mailto:g...@cfware.com>> wrote:


One thing that might improve this is if releases were merged back
to the major branch.  Currently the commit "Update for
13.19.0-rc1" is on the 13.19 branch and tagged as 13.19.0-rc1.  I
believe that if we added 'git checkout 13 && git merge
13.19.0-rc1' we would get better information from 'git describe
13' and tags would appear in 'git log 13 --oneline'.  This would
continue working even after we delete the minor branches.

Sounds reasonable.

As a test I just ran 'git merge 13.18.4' from the current 13
branch.  The merge did have 2 conflicts but that's because 13.18
was branched so long ago and a couple files that were modified in
minor releases have since been modified again or deleted.  Then I
ran 'git describe 13', it said '13.18.4-404-gd5d67bb1f4'.  This
tells us that my local branch had about 404 commits (including
merges) that are not part of 13.18.0-rc1 (the point where 13.18
diverged from 13 because 13.18.3 was not merged back).  Merging
each tag as soon as it's created would make the results more
accurate. and (almost always) eliminate merge conflicts.

"almost always" will be an issue since it's the scripts that do the 
work.  It's kinda frustrating already when you're trying to get 
releases out the door and something goes wrong with the script.  What 
conditions do you think might still cause merge conflicts?


The only wrinkle in this plan is that the '.lastclean' file is
created on the releases but it's listed in .gitignore.  I think we
might be able to just get rid of the .lastclean and .cleancount
files. This Makefile hack predates the use of SVN and I don't
think it's necessary.  One thing it does do is try to enable the
astdb2sqlite3 utility, but Berkely DB was last used in Asterisk
1.8.  The default is for that utility to be enabled, that's
enough.  In addition the mkrelease script actually copies
.cleancount to .lastclean, I think that means it's disabled for
releases.

These kind of things we can alter to suite our needs so there 
shouldn't be an issue.



On 12/20/2017 12:58 PM, George Joseph wrote:



On Wed, Dec 20, 2017 at 8:14 AM, Tzafrir Cohen
<tzafrir.co...@xorcom.com <mailto:tzafrir.co...@xorcom.com>> wrote:

On Wed, Dec 20, 2017 at 07:50:03AM -0700, George Joseph wrote:
> On Wed, Dec 20, 2017 at 2:44 AM, Tzafrir Cohen
<tzafrir.co...@xorcom.com <mailto:tzafrir.co...@xorcom.com>>
> wrote:
>
> > Off-topic:
> >
> > On Tue, Dec 19, 2017 at 01:50:03PM -0700, George Joseph
wrote:
> >
> > > Thankfully we tag EVERYTHING! :)
> >
> > asterisk(13)$ git describe
> > 13.15.0-rc1-908-ge31e3b581b
> >
> > asterisk(14)$ git describe
> > fatal: No tags can describe
'fb18797ae09a685ec71101499fb1c1c606b16397'.
> > Try --always, or create some tags.
> >
> > asterisk(15)$ git describe
> > fatal: No tags can describe
'd312068ee93ff8ce97b464f3c2ff3304e15cb3fe'.
> > Try --always, or create some tags.
> >
> >
> > I wasted half an hour yesterday trying to find out why a
build sis not
> > switch from master to 13, only to realize that the name
of the git
> > branch in the version string is always "master".
> >
> > We tag everything. But only well after it was branched
from the main
> >
> branch.
> >
>
> I'm not following you.
>
> We tag every release...
>
> $ git checkout 13.18.4
> HEAD is now at f4644317b7... Update for 13.18.4
> $ git describe
> 13.18.4


> $ git checkout 13.18
> Switched to branch '13.18'
> Your branch is up-to-date with 'gerrit/13.18'.
> $ git describe
> 13.18.4


> $
>
> We have to create the minor release branch (13.18) and do
the work there so
> that patch releases (13.18

Re: [asterisk-dev] tags on the main branches [was: Re: Minor Release Branches]

[Corey Farrell]

One thing that might improve this is if releases were merged back to the 
major branch.  Currently the commit "Update for 13.19.0-rc1" is on the 
13.19 branch and tagged as 13.19.0-rc1.  I believe that if we added 'git 
checkout 13 && git merge 13.19.0-rc1' we would get better information 
from 'git describe 13' and tags would appear in 'git log 13 --oneline'.  
This would continue working even after we delete the minor branches.


As a test I just ran 'git merge 13.18.4' from the current 13 branch.  
The merge did have 2 conflicts but that's because 13.18 was branched so 
long ago and a couple files that were modified in minor releases have 
since been modified again or deleted.  Then I ran 'git describe 13', it 
said '13.18.4-404-gd5d67bb1f4'.  This tells us that my local branch had 
about 404 commits (including merges) that are not part of 13.18.0-rc1 
(the point where 13.18 diverged from 13 because 13.18.3 was not merged 
back).  Merging each tag as soon as it's created would make the results 
more accurate. and (almost always) eliminate merge conflicts.


The only wrinkle in this plan is that the '.lastclean' file is created 
on the releases but it's listed in .gitignore.  I think we might be able 
to just get rid of the .lastclean and .cleancount files.  This Makefile 
hack predates the use of SVN and I don't think it's necessary.  One 
thing it does do is try to enable the astdb2sqlite3 utility, but Berkely 
DB was last used in Asterisk 1.8.  The default is for that utility to be 
enabled, that's enough.  In addition the mkrelease script actually 
copies .cleancount to .lastclean, I think that means it's disabled for 
releases.



On 12/20/2017 12:58 PM, George Joseph wrote:



On Wed, Dec 20, 2017 at 8:14 AM, Tzafrir Cohen 
> wrote:


On Wed, Dec 20, 2017 at 07:50:03AM -0700, George Joseph wrote:
> On Wed, Dec 20, 2017 at 2:44 AM, Tzafrir Cohen
>
> wrote:
>
> > Off-topic:
> >
> > On Tue, Dec 19, 2017 at 01:50:03PM -0700, George Joseph wrote:
> >
> > > Thankfully we tag EVERYTHING! :)
> >
> > asterisk(13)$ git describe
> > 13.15.0-rc1-908-ge31e3b581b
> >
> > asterisk(14)$ git describe
> > fatal: No tags can describe
'fb18797ae09a685ec71101499fb1c1c606b16397'.
> > Try --always, or create some tags.
> >
> > asterisk(15)$ git describe
> > fatal: No tags can describe
'd312068ee93ff8ce97b464f3c2ff3304e15cb3fe'.
> > Try --always, or create some tags.
> >
> >
> > I wasted half an hour yesterday trying to find out why a build
sis not
> > switch from master to 13, only to realize that the name of the git
> > branch in the version string is always "master".
> >
> > We tag everything. But only well after it was branched from
the main
> >
> branch.
> >
>
> I'm not following you.
>
> We tag every release...
>
> $ git checkout 13.18.4
> HEAD is now at f4644317b7... Update for 13.18.4
> $ git describe
> 13.18.4


> $ git checkout 13.18
> Switched to branch '13.18'
> Your branch is up-to-date with 'gerrit/13.18'.
> $ git describe
> 13.18.4


> $
>
> We have to create the minor release branch (13.18) and do the
work there so
> that patch releases (13.18.4) are based on the minor release
branch, not
> the major branch.

Those branches are likewise short-lived branches (at least with
respect
to the number of commits). Real work is done on master, 13, 15 and
such.
But when I'm on such a branch, I can't ask git on which branch I
am (not
to mention: at which stage in it).


 I _think_ I understand now.


For instance: maybe whenever you tag a new release branch (e.g.
13.18),
tag the split point as something like "13.18.base" or "base.13.18"?


Well, that's easy enough.  Toss us an issue for it.


But maybe it's just me and branches 13 and 15 are not widely used (for
master it is irrelevant anyway).

--
               Tzafrir Cohen
+972-50-7952406           mailto:tzafrir.co...@xorcom.com

http://www.xorcom.com

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





--
George Joseph
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com  & 
www.asterisk.org 






-- 
_
-- Bandwidth and Colocation Provided by 

[asterisk-dev] Minor Release Branches

[Corey Farrell]

I'd like to propose that we make minor release branches temporary.  What 
I mean is 13.15, 15.0, etc.  The sole purpose of the '13.15' was to 
allow cherry-picking fixes to the 13.15.x series.  As soon as 13.16.0 
was released the '13.15' branch was closed to new merges.  The latest 
commit on '13.15' is tagged as '13.15.1' so we would not be orphaning 
any commits by doing this. When you click "Cherry-pick" from the gerrit 
web ui and type "15", the selection list shows "13.15" first (and 
selects it by default).


Anyone have thoughts on this?


--
_
-- 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] Branching in the Testsuite

[Corey Farrell]

Sounds good.  So after this is done am I correct that when my review 
against asterisk 13 is approved jenkins will 'gate' using the 13 branch 
of testsuite?  Will the testsuite also get minor branches (like 13.19)?



On 12/15/2017 03:24 PM, George Joseph wrote:



On Fri, Dec 15, 2017 at 10:56 AM, Corey Farrell <g...@cfware.com 
<mailto:g...@cfware.com>> wrote:


It would be nice to strip out some/most of the per version
conditionals in tests.  The need to always cherry-pick changes is
the only pitfall I see with your proposal.  If a test never had
per version differences then the cherry-pick is trivial amount of
extra work, but for tests with differences per branch it would
mean dealing with conflicts.  In the testsuite I think I'd rather
deal with per branch conditionals over merge with conflicts.

Given the low commit volume, even lower commit volume against existing 
tests, high compartmentalization and small nature of the files in the 
testsuite,  I don't see conflicts being a big thing.  If there are 
any, then they're probably justified.


I just want to propose an alternative way of (mostly)
accomplishing the goal.  We could create a tag '12' from current
master.  This would represent the last revision of the testsuite
known to work with Asterisk 12 and below.  We would then be free
to remove compatibility with EOL Asterisk branches from testsuite
master.  We would tag testsuite '14' soon after September 26th,
2018 (EOL for Asterisk 14). This would avoid multiplying the
number of gerrit reviews for testsuite changes, but it would
require continuing to maintain version declarations for supported
versions of Asterisk.  I think this would be a good trade-off so
we aren't stuck with all the 1.8/11 baggage.

We were thinking of taking the current master, copying it to 13, 14, 
15, and "legacy" then starting with the next release our release 
scripts would automatically create tags in the testsuite just as they 
do in asterisk.


This also makes Jenkins much happier BTW.



On 12/15/2017 11:59 AM, Kevin Harwell wrote:

Greetings,

We're thinking about adding a branching system to the Asterisk
Testsuite. Each branch would be named the same as, and correspond
to, an Asterisk branch. So for instance the following branches
would probably be created:

13, 14, 15

For each release of Asterisk we will also create a tag in the
Testsuite that corresponds to that release's tag. That way
someone could checkout both tags for easy testing

Other advantages? Most all, if not all, the current versioning
stuff found in the Testsuite could go away, or be safely ignored
moving forward. The versioning has become a bit cumbersome
especially when you have to make a backward incompatible change
to a test. Moving the version control out of the Testsuite and
into a version control system should alleviate the need for this
moving forward.

Please let us know your thoughts and considerations on moving
forward with this model. Especially any potential pitfalls or
problems you might see with it.

Thanks!

-- 
Kevin Harwell

Digium, Inc. | Software Developer
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
<http://lists.digium.com/mailman/listinfo/asterisk-dev>




--
George Joseph
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com <http://www.digium.com/> & 
www.asterisk.org <http://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] Branching in the Testsuite

[Corey Farrell]

It would be nice to strip out some/most of the per version conditionals 
in tests.  The need to always cherry-pick changes is the only pitfall I 
see with your proposal.  If a test never had per version differences 
then the cherry-pick is trivial amount of extra work, but for tests with 
differences per branch it would mean dealing with conflicts.  In the 
testsuite I think I'd rather deal with per branch conditionals over 
merge with conflicts.


I just want to propose an alternative way of (mostly) accomplishing the 
goal.  We could create a tag '12' from current master.  This would 
represent the last revision of the testsuite known to work with Asterisk 
12 and below.  We would then be free to remove compatibility with EOL 
Asterisk branches from testsuite master.  We would tag testsuite '14' 
soon after September 26th, 2018 (EOL for Asterisk 14).  This would avoid 
multiplying the number of gerrit reviews for testsuite changes, but it 
would require continuing to maintain version declarations for supported 
versions of Asterisk.  I think this would be a good trade-off so we 
aren't stuck with all the 1.8/11 baggage.



On 12/15/2017 11:59 AM, Kevin Harwell wrote:

Greetings,

We're thinking about adding a branching system to the Asterisk 
Testsuite. Each branch would be named the same as, and correspond to, 
an Asterisk branch. So for instance the following branches would 
probably be created:


13, 14, 15

For each release of Asterisk we will also create a tag in the 
Testsuite that corresponds to that release's tag. That way someone 
could checkout both tags for easy testing


Other advantages? Most all, if not all, the current versioning stuff 
found in the Testsuite could go away, or be safely ignored moving 
forward. The versioning has become a bit cumbersome especially when 
you have to make a backward incompatible change to a test. Moving the 
version control out of the Testsuite and into a version control system 
should alleviate the need for this moving forward.


Please let us know your thoughts and considerations on moving forward 
with this model. Especially any potential pitfalls or problems you 
might see with it.


Thanks!

--
Kevin Harwell
Digium, Inc. | Software Developer
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] Can someone please review something

[Corey Farrell]

From your configure.ac:
> AST_EXT_LIB_CHECK([REDIS], [redis], [redisReaderCreate], 
[hiredis/hiredis.h])


This tries linking with '-lredis', but it needs to use '-lhiredis':

AST_EXT_LIB_CHECK([REDIS], [hiredis], [redisReaderCreate], 
[hiredis/hiredis.h])


In addition instead of using variable prefix "REDIS" (the first 
argument) I suggest switching to "HIREDIS" (don't forget makeopts.in and 
build_tools/menuselect-deps.in).  There are multiple C redis libraries 
so you want the variable name to identify which one you are using.  This 
will also make it match "hiredis" in cdr_redis.c so 
menuselect will enable the module.


Since redis is in-memory I'm not really sure about using it for CDR?  I 
could see res_sorcery_redis being useful assuming it could be used as an 
alternative to res_sorcery_astdb or res_sorcery_memory.



On 11/22/2017 06:01 PM, Nir Simionovich wrote:

Hi All,

  I've started a new branch at team/nirs/cdr-redis-support

  I'm having some issues integrating the hiredis library into the 
automake. It seems to be configured correctly,
however, for some odd reason it will not become available in the 
'menuselect' tool.


  Would highly appreciate it if someone can take a look for a minute 
and see if I missed anything major in there.



--

Kind Regards,

  Nir Simionovich

GreenfieldTech

(schedule) http://nirsimionovich.appointy.com/

  (w)http://www.greenfieldtech.net 

  (p) +972-73-2557799 (MSN):n...@greenfieldtech.net 



  (m) +972-54-6982826 (GTALK):nir.simionov...@gmail.com 



  (f) +972-73-2557202 (SKYPE): greenfieldtech.nir


--

Zero Your Inbox  | Cloud Servers 



--


*Disclaimer:*

This e-mail is intended solely for the person to whom it is addressed 
and may contain confidential or legally privileged information. Access 
to this e-mail by anyone else is unauthorized. If an addressing or 
transmission error has misdirected this e-mail, please notify the 
author by replying to this e-mail and destroy this e-mail and any 
attachments.
E-mail may be susceptible to data corruption, interception, 
unauthorized amendment, viruses and delays or the consequences 
thereof. If you are not the intended recipient, be advised that you 
have received this email in error and that any use, dissemination, 
forwarding, printing or copying of this email is strictly prohibited.






-- 
_
-- 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] Can someone please review something

[Corey Farrell]

Where did you push this branch?  I'm not seeing it on gerrit or github.


On 11/22/2017 06:01 PM, Nir Simionovich wrote:

Hi All,

  I've started a new branch at team/nirs/cdr-redis-support

  I'm having some issues integrating the hiredis library into the 
automake. It seems to be configured correctly,
however, for some odd reason it will not become available in the 
'menuselect' tool.


  Would highly appreciate it if someone can take a look for a minute 
and see if I missed anything major in there.



--

Kind Regards,

  Nir Simionovich

GreenfieldTech

(schedule) http://nirsimionovich.appointy.com/

  (w)http://www.greenfieldtech.net 

  (p) +972-73-2557799 (MSN):n...@greenfieldtech.net 



  (m) +972-54-6982826 (GTALK):nir.simionov...@gmail.com 



  (f) +972-73-2557202 (SKYPE): greenfieldtech.nir


--

Zero Your Inbox  | Cloud Servers 



--


*Disclaimer:*

This e-mail is intended solely for the person to whom it is addressed 
and may contain confidential or legally privileged information. Access 
to this e-mail by anyone else is unauthorized. If an addressing or 
transmission error has misdirected this e-mail, please notify the 
author by replying to this e-mail and destroy this e-mail and any 
attachments.
E-mail may be susceptible to data corruption, interception, 
unauthorized amendment, viruses and delays or the consequences 
thereof. If you are not the intended recipient, be advised that you 
have received this email in error and that any use, dissemination, 
forwarding, printing or copying of this email is strictly prohibited.






-- 
_
-- 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] autoconf version requirement increase

[Corey Farrell]

Thank you for pointing this out.  I have submitted a patch for Asterisk 
13, 15 and master so in future versions of Asterisk (13.19.0+ and 
15.2.0+) you will not need to run ./bootstrap.sh to build/install the 
open source Opus codec.



On 11/01/2017 04:27 PM, Jean-Denis Girard wrote:

Hi list,

Le 01/11/2017 à 09:08, Matt Fredrickson a écrit :

Hey Corey,

On Tue, Oct 31, 2017 at 3:52 PM, Corey Farrell <g...@cfware.com> wrote:

Hello all,

autoconf is required to run ./bootstrap.sh after changing configure.ac or
*.m4 in any folder of the Asterisk source tree (including menuselect and
third-party folders).  Currently we require 2.60, I'm working on a patch
that will require 2.64.  Note autoconf is not required to run './configure',
it is only needed to run ./bootstrap.sh.

Does anyone work on these files from a distribution which cannot install
autoconf-2.69?  Although the changes I'm working on only require autoconf
2.64 I see no reason to hold back at this point. Version 2.69 is from 2012
and is the latest version.  As far as I know CentOS 6 is the only major
currently supported distribution which has autoconf < 2.64, all others
provide 2.69.  Requiring 2.69 would be one less thing to worry about when
working on configure scripts, especially since I don't have access to test
any lower version.

We discussed this on IRC a bit yesterday, but to echo my response - I
myself work on a distribution that has at least version 2.69 of
autoconf so it shouldn't be a big deal.  I believe that re-running
bootstrap.sh should be so infrequent (only when we need to add things
to detect new libraries or do system related checks) that I don't have
a huge problem with bumping up the required version on it.  Since
generation of the configure script could be done on a more modern
system as a workaround I think it shouldn't be a big deal.

Anybody have opposing thoughts?


I'm using the open source Opus codec from
https://github.com/traud/asterisk-opus: it requires running
bootstrap.sh, so that would be a problem for my customers on CentOS-6.

Everything that breaks CentOS-6 seems bad to me.


Thanks,




-- 
_
-- 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] autoconf version requirement increase

[Corey Farrell]

Testing is always appreciated but this is unlikely to affect your 
ability to test patches from your CentOS 6 environment.  It's up to the 
author of a patch to include updated 'configure' script when needed.  
For example when you wrote the patch for the beanstalkd CDR engine you 
added detection of the beanstalk client to the configure.ac script, then 
you ran ./bootstap.sh' to regenerate configure.  Since you ran 
bootstrap.sh nobody else has to and your patch can be tested on a system 
that doesn't even have autoconf (or has an ancient version).


On Nov 1, 2017 2:08 AM, "Nir Simionovich" <nir.simionov...@gmail.com 
<mailto:nir.simionov...@gmail.com>> wrote:


   Hi Corey,

  Our production environment is based on CentOS 6.9 and we're
   slowly migrating to 7.X
   I would be happy to check out your patches inside our testing facility.


   On Tue, Oct 31, 2017 at 10:52 PM Corey Farrell <g...@cfware.com
   <mailto:g...@cfware.com>> wrote:

   Hello all,

   autoconf is required to run ./bootstrap.sh after changing
   configure.ac <http://configure.ac>
   or *.m4 in any folder of the Asterisk source tree (including
   menuselect
   and third-party folders).  Currently we require 2.60, I'm
   working on a
   patch that will require 2.64.  Note autoconf is not required to run
   './configure', it is only needed to run ./bootstrap.sh.

   Does anyone work on these files from a distribution which cannot
   install
   autoconf-2.69?  Although the changes I'm working on only require
   autoconf 2.64 I see no reason to hold back at this point.
   Version 2.69
   is from 2012 and is the latest version.  As far as I know CentOS
   6 is
   the only major currently supported distribution which has autoconf <
   2.64, all others provide 2.69.  Requiring 2.69 would be one less
   thing
   to worry about when working on configure scripts, especially since I
   don't have access to test any lower version.

   -Corey


   --
   _
   -- 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
   <http://lists.digium.com/mailman/listinfo/asterisk-dev>

   -- 


   Kind Regards,

  Nir Simionovich

   GreenfieldTech

   (schedule) http://nirsimionovich.appointy.com/
   <http://nirsimionovich.appointy.com/>

  (w)http://www.greenfieldtech.net <http://www.greenfieldtech.net/>

  (p) +972-73-2557799 <tel:+972%2073-255-7799>
   (MSN):n...@greenfieldtech.net <mailto:n...@greenfieldtech.net>

  (m) +972-54-6982826 <tel:+972%2054-698-2826>
   (GTALK):nir.simionov...@gmail.com <mailto:nir.simionov...@gmail.com>

  (f) +972-73-2557202 <tel:+972%2073-255-7202> (SKYPE):
   greenfieldtech.nir


   --

   Zero Your Inbox <https://mailstrom.co/referral/ARZJE> | Cloud
   Servers <https://www.digitalocean.com/?refcode=97eeea09917a>

   --


   *Disclaimer:*

   This e-mail is intended solely for the person to whom it is
   addressed and may contain confidential or legally privileged
   information. Access to this e-mail by anyone else is unauthorized.
   If an addressing or transmission error has misdirected this e-mail,
   please notify the author by replying to this e-mail and destroy this
   e-mail and any attachments.
   E-mail may be susceptible to data corruption, interception,
   unauthorized amendment, viruses and delays or the consequences
   thereof. If you are not the intended recipient, be advised that you
   have received this email in error and that any use, dissemination,
   forwarding, printing or copying of this email is strictly prohibited.


   --
   _
   -- 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
   <http://lists.digium.com/mailman/listinfo/asterisk-dev>

-- 
_
-- 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] autoconf version requirement increase

[Corey Farrell]

Hello all,

autoconf is required to run ./bootstrap.sh after changing configure.ac 
or *.m4 in any folder of the Asterisk source tree (including menuselect 
and third-party folders).  Currently we require 2.60, I'm working on a 
patch that will require 2.64.  Note autoconf is not required to run 
'./configure', it is only needed to run ./bootstrap.sh.


Does anyone work on these files from a distribution which cannot install 
autoconf-2.69?  Although the changes I'm working on only require 
autoconf 2.64 I see no reason to hold back at this point. Version 2.69 
is from 2012 and is the latest version.  As far as I know CentOS 6 is 
the only major currently supported distribution which has autoconf < 
2.64, all others provide 2.69.  Requiring 2.69 would be one less thing 
to worry about when working on configure scripts, especially since I 
don't have access to test any lower version.


-Corey


--
_
-- 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] AstDB mySQL implementation

[Corey Farrell]

I think astdb itself is an inappropriate place to deal with this.  astdb 
is initialized well before module preloads so it would be nearly 
impossible for modules to provide the astdb implementation.  In my 
opinion astdb is a database "implementation", not a connector (and that 
shouldn't change). For higher level connectors with configurable 
backends we have realtime and sorcery.  If someone were to write a redis 
or memcached connector for Asterisk I would expect it to be a realtime 
or sorcery driver.  If func_sorcery can be expanded to perform 
writes/deletes maybe it could be used in place of func_db?  I suspect 
that dialplan use of astdb is a bigger problem than the ways that 
Asterisk uses astdb directly on it's own.


Documentation: Maybe we need to add a warning to xmldoc for the astdb 
app/func/AMI/AGI that all astdb operations are serialized (dblock global 
mutex) and thus performance could suffer if used too much from too many 
threads?  Do we have any guides/sample files showing how to replace 
astdb operations with alternatives (func_odbc for example)?



On 10/29/2017 10:17 AM, Nir Simionovich wrote:
Seems like I have under estimated the task at hand, as this part of 
Asterisk requires some
more intricate familiarity with how AstDB truly works. It would be one 
thing to "change the backend"

it would a far more complex task to "make two backends selectable".

Conclusion - not sure this is worth the effort at this point in time, 
maybe in a later stage. :-(




On Thu, Oct 26, 2017 at 6:01 PM Nir Simionovich 
> wrote:


Correction, seems like this requires a bit more architecture than
I anticipated.

Basically, we need to separate this into several files and turn
the entire AstDB concept
into a pluggable module type module.

But, as AstDB is a mandatory module for Asterisk, can we create a
situation where
a pluggable module is a mandatory requirement for Asterisk to
launch correctly?

Is there anything like that in Asterisk? can someone point me in
some proper example
or preferably, something that I can look at and learn from?



On Thu, Oct 26, 2017 at 4:47 PM Nir Simionovich
> wrote:

I'll have a look at it.

If I read the code correctly, the AstDB is invoked from the
asterisk.c file, when asterisk is launched.
I think the best would be to add a configuration file like
astdb.conf, which will look something like this:

[general]
engine=builtin ; values can be either builtin, redis or
memcache (or others in the future)

;[redis]
;server = 127.0.0.1
;port = 6379
;database = 15 ; By default AstDB will use Redis database
number 15

;[memcache]
;server = 127.0.0.1
;port = 11211
;prefix = asterisk

Then, inside the db.c file add the proper statements and
backend to support each of these. I'm confident
that from a design perspective, this is not optimal, but it
would serve as a nice PoC to indicate if the task
is feasible or not.

For example, the following:

DEFINE_SQL_STATEMENT(put_stmt, "INSERT OR REPLACE INTO astdb
(key, value) VALUES (?, ?)")
DEFINE_SQL_STATEMENT(get_stmt, "SELECT value FROM astdb WHERE
key=?")
DEFINE_SQL_STATEMENT(del_stmt, "DELETE FROM astdb WHERE key=?")
DEFINE_SQL_STATEMENT(deltree_stmt, "DELETE FROM astdb WHERE
key || '/' LIKE ? || '/' || '%'")
DEFINE_SQL_STATEMENT(deltree_all_stmt, "DELETE FROM astdb")
DEFINE_SQL_STATEMENT(gettree_stmt, "SELECT key, value FROM
astdb WHERE key || '/' LIKE ? || '/' || '%' ORDER BY key")
DEFINE_SQL_STATEMENT(gettree_all_stmt, "SELECT key, value FROM
astdb ORDER BY key")
DEFINE_SQL_STATEMENT(showkey_stmt, "SELECT key, value FROM
astdb WHERE key LIKE '%' || '/' || ? ORDER BY key")
DEFINE_SQL_STATEMENT(create_astdb_stmt, "CREATE TABLE IF NOT
EXISTS astdb(key VARCHAR(256), value VARCHAR(256), PRIMARY
KEY(key))")

Can be augmented with something like the following:

DEFINE_REDIS_STATEMENT(put_redis_stmt, "");
DEFINE_REDIS_STATEMENT(get_redis_stmt, "");
DEFINE_REDIS_STATEMENT(del_redis_stmt, "");

Following this, we can simply point to the proper statements
following the engine selection.

What do you think, sounds reasonable?


On Thu, Oct 26, 2017 at 4:30 PM Olle E. Johansson
> wrote:


On 26 Oct 2017, at 15:25, Nir Simionovich
> wrote:

I suspect the original code didn't change the overall
mechanism dramatically, it's 

[asterisk-dev] chan_sip maintenance mode

[Corey Farrell]

I propose that we restrict the kind of bugs/patches that are accepted 
against chan_sip to only major/critical bugs, regressions and security 
fixes.  This means rejecting all new features, improvements and most 
minor bugs (AKA known limitations) reported against chan_sip.  I am not 
proposing we reject reports of crashes, deadlocks, major memory leaks, 
etc.  An exception would be new feature/improvement tickets where JIRA 
is being used to coordinate work - ASTERISK-13145 for example.  I 
believe we should allow people to continue using JIRA for such things, 
with the understanding that the feature is not going to be merged into 
any official Asterisk branch/release.


I'm interested in comments on this proposal from other current chan_sip 
users or anyone advocating for us.  The purpose of this proposal is to 
minimize the chance of regressions for existing chan_sip users, not to 
coerce anyone into migrating to chan_pjsip.



--
_
-- 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] One sip stack to rule them all....

[Corey Farrell]

Correction on number of bugs, most PJSIP bugs are filed against resource 
modules.  If you select all of the "Resources/res_pj*" categories in 
addition to Channels/chan_pjsip it shows 121 open bugs.  People are 
reporting issues against the pjsip modules, most are not against the 
channel driver itself.



On 10/08/2017 07:47 PM, James Finstrom wrote:
A large percentage of "PJSIP" Sucks comes down to comfort.  I talked 
to several users at astricon and the summary is:


Every provider that actually provides documentation only gives a 
chan_sip block

We don't understand how to configure it.
My customers need ccss.

So one issue with feature parody and mostly people who simply don't 
want to configure it.


The process of eventual removal when the ball gets rolling to do so is 
several releases away.
PJSIP is already in use on Digium's commercial platform which shows 
their level of confidence in the stack.


This ultimately comes down to the chicken vs the egg.
Once major adoption occurs PJSIP will become a rock. PJSIP will become 
a rock when major adoption occurs.


Looking at the tracker chan_sip has 233 open bugs, Chan_pjsip 38.
So if our metric is "bugs" then there is a clear winner

Remember the golden rule of software. No ticket, no bug.

Side note remember if it is removed in say Asterisk 19 (made up 
scenario) You don't have to use 19. All the previous releases will 
still have it.



On Sun, Oct 8, 2017 at 4:51 PM, Seán C. McCord > wrote:


I obviously failed to sufficiently emphasize the point.  Whether
you like it or not, whether you think pjsip is ready or not,
whether it is better or not, chan_sip is effectively at a dead
end.    Unless some miraculously talented and motivated person
emerges to maintain chan_sip (which is somewhat less likely than
my dead grandmother taking up x86 assembly), there is no future
for it.  The discussion is not about that.  There is no discussion
about that.  This is not about chan_sip vs chan_pjsip.  It is
pointless to wax about the perceived solidity of chan_sip.  It is
not solid.  It is not maintainable.  It is already years behind. 
People have managed to patch it into a simulacrum of stability
under certain use cases (though I will admit that those use cases
are wide and, in a self-fulfilling manner, perhaps do represent
the majority of present use cases of active users of chan_sip),
but this will not and has not continued.

Factual deprecation itself is not even under discussion.  chan_sip
_is_ deprecated, whether that is officially acknowledged or not.

Rather, this discussion is about making sure lurkers who are still
using chan_sip but have not reported specific problems or feature
gaps have their say, are aware that chan_sip is NOT the
recommended stack, and understand that chan_sip will (again,
whether anyone likes it or not) progressively worsen as time
progresses.


On Sun, Oct 8, 2017 at 3:33 PM Bryant Zimmerman
> wrote:

I would agree with this. We have tried to deploy pjsip several
times over the last year with limited success.
We have had nothing but issues with database real-time
deployments. Tables not working from one 13.x release to another.
Table builders sorcery failing out.
Issues when there are multiple transports on varying networks
were udp is not routed correctly through the asterisk servers.
No matter the settings.
Connectivity issues with varying success by carrier.
Unexplained audio quality issues that don't occur on the same
spec running chan_sip
We want to move to pjsip but the functionality and stability
have only proven out for limited applications.
Bryant

*From*: "Daniel Journo" >
*Sent*: Sunday, October 8, 2017 3:12 PM
*To*: "Asterisk Developers Mailing List"
>
*Subject*: Re: [asterisk-dev] One sip stack to rule them all

> What is _also_ needed, however, is more use of PJSIP and
reports of  specific problems, and specific deficits of PJSIP
so that the fear can be eased before, at some point many years
from now, chan_sip just doesn't work any more.

There are a number of specific issues on issue tracker which
still need addressing before more people will take it on
properly. Some issues probably require a semi-major rethink
and probably won’t be dealt with for months.
Making chan_sip depreciated would leave Asterisk with no
production grade sip stack that is officially being 

Re: [asterisk-dev] CentOS 6 and Ubuntu 12 Testing Support

[Corey Farrell]

CentOS 6 "Full Updates" support ends May 10th [1], after that "only 
Security errata and select mission critical bug fixes will be 
released".  I think this combined with testing difficulties justifies 
moving CentOS 6 into "extended" support for Asterisk. Probably worth 
announcing to asterisk-users the OS versions that are no longer 
automatically tested.


[1] https://wiki.centos.org/About/Product


On 04/12/2017 02:08 PM, Scott Griepentrog wrote:
After testing with CentOS 6.8, I agree that it has become difficult to 
continue supporting it.


On Wed, Apr 12, 2017 at 9:27 AM, George Joseph > wrote:




On Wed, Apr 12, 2017 at 7:22 AM, Scott Griepentrog
> wrote:

I agree that 12.04 is old enough to not worry about
supporting.  Because of the widespread use of CentOS 6 as an
Asterisk platform, I'd be concerned about abandoning it
however.  Which tests (beyond ODBC) are you having trouble with?


Pretty much every rest_api test fails because of Python package
issues.   The last errors are related to txaio and twisted.
The latest version of twisted to support python 2.6 is 14 which
starts  a cascade of package interdependencies that can only be
resolved by using pip to install specific versions of things.  I
could probably get it to work but is it worth the time and effort
to do so?

These are the packages that have to be installed via pip just to
get this far...
alembic
setuptools
requests
pyparsing
urlparse3
urllib3
virtualenv
virtualenv-clone
virtualenvwrapper
construct
pep8
autobahn
service-identity
construct==2.5.1
Twisted==14

Oh yeah, one of the packages we install from source (I think it's
sipp but I forget) requires autoconf268 which is available via yum
but it's a pain to get it to be used.


On Wed, Apr 12, 2017 at 7:45 AM, Joshua Colp > wrote:

On Tue, Apr 11, 2017, at 03:28 PM, George Joseph wrote:
> Both CentOS 6 and Ubuntu 12 have fallen into the state
where we can't
> actually create a new instance of either that can run
the Asterisk
> Testsuite.  In order to get it to work I've had to
fiddle Python packages
> both from the distributions' repositories and directly
from pip which
> makes
> the Python environment fragile FrankenSnake.  Also, ODBC
packages from
> that
> era are unreliable so I've had to download and install
both UnixODBC and
> the postgresql ODBC drivers from source to get a working
realtime setup.
> Finally, the Ubuntu 12 ISO images contain an
/etc/apt/sources.list that
> no
> longer works right out of the box.
>
> So what do you folks think the future of testing on
CentOS 6 and Ubuntu
> 12
> should be?

12.04 is EOL so not testing it is fine to me. CentOS 6 I
don't really
have a comment on, I'm not in that ecosystem myself.

--
Joshua Colp
Digium, Inc. | Senior Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
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





-- 
Digium logo

Scott Griepentrog
Digium, Inc · Software Developer
445 Jan Davis Drive NW · Huntsville, AL 35806 · US
direct/fax: +1 256 428 6239 · mobile: +1 256 580 6090
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





-- 
George Joseph

Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
Check us out at: www.digium.com  &
www.asterisk.org 


--

Re: [asterisk-dev] AST_MODULE_LOAD_FAILURE vs AST_MODULE_LOAD_DECLINE

[Corey Farrell]

Probably not the popular opinion, but I don't think any modules are 
"generally" important enough to abort Asterisk start-up.  For any module 
that is important enough for a system the admin can use 'require' or 
'preload-require' settings in modules.conf.  The exception to this would 
be when a module creates an unstable state by being half-loaded 
(example: global symbols exported that will crash if called).


On the 'core' side of things, anytime main/loader.c aborts start-up it 
should display an error message saying which module returned 
AST_MODULE_LOAD_FAILURE and make sure the logger gets flushed.  Even if 
every module gives a reason it is failing the module loader should still 
print a message (two errors on abort are better than possibly missing one).



On 04/05/2017 04:43 PM, George Joseph wrote:
Over the years I've been frustrated at times where Asterisk fails to 
start for absolutely no (apparent) reason.  No error message, no trap, 
nothing.  It just ends.  Every case of this I've tracked down has been 
the result of a module returning AST_MODULE_LOAD_FAILURE when it 
encounters a problem but not bothering to print an error first.   If 
you don't know already, AST_MODULE_LOAD_FAILURE tells asterisk to 
*stop loading and exit*.  What the module should have done was 
actually print an error and return AST_MODULE_LOAD_DECLINE which just 
tells asterisk that the module couldn't load.


So now I'm doing an audit of module return codes to see which call 
FAILURE vs DECLINE and found the modules below all call FAILURE.


The big question is...  Exactly which modules are so critical to 
asterisk that asterisk should exit if the module couldn't load?  
Surely not all 103 modules listed below.  If we can agree on the ones 
that really /should/ call FAILURE, I'll update those with error 
messages, then change the rest to DECLINE and add error messages as 
needed.


Reply with the list intact.  Don't add, remove, split or re-order the 
lines.  Just stick an "F " before the modules you think should remain 
failure.


Modules returning FAILURE:

./addons/app_mysql.c
./addons/cdr_mysql.c
./addons/chan_mobile.c
./apps/app_adsiprog.c
./apps/app_agent_pool.c
./apps/app_alarmreceiver.c
./apps/app_amd.c
./apps/app_authenticate.c
./apps/app_cdr.c
./apps/app_confbridge.c
./apps/app_festival.c
./apps/app_followme.c
./apps/app_forkcdr.c
./apps/app_meetme.c
./apps/app_queue.c
./apps/app_skel.c
./apps/app_voicemail.c
./cdr/cdr_custom.c
./cel/cel_custom.c
./cel/cel_odbc.c
./channels/chan_alsa.c
./channels/chan_console.c
./channels/chan_dahdi.c
./channels/chan_iax2.c
./channels/chan_mgcp.c
./channels/chan_misdn.c
./channels/chan_motif.c
./channels/chan_nbs.c
./channels/chan_oss.c
./channels/chan_phone.c
./channels/chan_pjsip.c
./channels/chan_sip.c
./channels/chan_skinny.c
./channels/chan_unistim.c
./channels/chan_vpb.cc
./codecs/codec_a_mu.c
./codecs/codec_adpcm.c
./codecs/codec_alaw.c
./codecs/codec_g722.c
./codecs/codec_g726.c
./codecs/codec_gsm.c
./codecs/codec_ilbc.c
./codecs/codec_lpc10.c
./codecs/codec_resample.c
./codecs/codec_ulaw.c
./formats/format_g723.c
./formats/format_g726.c
./formats/format_g729.c
./formats/format_gsm.c
./formats/format_h263.c
./formats/format_h264.c
./formats/format_ilbc.c
./formats/format_jpeg.c
./formats/format_ogg_vorbis.c
./formats/format_pcm.c
./formats/format_sln.c
./formats/format_vox.c
./formats/format_wav.c
./formats/format_wav_gsm.c
./funcs/func_cdr.c
./main/loader.c
./pbx/pbx_dundi.c
./pbx/pbx_loopback.c
./pbx/pbx_realtime.c
./pbx/pbx_spool.c
./res/res_pjsip/config_transport.c
./res/res_ari.c
./res/res_ari_events.c
./res/res_ari_model.c
./res/res_calendar.c
./res/res_chan_stats.c
./res/res_clialiases.c
./res/res_config_ldap.c
./res/res_config_sqlite.c
./res/res_config_sqlite3.c
./res/res_curl.c
./res/res_endpoint_stats.c
./res/res_fax.c
./res/res_hep_rtcp.c
./res/res_http_websocket.c
./res/res_musiconhold.c
./res/res_odbc.c
./res/res_phoneprov.c
./res/res_pjsip_nat.c
./res/res_pjsip_one_touch_record_info.c
./res/res_pjsip_outbound_publish.c
./res/res_pjsip_outbound_registration.c
./res/res_pjsip_sdp_rtp.c
./res/res_pjsip_send_to_voicemail.c
./res/res_pjsip_t38.c
./res/res_smdi.c
./res/res_snmp.c
./res/res_stasis.c
./res/res_stasis_device_state.c
./res/res_stasis_playback.c
./res/res_stasis_recording.c
./res/res_stasis_test.c
./res/res_statsd.c
./res/res_timing_kqueue.c
./res/res_xmpp.c
./res/res_pjsip_pubsub.c
./tests/test_bucket.c
./tests/test_channel_feature_hooks.c

--
George Joseph
Digium, Inc. | Software Developer
445 Jan Davis Drive NW - Huntsville, AL 35806 - US
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] AMI/ARI versioning

[Corey Farrell]

On 03/31/2017 10:47 AM, Kevin Harwell wrote:



On Thu, Mar 30, 2017 at 6:54 PM, Corey Farrell <g...@cfware.com 
<mailto:g...@cfware.com>> wrote:


On 03/30/2017 07:14 PM, Kevin Harwell wrote:
I think it's worth referencing a previous discussion on this [1].


Yes, thank you! I looked for this and for some reason my searches 
turned up nothing.


I agree with Mark's idea that having the ARI/AMI major version
tied to the Asterisk branch could lead to confusion, lead people
to believe that ARI 14.3.0 == Asterisk 14.3.0.


Yeah I could see that causing confusion.


[1]
http://lists.digium.com/pipermail/asterisk-dev/2016-November/075964.html
<http://lists.digium.com/pipermail/asterisk-dev/2016-November/075964.html>


Mark Michelson wrote:

2) Bump the major version of ARI for each major release of
Asterisk. We
won't retroactively apply this to the upgrade from Asterisk 12 to
Asterisk 13. So Asterisk 13 will have ARI versions 1.X.Y, Asterisk 14
will have ARI versions 2.X.Y, and Asterisk 15 will end up with
Asterisk 3.X.Y


I'm assuming the other numbers would just be reset here? For instance 
when Asterisk 15 is released it would it become 3.0.0?


I think either way we do it the versioning ends up being somewhat 
localized to the associated branch and the major number can't change 
once set on a branch.


Yes each new major version of Asterisk would start with AMI and ARI 
version X.0.0.  Once Asterisk 15 is released with ARI/3.0.0 we can't 
bump Asterisk 14 to also use ARI/3.x.x.  Although the versions are 
localized to the associated branch I think we should enforce that an 
older branch of Asterisk always has a lower major version for ARI/AMI.


With version X.Y.Z, I think this should represent:
X: architecture / Asterisk branch.  On bump of X all bets are off. X is 
associated to a specific major version of Asterisk but not an equal number.
Y: breaking change to existing features, but overall architecture in 
tact.  Might break/remove a function or event, ignore a parameter, add a 
new required parameter, etc.
Z: non-breaking change/addition: added optional parameter, new attribute 
in response, new function/event (including from any new module).


So an app written for 3.0.0 should work unmodified against 3.0.1, but 
may require tweaks to work with 3.1.0.  An app written for 3.0.1 might 
work with 3.0.0, but not guaranteed if the app uses features added to 3.0.1.
-- 
_
-- 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] AMI/ARI versioning

[Corey Farrell]

On 03/30/2017 07:14 PM, Kevin Harwell wrote:



[asterisk-branch-number].[minor].[patch]


Actually, the proposal might be better represented as the following:

[asterisk-branch-number].[major].[minor/patch]

Or another way to state it:

[asterisk-branch-number].[api breaking].[api non breaking]


I think it's worth referencing a previous discussion on this [1].  I 
agree with Mark's idea that having the ARI/AMI major version tied to the 
Asterisk branch could lead to confusion, lead people to believe that ARI 
14.3.0 == Asterisk 14.3.0.


[1] http://lists.digium.com/pipermail/asterisk-dev/2016-November/075964.html
-- 
_
-- 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] Line length restrictions in code changes

[Corey Farrell]

On 03/16/2017 04:35 PM, Kevin Harwell wrote:
On Thu, Mar 16, 2017 at 2:54 PM, Matthew Jordan > wrote:




Questions than:
(1) Should there even be a line length rule?


I lean toward yes just to keep things sane (e.g. keeping someone 
extending a function definition to infinity and beyond). Also 
sometimes I split my editor's screen vertically and have multiple code 
views up. The vertical split reduces my column length and I either 
have to start scrolling or (in my current setup) the line gets 
wrapped. Both hurt readability for me.
This is an issue on gerrit sometimes, an issue with the side-by-side 
view of patches sometimes doesn't allow you to scroll all the way to the 
right edge of a line.
-- 
_
-- 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] Problems with custom C-file: undefined reference

[Corey Farrell]

Forget about *.exports if your goal is to make system.c functions available
for use from chan_sip.c.  All that is needed is for system.c to be placed
in channels/sip/ - not channels/.  Putting the file in the correct place
will link it into chan_sip.so, thus making the functions available to call
from chan_sip.c.  If this does not resolve your issues you will need to
post your patched asterisk to github.com or similar world readable GIT
repository - we can't see what you're doing wrong unless you show us.

On Sat, Dec 17, 2016 at 1:31 AM, Julian Fleischhauer <
julian.fleischha...@stud.uni-due.de> wrote:

> Because this problem most likely being the last barrier to finish my
> project, I will explain it once again in a little more detail.
>
> There are the following files that I think may be important to solve the
> problem:
> *channels/chan_sip.c*
> *channels/sip/include/system.h*
> *channels/system.c*
> *channels/system.exports.in <http://system.exports.in>*
>
> I want *system.c* to export several functions, one of them is
> *sys_system_get_our_address(struct ast_sockaddr *us).*
>
> That function is implemented in *system.c, *gets called in *chan_sip.c*
> and it's prototype is declared in *system.h*.
>
> To export that function, I created the file system.exports.in in
> channels/, which content is the following:
> {
> global:
> LINKER_SYMBOL_PREFIXsys_*;
> LINKER_SYMBOL_PREFIX_IO_stdin_used;
> local:
> *;
> };
>
> I tried a *make dist-clean* and a complete recompilation afterwards. The
> build system doesn't create the file *system.exports*, but it creates
> *chan_sip.exports* which is in the same folder. I dig into to the system
> and found *build-tools/make_linker_version_script* which probably creates
> the exports files.
> test -f ${1}.exports.in && ${AWK} "{sub(\"LINKER_SYMBOL_PREFIX\",
> \"${2}\"); print;}" ${1}.exports.in > ${1}.exports && exit 0
> test -f ${1}.exports.in || rm -f ${1}.exports && cp ${3}/default.exports
> ${1}.exports && exit 0
>
>
> Can anyone see any mistakes I make? Thank you very much.
>
> Regards,
> Julian Fleischhauer
>
>
> 2016-12-12 11:40 GMT+01:00 Julian Fleischhauer <
> julian.fleischha...@stud.uni-due.de>:
>
>> Thank you for this simple solution, it solved the last problem.
>>
>> Another issue arised: Before, system.c was in the main/ directory and
>> used the project wide exports file to export its functions, now it's in
>> channels/ where apparantly doesn't exist such a file.
>> I created *system.exports.in <http://system.exports.in>* in channels/ to
>> export the functions but the Asterisk build system does not automatically
>> generate the *system.exports* file and manually creating it doesn't work
>> either.
>>
>> *system.h*
>> void ast_system_get_address(struct ast_sockaddr *us);
>>
>> *system.c*
>> #include "asterisk/system.h"
>>
>> void ast_system_get_address(struct ast_sockaddr *us)
>> {
>>...
>> }
>>
>> *chan_sip.c*
>> #include "asterisk/system.h"
>>
>> ast_system_get_address(us);
>>
>> 2016-12-09 18:42 GMT+01:00 Corey Farrell <g...@cfware.com>:
>>
>>> I'm not sure what you're trying to accomplish but this is not the
>>> approach I'd take for expanding chan_sip.  My advice is to simply put your
>>> system.c into channels/sip.  This will automatically link it into the
>>> binary chan_sip module, but allow you to keep your source code in a
>>> separate file.  I believe MODULEINFO (for any added dependencies) and
>>> possibly a call to your init/cleanup would be the only changes you'd need
>>> to make in chan_sip.c.  All non-static symbols in chan_sip.c and
>>> channels/sip/*.c are available across all sources of chan_sip.so without
>>> needing chan_sip.exports.in or AST_MODFLAG_GLOBAL_SYMBOLS.
>>>
>>> The only exception is if you need to call chan_sip functions from
>>> another existing module, then you will have to export the symbols.  To do
>>> this you have to tell the linker (via chan_sip.exports.in) and you have
>>> to tell the module loader.  AST_MODULE_INFO at the very end of chan_sip.c
>>> would need AST_MODFLAG_GLOBAL_SYMBOLS so the module loader knows to share
>>> the globals:
>>>
>>> AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS
>>> | AST_MODFLAG_LOAD_ORDER, "Session Initiation Protocol (SIP)",
>>>
>>>
>>>
>>> On Fri, Dec 9, 2016 at 10:50 AM, Julian Fleischhauer <
>>> julian.fleischha...@

Re: [asterisk-dev] Problems with custom C-file: undefined reference

[Corey Farrell]

I'm not sure what you're trying to accomplish but this is not the approach
I'd take for expanding chan_sip.  My advice is to simply put your system.c
into channels/sip.  This will automatically link it into the binary
chan_sip module, but allow you to keep your source code in a separate
file.  I believe MODULEINFO (for any added dependencies) and possibly a
call to your init/cleanup would be the only changes you'd need to make in
chan_sip.c.  All non-static symbols in chan_sip.c and channels/sip/*.c are
available across all sources of chan_sip.so without needing
chan_sip.exports.in or AST_MODFLAG_GLOBAL_SYMBOLS.

The only exception is if you need to call chan_sip functions from another
existing module, then you will have to export the symbols.  To do this you
have to tell the linker (via chan_sip.exports.in) and you have to tell the
module loader.  AST_MODULE_INFO at the very end of chan_sip.c would need
AST_MODFLAG_GLOBAL_SYMBOLS so the module loader knows to share the globals:

AST_MODULE_INFO(ASTERISK_GPL_KEY, AST_MODFLAG_GLOBAL_SYMBOLS
| AST_MODFLAG_LOAD_ORDER, "Session Initiation Protocol (SIP)",



On Fri, Dec 9, 2016 at 10:50 AM, Julian Fleischhauer <
julian.fleischha...@stud.uni-due.de> wrote:

> I could resolve the XML and pcap issues, but still have problems
> concerning "sip_get_header()". I don't want to rename the function since
> it's a frequently used function in chan_sip.c, I therefore defined an .
> exports.in file. I don't know if Mr. Michelson understood me correctly,
> but I think chan_sip.c exports "sip_get_header()" (doesn't it?) so I
> modified *chan_sip.exports.in * so it looks
> like
> {
> global:
> LINKER_SYMBOL_PREFIXsip_get_header;// I also tried
> LINKER_SYMBOL_PREFIXsip_*;
> LINKER_SYMBOL_PREFIX_IO_stdin_used;
> local:
> *;
> };
> But it doesn't work. I did the same with *system.exports.in
> *, without success.
>
>
> Sorry for the badly formatted last mail. I will post the updated file
> structure again below.
>
> *sip.h*
> const char *sip_get_header(const struct sip_request *req, const char
> *name);
>
> *system.h*
> #include "asterisk/netsock2.h"
>
> void ast_some_function(struct sip_pvt *p, struct ast_sockaddr *addr);
>
> *system.c*
> #include "asterisk/system.h"
> #include "asterisk/sip.h"
>
> void ast_some_function(struct sip_pvt *p, struct ast_sockaddr *addr)
> {
>ast_copy_string(touser, sip_get_header(req, "To"),
> strlen(sip_get_header(req, "To")) + 1);
> }
>
> *chan_sip.c*
> #include "asterisk/system.h"
> #include "asterisk/sip.h"
>
> ast_some_function(p, addr);
>
>
> Best regards,
> Julian Fleischhauer
>
> 2016-12-06 18:35 GMT+01:00 Mark Michelson :
>
>> On 12/05/2016 04:53 PM, Julian Fleischhauer wrote:
>>
>>> Hey all,
>>>
>>> I have a problem using a custom C-file. The error ouput received when
>>> compiling is given below.
>>>
>>> error output
>>> .../system.c:1330: undefined reference to `XML_ParserCreate'
>>> ...
>>> .../system.c:1465: undefined reference to `sip_get_header'
>>> ...
>>> .../system.c:1608: undefined reference to `pcap_lookupnet'
>>> ...
>>>
>>>
>>> Before, I had all code in chan_sip.c. I want to transfer the code into a
>>> separate file (system.c) now. The dependencies given below MODULEINFO are
>>> customly set and worked fine when used in chan_sip.c. Can anyone figure out
>>> if something else has to be added somewhere? The code is quoted below:
>>>
>>> sip.h
>>> const char *sip_get_header(const struct sip_request *req, const char
>>> *name);
>>>
>>> system.h
>>> #include "asterisk/netsock2.h"
>>>
>>> void ast_some_function(struct sip_pvt *p, struct ast_sockaddr *addr);
>>>
>>> system.c
>>> /*** MODULEINFO
>>>  sqlite3
>>>  expat
>>>  pcap
>>> extended
>>> ***/
>>>
>>> #include "asterisk/system.h"
>>> #include "asterisk/sip.h"
>>>
>>> void ast_some_function(struct sip_pvt *p, struct ast_sockaddr *addr)
>>> {
>>> xmlParser = XML_ParserCreate(NULL);
>>>...
>>>ast_copy_string(touser, sip_get_header(req, "To"),
>>> strlen(sip_get_header(req,   "To"))+1);
>>>...
>>>pcap_lookupnet(pcapDev, , , errbuf);
>>> }
>>>
>>> chan_sip.c
>>> #include "asterisk/system.h"
>>> #include "asterisk/sip.h"
>>>
>>> ast_some_function(p, addr);
>>>
>>>
>>> Thank you for any help!
>>>
>>> Regards,
>>> Juliannn
>>>
>>> I can explain why sip_get_header() is causing a problem. In order for
>> functions to be callable from other modules, they need to be explicitly
>> exported. This is why you'll see files like res/res_agi.exports.in,
>> which define functions that can be called globally from that module. You
>> have a couple of options here:
>>
>> * Define a .exports.in file for your system.c file that exports the
>> sip_get_header() function.
>> * Change "sip_get_header()" to "ast_sip_get_header()". Assuming that
>> system.c is in the main/ directory, there is a project-wide exports file
>> that makes it so any function starting with "ast" 

Re: [asterisk-dev] Anyone interested in having file/line number shown in dialplan show output?

[Corey Farrell]

Overall I like it.  I think leaving pbx_ael out of this is best as it
might not be straight forward.  Anyone who wants pbx_ael to provide
information about config sources can create a follow-up patch once the
core functionality is implemented.

Some thoughts:
* It would be nice if the registrar ("pbx_config:") were omitted when
config file/line are available.
* I assume we can strip 'ast_config_AST_CONFIG_DIR' from included
config filenames?

I suspect both of these things could be done in follow-up patches, I
just want to consider limited terminal width and try to avoid unneeded
wrapping.


On Wed, Dec 7, 2016 at 1:49 PM, Jonathan Rose
 wrote:
> I've been working through a lot of complicated dialplan setups lately,
> mostly written in AEL. Doing so has left me painfully aware of the fact that
> it can sometimes be hard to trace a dialplan through its various extensions.
> More so in pbx_ael of course where what you write isn't necessarily matched
> in an easy to read way to the extensions you create, but even in pbx_config
> it can get slightly tricky to follow when included files are involved or
> there is heavy use of 'n' to assign priority.
>
> So far I've written a patch against trunk which adds in the basic
> functionality I'm describing and applies it to extensions created by
> pbx_config. I'm not sure if I should bother following it through to
> completion since the benefits in pbx_config are limited, I'm unsure of how
> many people actually use AEL to write their dialplans, and I'm not entirely
> sure if I'm sharp enough to figure out how to add it to AEL in a
> non-cataclysmic manner anyway. But I figured I'd at least post here to gauge
> people's interest.
>
> Linked is a picture showing illustrating the difference in my experimental
> patch. It would also be fairly trivial to add a similar file/line number tag
> to verbose 3 "channel ran extension" messages with the patch I've written.
>
> http://imgur.com/LCh61fx
>
> --
> Jonathan R. Rose
> Senior Systems Engineer
>
> Motorola Solutions
>
> email: jonathan.r...@motorolasolutions.com
>
> --
> _
> -- 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

-- 
_
-- 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] tcptls: Use new certificate on reload

[Corey Farrell]

A review [1] has been posted to fix an issue where TLS servers would
not be restarted unless the bind address was changed.  This would
prevent use of new certificates if available.  Unfortunately this
change does cause an ABI change.  Fields are added to public
structures 'struct ast_tls_config' and 'struct
ast_tcptls_session_args'.  Within Asterisk itself these structures are
used by app_externalivr, chan_sip, res_http_websocket, http.c and
manager.c.

tcptls.h does not provide an allocation method for it's structures.
These means it is impossible to add fields to these structures without
breaking the ABI.  How does everyone feel about moving forward with
the fix as is?

[1] https://gerrit.asterisk.org/4448

-- 
_
-- 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] ARI versioning in 13 and 14

[Corey Farrell]

+1 for option 2. I'm against option 1 because the ARI version should
be enough to verify that the features of 14.2.0 are available.

One remaining question: how would we handle situations where a
critical or security bug requires a breaking change to ARI in Asterisk
13?

-- 
_
-- 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] ARI StasisEnd event vs. channel variables

[Corey Farrell]

On Fri, Oct 21, 2016 at 12:32 PM, Kevin Harwell <kharw...@digium.com> wrote:
> On Fri, Oct 21, 2016 at 10:49 AM, Corey Farrell <g...@cfware.com> wrote:
>>
>>
>> I'm in favor per app config.  I do not yet use ARI, but when I do I
>> will have '#tryinclude /etc/asterisk/ari.d/*.conf' in ari.conf.  My
>> hope is that each ARI app would install it's own config to
>> /etc/asterisk/ari.d, avoid modification of ari.conf / global options.
>> Per app config would also be important if dialplan functions can be
>> added to StasisEnd.
>>
>
> This is getting down more into implementation details, but instead of
> configs what about adding the desired variables to the event subscribe
> command? Each app could subscribe (or when they subscribe) to an event and
> specify the variable(s) they desire. At least to start this could be limited
> to a small list of allowed events and variables.

Sounds fine.  I'm not sure what benefit there would be to limiting
which variables can be used but that sort of restriction could easily
be relaxed/removed later.  The obvious exception being to block
live_dangerously functions by default.

-- 
_
-- 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] ARI StasisEnd event vs. channel variables

[Corey Farrell]

On Fri, Oct 21, 2016 at 9:37 AM, Sébastien Duthil
 wrote:
> On 19/10/16 11:59 AM, Mark Michelson wrote:



>> I had thought about making the list of channel variables per-ARI app
>> instead of global, but I don't know if that's actually desired.
>
> The underlining question would be: do people use ARI as one big
> application that does all the logic, or do they use ARI as many small
> applications that handle different parts of their logic? In the "one big
> application" case, global configuration is enough. In the "many small
> applications" case, the different applications will probably need
> different variables, and global configuration will clutter the events
> with variables that application mostly don't care about. I guess
> per-application configuration fits better with the philosophy of ARI.
>
> --
> Sébastien Duthil

I'm in favor per app config.  I do not yet use ARI, but when I do I
will have '#tryinclude /etc/asterisk/ari.d/*.conf' in ari.conf.  My
hope is that each ARI app would install it's own config to
/etc/asterisk/ari.d, avoid modification of ari.conf / global options.
Per app config would also be important if dialplan functions can be
added to StasisEnd.

Please also consider the existence of explicit subscriptions to
StasisEnd.  I'm not sure the best way to handle them but assume they
could need access to variables as well.  In situations where we send
StasisEnd events to multiple apps I think it would be better to
generate one packet containing the union of all variables requested by
active subscriptions.  I believe the goal is to minimize overhead of
generating StasisEnd, so generating a unique packet per destination
would be wasteful.  On the other side the overhead of parsing JSON and
ignoring unneeded variables is low.

-- 
_
-- 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] Viva Chan_Sip, may it rest in peace

[Corey Farrell]

On Mon, Oct 10, 2016 at 10:39 AM, Matthew Jordan <mjor...@digium.com> wrote:
>
> On Fri, Oct 7, 2016 at 10:31 AM, Corey Farrell <g...@cfware.com> wrote:
> > Many people don't like chan_sip, most people hate working with the code.
> > The rush to throw out chan_sip when PJSIP isn't ready to be the only SIP
> > stack annoys me a bit.  Nobody is forcing anyone to use or contribute to
> > chan_sip.  Digium changed chan_sip from core to extended support so they can
> > significantly reduce involvement.  At some point chan_sip will fizzle out
> > but that hasn't happened yet.
>
> I don't think we're rushing - if anything, most people at DevCon (and
> I'd also say in this conversation) are being pretty realistic about
> how long it will take before chan_sip could be removed from the source
> tree. I think all we're seeing right now is a conversation about how
> we might eventually get there.

I wasn't at DevCon this year, sorry I latched onto the harshest
statements.  I should have ignored these comments knowing that Digium
will not just pull the rug out.

> Generally, when a module is first marked as being 'deprecated', we
> usually let it go through at least one additional release before it
> would ever be removed. Even then, our preference is to just leave the
> module in the source tree (usually disabled), in case someone still
> wants to use it. About the only time we remove a module is when it
> either no longer compiles, or when it is actively causing harm.
> Assuming we all decided to mark chan_sip as deprecated in Asterisk 15,
> then soonest it would be removed from the source tree is Asterisk 17 -
> so I think we've got some time to hash this out.
>
> As far as extended support is concerned - yes, Digium is not
> interested in maintaining chan_sip any longer. Our commercial products
> and interests are focused on the PJSIP stack. As such, once Asterisk
> 11 leaves bug fix support, I would expect our fixing of chan_sip
> related issues to drop dramatically - even more than they already
> have. Patches submitted for chan_sip through Gerrit will still be
> reviewed and receive attention, as will security patches.

Ack.  As long as patches are accepted and chan_sip is kept in the
source tree while maintainers exist, this is good enough for me.

> > Last time I checked about 200 of the PJSIP testsuite tests produced AO2
> > leaks, in many cases hundreds of objects were leaked [1].  Some of these
> > leaks may be caused by bugs in tests and I realize that many use cases must
> > work without major leaking, but some use cases could cause failures.  I've
> > submitted some fixes but I find PJSIP leak tracing more difficult than other
> > parts of Asterisk.
>
> I'll admit that some of the tracing is a bit more challenging, both
> with sorcery as well as with PJSIP memory pools.

I have made no attempts to check PJSIP memory pools, so far for PJSIP
I've only ever looked at AO2 leaks.  No point doing
MALLOC_DEBUG/valgrind checking until AO2 leaks are addressed.

> My impression right now is that any objects that are not released on
> shutdown are configuration objects or other similar objects that
> allocated a single time during some PJSIP module initialization. While
> those make AO2 debugging challenging (and it would be great to clean
> them up), they don't indicate any harmful memory leak.

I'll need to see what the current results show (last run was 3 months
ago).  These startup leaks may not be directly harmful to production,
but now or in the future a harmful leak will exist.  Hidden in the
haystack is the problem here.  Automated testing can only pass or
fail, since no automated process can determine what is a startup leak
and what is harmful, this blocks leak testing.

> Of course, I do understand if that means you don't want to use those modules.

This has no immediate impact on my plans, I am not considering PJSIP
before 15 (possibly even 17).  I'm raising this concern in the hope
that we can try to improve the situation before chan_sip becomes
non-viable.  This has an impact on testing ARI as well.  I assume at
some point existing tests outside tests/channels will be converted to
use chan_pjsip instead of chan_sip (pbx apps/funcs, AMI, etc).  Long
term this could become a leak testing issue system-wide.

> > The current policy of allowing new features into released LTS branches is a
> > concern for me with PJSIP.  If I were to start using PJSIP I would have to
> > worry about each 13.x.0 release having a new PJSIP feature possibly cause a
> > bug.  A lot of bad things can be said about chan_sip, but new features are
> > extremely unlikely in 13.12.0.  It would be nice if a core set of PJSIP and
> > other modules could be declared LTS frozen.  During LTS releases these
> > 

Re: [asterisk-dev] Viva Chan_Sip, may it rest in peace

[Corey Farrell]

Many people don't like chan_sip, most people hate working with the code.
The rush to throw out chan_sip when PJSIP isn't ready to be the only SIP
stack annoys me a bit.  Nobody is forcing anyone to use or contribute to
chan_sip.  Digium changed chan_sip from core to extended support so they
can significantly reduce involvement.  At some point chan_sip will fizzle
out but that hasn't happened yet.

Last time I checked about 200 of the PJSIP testsuite tests produced AO2
leaks, in many cases hundreds of objects were leaked [1].  Some of these
leaks may be caused by bugs in tests and I realize that many use cases must
work without major leaking, but some use cases could cause failures.  I've
submitted some fixes but I find PJSIP leak tracing more difficult than
other parts of Asterisk.

The current policy of allowing new features into released LTS branches is a
concern for me with PJSIP.  If I were to start using PJSIP I would have to
worry about each 13.x.0 release having a new PJSIP feature possibly cause a
bug.  A lot of bad things can be said about chan_sip, but new features are
extremely unlikely in 13.12.0.  It would be nice if a core set of PJSIP and
other modules could be declared LTS frozen.  During LTS releases these
modules would be strictly bug fix only.  I suspect this is not yet wanted
or possible for PJSIP modules, but hope it can be re-evaluated before new
LTS branches.  My hope is that eventually a basic PJSIP PBX could be run
using only frozen modules.  Users with a specific needs or higher risk
tolerance could run some / all of the unfrozen modules to get more advanced
or less mature features.  Eventually the list of frozen modules could grow
as each module becomes feature complete and is proven stable.

I do see chan_pjsip in my future.  It has all the features I need plus the
config handling will improve my handling of dynamic peers.  I have no
timeframe to migrate as I can't ignore my concerns and they can't be
addressed overnight.

[1] https://jenkins.asterisk.org/jenkins/job/periodic-ref_de
bug-asterisk-13/8/

On Tue, Oct 4, 2016 at 8:46 PM, James Finstrom  wrote:

> So the discussion of deprecating chan_sip came up at the devcon this year
> and it caused a bit of a stir. The end result was the need for broader
> discussion with a wider audience.  So let's discuss.
>
> Currently, Asterisk is running dual sip stacks. The argument is that to
> deprecate PJSIP there must be broader adoption. There is currently nothing
> motivating adoption but much slowing it.
>
> What are some of the hurdles to adoption?
> 1. Apathy.  If it ain't broke don't fix it. Many would argue chan_sip is
> broke but it is the "devil you know". A decade of documentation and a broad
> user base allows people to be pretty forgiving of flaws. Almost any issue
> has some sort of work around or generally accepted idea of I guess we can
> live with it.
>
> 2. One Ring to rule them all!!  PJSIP requires up to 6 sections of
> configuration. Once you dig in, this method makes sense. But at a glance,
> you have just multiplied the workload to  6 times that of chan_sip's single
> blob config. Though it is not really 600% more effort it may be enough to
> scare some away
>
> 3. Mo Adoption, Mo problems!
> The only way to clean up all the edge cases and weird bugs is to hit them
> in the first place.  Dogfooding only gets you so far.  You can make
> anything working clean in a single environment and single use case. But
> what happens when people start flinging wrenches. Things break. 100
> wrenches may break 10 things. 1000 wrenches may break 100 things.  In the
> ladder case, you have 100 people saying pjsip sucks, and pjsip is crap. As
> with all things the 900 assume all is good and move on with their lives
> telling no one of their glory. So you have 10% of the voices running
> unopposed. You fix the 100 issues and that is great but those 100 people
> have gone back to the comfort of chan_sip and are stuck at point 1.
>
> Escaping the cycle.
>
> So how do we dredge through this mess and get high adoption?
>
> You have to make #1 not an option.  This means potentially breaking the
> universe. This is why I think there is a tendency to be gunshy. No one
> wants to be the guy who broke the universe.  But breaking the universe gets
> us to #3 without falling back into #1.   Once The universe breaks and we
> are in #3 many of the edges will be found and fixed. Suddenly PJSIP becomes
> usable in most, if not all situations. The issues in #2 will automatically
> resolve as there is more information and it becomes the "accepted way" of
> doing things.  The old dogs will have to refactor how they do configuration
> but I am confident once they do a few they will figure out the bark is
> bigger than the bite.
>
> tl;dr to get adoption you have to force it.  There will be blood, but
> nothing that can't be cleaned up with a little bleach and some elbow grease
>
> --
> James
>
> --
> 

Re: [asterisk-dev] Working Groups

[Corey Farrell]

On Thu, Oct 6, 2016 at 6:44 AM, Dan Jenkins  wrote:

> So if I understand properly you'd like to see a working group around "My
> first Asterisk Commit" - which would drive forward making contributing to
> the project easier - whether it was improving documentation, or trying to
> remove some of the road blocks people hit (like running the test suite
> locally) or getting gerrit/gitreview setup etc?
>

I was following the devcon discussions the best I could, I heard about a
Docker container that ran the testsuite [1].  It seemed to be outdated
(still using SVN) and in production I use CentOS 7 so I wanted to test
against those library versions.  I've created a new Docker container [2]
 to run the testsuite.  It can clone from any URL / branch of Asterisk,
Testsuite and custom tests.  I welcome feedback (on github or here),
especially for the built-in configure / menuselect options.

The container doesn't have support for cherry-picking gerrit reviews yet,
I've opened a feature request for git-review to support anonymous use [3].
I'm unsure the feature request policy of the git-review project, hopefully
they decide I'm asking for something that is generally useful.

[1] https://github.com/sboily/asterisk-testsuite
[2] https://github.com/coreyfarrell/centos-asttest
[3] https://storyboard.openstack.org/#!/story/2000736
-- 
_
-- 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] Using command line arguments in other file than asterisk.c

[Corey Farrell]

A custom command-line option is undesirable due to the limited number
of possible option codes / chance of future conflict if we need to add
a standard option.  If you really need a custom option just for your
own chan_sip, it would be better to add it to sip.conf and parse it
during config load.  Another option would be to have chan_sip
recognize an environmental variable.

You haven't said what your option is needed for so nobody can judge if
you have a common need that should be added to chan_sip.

On Wed, Sep 14, 2016 at 11:48 AM, Julian Fleischhauer
 wrote:
>
> 2016-09-14 11:18 GMT+02:00 Tzafrir Cohen :
>>
>> On Tue, Sep 13, 2016 at 05:09:59PM +0200, Julian Fleischhauer wrote:
>> > Hi everyone,
>> >
>> > Sorry if this mail is a repost, but I can't figure out if my first mail
>> > went through, because I have first been registered now.
>> >
>> > I have an issue which I can't fix myself.
>> >
>> > I want to use a command line argument variable in a different file than
>> > asterisk.c.
>>
>> Command-line arguments are kept in ast_options. 'git grep -w
>> ast_options' would show you many places in the code where they are
>> tested. Those options are also asterisk.conf options as command-line
>> arguments generally relate to asterisk.conf options.
>>
>
> I've already tried using ast_options but I figured out that it will be
> difficult because ast_flags is an unsigned int, which has a maximal size of
> 32 bit. Unfortunately, there are already 31 different option flags, and I
> need to use four additional ones.
>
> Regards,
> Juliannn
>
>
> --
> _
> -- 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

-- 
_
-- 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] BRIDGEPEER on multi-party conferences: Thoughts?

[Corey Farrell]

We should assume that someone is writing BRIDGEPEER to CDR or CEL, and
that this includes 3 party bridges.  I don't like the idea of changing
the format of this variables value, at least not in released branches.

I actually like Matt's "unfortunate" idea of adding an option to
asterisk.conf, maybe re-add the [compat] section.  We create the
built-in variable for retrieval purposes, but still set the value to
generate VarSet events.  It could be enabled by default on 13,
disabled on 14 and removed from 15?

On Wed, Aug 10, 2016 at 11:52 AM, Mark Michelson <mmichel...@digium.com> wrote:
> An alternative could be to set BRIDGEPEER to something like "multi-party",
> "conference", or the bridge ID if there are more than two participants. This
> way, the only channel you have to set a variable on is the one
> joining/leaving the bridge, which means you are not having to lock the
> bridge at all. Yes, there would likely be a special case for when the number
> of channels first goes from 2 to 3, but the majority of the time, that's not
> what would be happening. This way, you still get the VarSet when the channel
> enters and leaves, and you have a non-empty value for BRIDGEPEER when the
> channel is with multiple parties in a bridge. You won't get VarSets on the
> other channels in a bridge when a channel enters or leaves, though. I'm not
> sure if that's something that people rely on or not.
>
> On Wed, Aug 10, 2016 at 10:11 AM, Corey Farrell <g...@cfware.com> wrote:
>>
>> It might be a problem if BRIDGEPEER is being compared to an empty string
>> to see if a channel is alone.
>>
>> What if you made BRIDGEPEER into a built-in channel variable (like
>> ${EPOCH}).  This would mean that you wouldn't be setting any channels, you'd
>> only do a lookup when requested.  One side-effect is that VarSet events
>> would never be produced for this variable, not sure how much this would
>> matter given better events to monitor ConfBridge joins/parts?
>>
>> On Tue, Aug 9, 2016 at 7:01 PM, Mark Michelson <mmichel...@digium.com>
>> wrote:
>>>
>>> Hi folks,
>>>
>>> I've been looking into a Digium customer issue where ConfBridge audio has
>>> been dropping out. The main issue there had to do with DNS, and there is
>>> currently a review [1] up to fix that.
>>>
>>> A secondary issue, though, is that there would be brief audio cutouts
>>> when participants joined and left the conference. Looking into it, I believe
>>> the problem is that when a participant enters or leaves the bridge, the
>>> BRIDGEPEER channel variable is updated for every remaining participant in
>>> the bridge. The basic algorithm is like this:
>>>
>>> * Lock the bridge
>>> * Iterate through the channels in the bridge (a maximum of 11 of them)
>>> * Lock the channel
>>> * Append the channel to an array of channel names
>>> * Unlock the channel
>>> * Iterate through the channels in the bridge again (no upper limit this
>>> time)
>>> * Lock the channel
>>> * Set the BRIDGEPEER channel variable using the names in the
>>> generated array from before (comma-separated)
>>> * Unlock the channel
>>> * Unlock the bridge
>>>
>>> In addition, this same process occurs every time an announcement is
>>> played into a bridge, such as join/leave beeps. Playing an announcement
>>> involves pushing an announcer channel into the bridge and then removing the
>>> announcer channel when the playback completes.
>>>
>>> My question to the list is this: do you find value in having the
>>> participants in a multi-party bridge packaged into the BRIDGEPEER channel
>>> variable? I know that for two-party bridges, there are probably lots of
>>> scripts and dialplans out there that rely on that variable to be set; my
>>> question specifically is for bridges with more than two parties.
>>>
>>> My thoughts on the matter are that since bridges are query-able now
>>> directly, getting the list of participants from the bridge makes more sense
>>> than trying to get the participants based on a single channel in that
>>> bridge. In addition, the code places a hard limit on the number of channel
>>> names it will actually put in the BRIDGEPEER variable. The code currently
>>> has a #define that makes it so that only the first 11 channels in the bridge
>>> will have their names in BRIDGEPEER. Because of the hard-coded maximum, if
>>> you have more than 11 channels in the bridge, you can't get the full list of
>>&

Re: [asterisk-dev] BRIDGEPEER on multi-party conferences: Thoughts?

[Corey Farrell]

It might be a problem if BRIDGEPEER is being compared to an empty string to
see if a channel is alone.

What if you made BRIDGEPEER into a built-in channel variable (like
${EPOCH}).  This would mean that you wouldn't be setting any channels,
you'd only do a lookup when requested.  One side-effect is that VarSet
events would never be produced for this variable, not sure how much this
would matter given better events to monitor ConfBridge joins/parts?

On Tue, Aug 9, 2016 at 7:01 PM, Mark Michelson 
wrote:

> Hi folks,
>
> I've been looking into a Digium customer issue where ConfBridge audio has
> been dropping out. The main issue there had to do with DNS, and there is
> currently a review [1] up to fix that.
>
> A secondary issue, though, is that there would be brief audio cutouts when
> participants joined and left the conference. Looking into it, I believe the
> problem is that when a participant enters or leaves the bridge, the
> BRIDGEPEER channel variable is updated for every remaining participant in
> the bridge. The basic algorithm is like this:
>
> * Lock the bridge
> * Iterate through the channels in the bridge (a maximum of 11 of them)
> * Lock the channel
> * Append the channel to an array of channel names
> * Unlock the channel
> * Iterate through the channels in the bridge again (no upper limit this
> time)
> * Lock the channel
> * Set the BRIDGEPEER channel variable using the names in the generated
> array from before (comma-separated)
> * Unlock the channel
> * Unlock the bridge
>
> In addition, this same process occurs every time an announcement is played
> into a bridge, such as join/leave beeps. Playing an announcement involves
> pushing an announcer channel into the bridge and then removing the
> announcer channel when the playback completes.
>
> My question to the list is this: do you find value in having the
> participants in a multi-party bridge packaged into the BRIDGEPEER channel
> variable? I know that for two-party bridges, there are probably lots of
> scripts and dialplans out there that rely on that variable to be set; my
> question specifically is for bridges with more than two parties.
>
> My thoughts on the matter are that since bridges are query-able now
> directly, getting the list of participants from the bridge makes more sense
> than trying to get the participants based on a single channel in that
> bridge. In addition, the code places a hard limit on the number of channel
> names it will actually put in the BRIDGEPEER variable. The code currently
> has a #define that makes it so that only the first 11 channels in the
> bridge will have their names in BRIDGEPEER. Because of the hard-coded
> maximum, if you have more than 11 channels in the bridge, you can't get the
> full list of participants using BRIDGEPEER.
>
> By not setting BRIDGEPEER on channels in multi-party bridges, we can avoid
> holding the bridge ransom while calculating the value and setting the
> variable.
>
> Let me know what your thoughts are on the matter.
>
> Thanks,
> Mark Michelson
>
> [1] https://gerrit.asterisk.org/#/c/3445
>
> --
> _
> -- 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
>
-- 
_
-- 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] AO2 API changes for REF_DEBUG

[Corey Farrell]

Hello everyone,

I've opened ASTERISK-26171 to track my effort to include a "%p" pointer
(debugstorage) in the REF_DEBUG log.  Providing this information to
refcounter.py allows it to remove matching ref and unref from the output.
It also allows detection of indirect leaks where one object leaked because
of another object.  Examples of updated output is posted to JIRA [1],
functional code is on Gerrit [2] and [3].  I'd like to get feedback on the
API before I proceed further.

Tracking of these pointers required a new parameter to be used when
manipulating AO2 references to associate the reference with an address
(debugstorage).  This does not have to be where the reference is actually
being stored and multiple references can be marked as stored at the same
pointer.  Ex: ao2_link/ao2_unlink uses the container as debugstorage
instead of the node.

How should I deal with addition of 'void *debugstorage' to AO2 reference
manipulation API's? The AO2 API's we are looking at are:
__ao2_alloc, __ao2_ref
__ao2_find, __ao2_callback, __ao2_callback_data
__ao2_global_obj_ref (possibly others in the ao2_global_obj namespace)
__ao2_container_alloc_hash, __ao2_container_alloc_list,
__ao2_container_alloc_rbtree, __ao2_container_clone
ao2_iterator_init, __ao2_iterator_next

My current patch suffixes many of the existing functions with _full and
have macro's to cover the original API.  After a comment from Matt Jordan I
think it would be better to just add 'void *debugstorage' parameter to the
current ABI functions.  They are very infrequently used directly, so this
will be small refactor of Asterisk sources.  For each of the ABI functions
listed above, do we want to create a new macro variant that supports
passing debugstorage, or do we add debugstorage to one or more of the
existing macros?  My hope is for the API to encourage use of non-NULL
debugstorage where possible, without being a nuisance where it's not
possible.



Additionally should we have _s prefixed macro's?  Examples from Gerrit
reviews: ao2_s_init, ao2_s_set, ao2_s_cleanup, ao2_s_replace,
ao2_s_container_alloc_hash, ast_s_format_create

The idea is that objects would use the _s API variant to set/unset ao2
objects on member fields and local variables.  In all cases the _s variant
take the first parameter is a pointer to a pointer, in most cases the
pointer is written to.  For example 'myobj->field = ao2_bump_full(value,
"", >field);' can be rewritten as 'ao2_s_set(>field,
value);'.  Beyond the simplest ao2_s_ macro's many are implemented
indirectly using ao2_s_getter.


Links:
[1] https://issues.asterisk.org/jira/browse/ASTERISK-26171
[2] https://gerrit.asterisk.org/3141
[3] https://gerrit.asterisk.org/3161
-- 
_
-- 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] Asterisk Docker Containers: Phase 1

[Corey Farrell]

I make use of a couple patches so the latest Fedora spec is a good enough
base for me.  If other people want to see current binaries for EPEL
hopefully they'll speak up.  I had a chance to look over the asterisk.spec
from Fedora master, I have a couple comments.

* res_ari_mailboxes.so should be moved to the asterisk-mwi-external
sub-package.  It has always depended on res_stasis_mailboxes but that
wasn't listed in menuselect until very recently.

* I don't like that the config files are split across sub-packages.  Having
a dedicated sub-package for all configs makes it easier to have another
package (like a web gui) provide /etc/asterisk.  This would also make it
easier to have a config package for "Super Awesome Company".  I realize
this comment is unlikely to go anywhere, but had to mention it anyways.

On Tuesday, November 24, 2015, Jared Smith 
wrote:

>
> On Thu, Nov 19, 2015 at 9:24 PM, Leif Madsen  > wrote:
>
>> The way it works is that major versions of Asterisk (and same with other
>> packages) are associated with specific releases of Fedora and RHEL, which
>> means the major versions are "stuck" to those releases.
>
>
>
> If it's easier, I'd be happy to setup some repositories for newer versions
> of Asterisk on RHEL/CentOS 6 and RHEL/CentOS 7.
>
> --
> Jared Smith
>
-- 
_
-- 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 Docker Containers: Phase 1

[Corey Farrell]

Leif,

Thanks for explaining I should have read your blog post before replying to
Jared's email about the RPM's.  I didn't realize EPEL was that strict with
version freezes to stay with a no longer supported version.

I do not know too much about Docker yet but I took a look anyways.  Can
variables be used in Dockerfile.asterisk?  The idea is to be able to use a
new version of Asterisk without having to update the version on 40 lines.
Also when someone forks your GIT repo to add/remove packages from the
install list, they won't face 'git rebase' conflicts after you update the
version.

Thanks,
Corey

On Thursday, November 19, 2015, Leif Madsen <leif.mad...@avoxi.com> wrote:

> Hey Corey!
>
> The way it works is that major versions of Asterisk (and same with other
> packages) are associated with specific releases of Fedora and RHEL, which
> means the major versions are "stuck" to those releases.
>
> However, you can still build the newer version of Asterisk by pulling the
> spec file and sources from later Fedora versions (Fedora 23 for instance).
>
> You can rebuild the RPMs supplied using fedpkg, as I've done in phase 1 of
> my blog post. There are other ways you can do it was well, like with mock
> etc.
>
> (You'll need some dependencies in order to build everything).
>
> If you look at my buildit.sh script, it provides the primitives for
> building the dependencies. I'm doing this via a local build with rpmbuild
> (because the script is run in the Docker container), but you could also
> replace this with a fedpkg mock build which would build the RPMs in the
> chroot for you. There might be some things that are slightly more
> complicated in that method, because you'd have to get the resulting
> dependency RPMs into the chroot (possible, it's just not super straight
> forward if you haven't done it before).
>
> I'd probably still suggest doing it in the Docker container, simply
> because it's super simple to get things spun up. You can even use the
> local.repo file I'm providing as well. Basically everything you need to
> build Asterisk for your platform (Red Hat based) is in the repo linked
> above.
>
> Note: this won't work on CentOS 6. There are some oddities with CentOS 6
> in Docker that resulted in me not being able to build Asterisk in the
> CentOS 6 container (gcc complained about not being able to result in a
> binary or something).
>
> Long story short, use fedpkg to build the RPMs from the Fedora
> repositories. Just clone the Fedora 23 version (which gets you Asterisk
> 13.3.2 as of now), and then build it on top of the system you want (CentOS
> 6 or 7 for example).
>
> Thanks!
> Leif.
>
> On Thu, Nov 19, 2015 at 2:23 PM, Corey Farrell <g...@cfware.com> wrote:
> >
> > Jared,
> >
> > I just looked through the EPEL website at EPEL6 and EPEL7, only found
> > Asterisk 1.8.  Can you point me to the spec file you are using or  an
> > SRPM?
> >
> > Thanks,
> > Corey
> >
> > On Thu, Nov 19, 2015 at 2:02 PM, Jared Smith <jaredsm...@jaredsmith.net>
> wrote:
> > >
> > > On Thu, Nov 19, 2015 at 10:14 AM, Matthew Jordan <mjor...@digium.com>
> wrote:
> > >>
> > >> Would it be appropriate to summarize the current state of things as
> "we
> > >> need a spec file for Asterisk"?
> > >
> > >
> > >
> > > At one point, there was an awful .spec file in the Asterisk sources...
> > > hopefully it's not around any more.
> > >
> > > That being said, I just took over as the main maintainer/contact for
> the
> > > Asterisk packages in Fedora/EPEL -- It's one of the most complicated
> spec
> > > files in Fedora, but that obviously hasn't scared me off.
> > >
> > > I'd love feedback on things we can do to make those packages better,
> and get
> > > a tighter feedback loop between the Asterisk development community and
> the
> > > packagers in Fedora/RHEL/CentOS/etc.
> >
>
> --
> Leif Madsen
> Director of Engineering, Product Development
> http://avoxi.com
>
>
-- 
_
-- 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] Asterisk Docker Containers: Phase 1

[Corey Farrell]

Jared,

I just looked through the EPEL website at EPEL6 and EPEL7, only found
Asterisk 1.8.  Can you point me to the spec file you are using or  an
SRPM?

Thanks,
Corey

On Thu, Nov 19, 2015 at 2:02 PM, Jared Smith  wrote:
>
> On Thu, Nov 19, 2015 at 10:14 AM, Matthew Jordan  wrote:
>>
>> Would it be appropriate to summarize the current state of things as "we
>> need a spec file for Asterisk"?
>
>
>
> At one point, there was an awful .spec file in the Asterisk sources...
> hopefully it's not around any more.
>
> That being said, I just took over as the main maintainer/contact for the
> Asterisk packages in Fedora/EPEL -- It's one of the most complicated spec
> files in Fedora, but that obviously hasn't scared me off.
>
> I'd love feedback on things we can do to make those packages better, and get
> a tighter feedback loop between the Asterisk development community and the
> packagers in Fedora/RHEL/CentOS/etc.
>
> --
> Jared Smith
>
> --
> Jared Smith
>
> --
> _
> -- 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

-- 
_
-- 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] Journald support for Asterisk

[Corey Farrell]

CentOS/RHEL 6 are under production support until Nov 2020 [1] and journald
is not available.  Even if journald could improve Asterisk logging, that's
not an acceptable reason to force Asterisk users to upgrade from a
supported OS.  If there was a module that allowed Asterisk logging to talk
to journald I wouldn't care, but it should not replace the logging system
or be required to run Asterisk.

[1]
http://wiki.centos.org/FAQ/General#head-fe8a0be91ee3e7dea812e8694491e1dde5b75e6d

On Sun, May 10, 2015 at 5:22 PM, Ludovic Gasc gml...@gmail.com wrote:

 2015-05-10 20:46 GMT+02:00 Bruce Ferrell bferr...@baywinds.org:

 On 05/09/2015 01:53 PM, Ludovic Gasc wrote:
  2015-05-09 16:15 GMT+02:00 Bruce Ferrell bferr...@baywinds.org
 mailto:bferr...@baywinds.org:
 
  On 05/09/2015 01:17 AM, Ludovic Gasc wrote:
   Hi,
  
   Systemd and Journald is now by default on Debian Jessie and
 Ubuntu 15.04, as on RHEL/CentOS.
   Journald supports syslog format, nevertheless, at least for us,
 the structured log system provided with journald helps us to debug the
 production.
  
   The idea behind that is to attach metadata with a log line to
 facilitate the search with journalctl, you can write queries to find the
 errors.
  
   For example, with Apache:
 http://httpd.apache.org/docs/trunk/mod/mod_journald.html
  
   For our Python daemons, for each log line, we store account_id,
 request_id, endpoint and method used to be easy to retrieve quickly
 interesting logs.
  
   Moreover, not yet used for us, but you can generate statistics
 about your source code real usage based
   on:
 http://www.freedesktop.org/software/systemd/man/systemd.journal-fields.html#CODE_FILE=
  
   For Asterisk, for example with dialplan logging, you should
 attach the context, the extension and the channel.
  
   You can simulate this journald feature with a specific format
 message for your logs, nevertheless, journalctl is more user-friendly to
 retrieve pertinent logs compare to the
   classical grep usage.
   ave no idea if i
   I'm permitted to raise the question about an eventual journald
 support in Asterisk because I don't find anything about that in issues
 tracker nor mailing list.
  
   Moreover, I understand that even if you add the journald support,
 it's certainly necessary to change logging everywhere in Asterisk, however,
 I should help to do that.
  
   Regards
   --
   Ludovic Gasc (GMLudo)
   http://www.gmludo.eu/
  
  
  systemd and journald SUCK to high heaven.
 
 
  I've no problems to believe you, nevertheless, could you be more
 precise ?
  What are the facts that you arrive to this conclusion ?
 
 
I have no idea if the issues I've had with them (OpenSUSE) are
 distro related or inherent.
 
 
  It's issues on your production with several servers ? Or on your laptop
 ?
  Could give us links with unresolved bugs you have ?
  For now, to my experience, with 6 VMs on Debian Jessie on production
 with small clients and Asterisk 13, I've no issues with systemd nor
 journald.
 
 
  I have seen that the implementation of apache with a static
 systemd/journald module will no longer correctly serve content.
 
 
  Could you give me a link about that ?
 I would suggest you try it for yourself.  Procedure to reproduce:

 Perform fresh install of OpenSUSE 13.2
 assure apache/php/MySQL support are installed
 install wordpress from wordpress.org
 Observe CSS is NOT correctly served.

 I did this several times to assure my findings.

 I rebuilt the opensuse rpms from the source rpms to omit systemd/journald
 from the apache build.  Correct operation was now observed


 Interesting, it seems like a regression bug.
 The question now is the patchs from opensuse has broken this module, or
 the bug is in Apache itself.
 The bug seems to be too obvious to be present in Apache source code, but
 who knows ?


 The response to the bug report was Use Nginex


 To be really honest with you, I've thought that when you explain your
 problem, I've dropped Apache on our production a long time ago for
 efficiency issues.



  Please do NOT do this
 

  Why ? If it works for me, what's the problem for you ?

   Systemd integration has been insidious.  Should you wish to discuss
 further, please off this list.

 
 
It also breaks fail2ban site security.
 
 
  I don't understand the issue here, I see at least two solutions:
  1. You can continue to use rsyslog even if you have journald, BTW, it's
 the default setup of Debian Jessie, where journald forwards everything to
 rsyslog to avoid to perturb
  sysadmins with log files.
  2. Apparently, fail2ban supports journald:
 https://github.com/fail2ban/fail2ban/pull/82

 1.) I don't know of any asterisk systems that use any form of system
 logging.  It can, and the fact that you keep bringing syslog logging into
 this indicates a lack of
 understanding of asterisk usage 

[asterisk-dev] asterisk.conf options override command-line options.

[Corey Farrell]

I've discovered that any option set in asterisk.conf cannot be overridden
using the command-line.  For example, if you have verbose=0 in
asterisk.conf, 'asterisk -cvvv' will start with verbose 0.  The same
goes for -f / nofork or -F / alwaysfork.

The only way to fix this is to change the behaviour.  My concern for
release branches is that someone could be running an init script that sets
one option, and they are currently overriding it in asterisk.conf.

Should this be fixed in 11 or 13?  Is anyone against fixing it in master?

JIRA Ticket: https://issues.asterisk.org/jira/browse/ASTERISK-25042
-- 
_
-- 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] A Week with GIT/Gerrit

[Corey Farrell]

My additions to the list:
1) Procedure for 'git review' of security related patches.  I think this
could be done with an asterisk-security mirror repository setup in gerrit
with restricted access.  I know this is already being thought about, just
wanted to make sure it on the list.
2) Is there a command similar to 'git show-branch 13 master' that will
operate on Change-Id's?  Something to show me the first line of all commits
with change-id's that hit one branch but not the other.
3) It would be great if gerrit had the ability to detect the string
ASTERISK-# in commit messages and review comments, convert it to a
JIRA link.
4) 'git review -D' should not generate emails or allow the review to be
seen on the web UI by anyone except the submitter.  I know this is probably
a bug in gerrit, but still worth having on the list.
5) We should add instructions for squashing commits before review somewhere
on the Git or Gerrit usage page.


On Thu, Apr 16, 2015 at 6:00 PM, George Joseph george.jos...@fairview5.com
wrote:


 The Emails:

 Overall I think they're too verbose.
 Change in asterisk[master]: bridge.c: NULL app causes crash during
 attended transfer
 might be better as
 bridge.c: NULL app causes crash during attended transfer
 (asterisk[master])
 It's not a lot shorter but it has the most valuable info at the front.


+1, this would be great.  I'd also like to vote for removing the mailing
list name from subjects on asterisk-code-review.  The list is high enough
volume that any subscribers will have a rule putting all the messages in a
folder, so we don't need the list name repeated in every subject.


While having all the reviews up at the same time is a good thing, I think
 we need to always start with the lowest branch the patch is expected to
 apply to and merge up towards master.   I think the wiki says merge down
 for features and merge up for bugs.  It should be up always.  You don't
 want to start a feature in master only to discover that it can't go into 13
 because some other feature is only in master.  Same for review process.
 Start in the lowest branch and work up.  This will hopefully minimize where
 different people are starting from different directions.  It's hard to
 follow feedback and ensure you've got all the comments covered like that.


I disagree with the argument for merging up features.  I'd prefer all new
features be coded with the best API's of master.  This makes optimized for
master the default position of new features.  Getting new features
back-ported to 13 can be done if wanted, possible, well tested and
compatible.

I do agree with you about reviewing, always start with the oldest posted
branch.  If someone disapproves of the patch against 11, it seems
reasonable to assume they also disapprove of the patch for 13 and master.

For verification of testing, can we combine the Asterisk Testsuite - Full
and Asterisk Unit Tests Bamboo jobs?  The goal is to get a single
coverage report showing data for as many tests as possible.  It would be
really nice if we had a permanent URL for browsing coverage reports of each
branch.  This is related to the question of new features in existing
branches.  If Bamboo says we don't have good coverage of a new feature I
think that's enough reason to pull it from released branches, but to take
that position we need Bamboo to produce a coverage report based on all
available tests.


There's a rebase button on reviews which need it but if you click it, you
 lose you're votes.  :(  We probably need a policy around rebasing reviews.


I have mixed feelings on loss of votes after rebase.  I think it's correct
for the +1 votes to drop, but the -1 votes should be kept.  The emails sent
out should make it clear in the first line of the body that it's just a
rebase.  This way the reviewer can decide if the rebase could have
introduced breakage knowing that the patch itself was unchanged.  Once we
have functional jenkins we might want it so all votes were left alone on
rebase, and jenkins could just -1 if the rebase broke something.


If you move between major releases you'll find various orphaned files and
 directories that are part of one release but not antother.  Use 'git clean
 -fd' to clean them up.  BEWARE:  this will remove all files/directories
 that aren't part of the current branch or in the .gitignore files.   If you
 have local scripts or other stuff you want to keep between checkouts, add
 them to .git/info/exclude.


You can also use 'git clean -fxd' to clean all files, including those which
are ignored.  That is if you don't keep any files in the Asterisk source
folder.  One exception I've noticed, if you clone another repository into
the Asterisk source dir, it will be untouched by 'git clean' from the
Asterisk source dir.
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

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

Re: [asterisk-dev] [Code Review] 4580: Fix issue with Makefile that prevents use of ~/.asterisk.makeopts and /etc/asterisk.makeopts

[Corey Farrell]

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

(Updated April 14, 2015, 3:48 p.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

The Makefile claims that you can set default menuselect options by creating 
~/.asterisk.makeopts or /etc/asterisk.makeopts, but they are never read.  The 
rule for menuselect.makeopts is only allowed to run if the active target is 
'menuselect', but the menuselect target doesn't depend on menuselect.makeopts.  
A dot (wildcard character) was added so the rule will be active for the targets 
that cause it to run: nmenuselect, cmenuselect, and gmenuselect.

When I first enabled this it caused ~/.asterisk.makeopts to overwrite 
./menuselect.makeopts every time I ran 'make menuselect', so I modified the 
Makefile to only consider the file in $HOME and /etc when ./menuselect.makeopts 
doesn't exist.


Thoughts:
* Having two files ($HOME and /etc) at the same time really doesn't make sense 
to me, seems like it would be better to use only the most local file.
* As far as I can tell this has never worked in any currently supported 
version.  I see this as a new for the second time feature, not a bug fix.  
I'm thinking trunk only.  If anyone disagrees let me know.


Diffs
-

  /trunk/Makefile 433966 

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


Testing
---

Repeatedly ran 'make menuselect' with and without files in the 3 locations.  No 
one else has tested this.


Thanks,

Corey Farrell

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

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

Re: [asterisk-dev] [Code Review] 3603: func_jitterbuffer: fix audio failure caused by certain masquerade's

[Corey Farrell]

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

(Updated April 14, 2015, 12:12 a.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers, Joshua Colp and Matt Jordan.


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


Repository: Asterisk


Description
---

During masquerade it is possible for the AST_JITTERBUFFER_FD to be cleared (set 
to -1).  This change adds a check when copying channel fd's to prevent clearing 
an FD with -1.  This seems to resolve the bad audio quality experienced after 
the masquerade.  When AST_JITTERBUFFER_FD was set to -1, this prevented the 
channel from polling that timer.  This caused RTP packets to be received late, 
and discarded.


Diffs
-

  /branches/11/main/channel.c 426593 
  /branches/11/funcs/func_jitterbuffer.c 426593 

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


Testing
---

Verified the scenario outlined in ASTERISK-22409 no longer experiences audio 
quality loss.


Thanks,

Corey Farrell

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

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

[asterisk-dev] Change in asterisk[1.8]: .gitignore: Ignore tarballs (*.gz)

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: .gitignore: Ignore tarballs (*.gz)
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/86
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 1.8
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[master]: git migration: Refactor the ASTERISK_FILE_VERSION macro

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Refactor the ASTERISK_FILE_VERSION macro
..


Patch Set 5: Code-Review+1

Used grep, found a few tests that were still using parameters with the new 
macro.  Fixed them.  Also verified with grep that no references to the old 
macro exist.

-- 
To view, visit https://gerrit.asterisk.org/58
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cf0ff280e1668bf4957dc21f32a5ff43444a40e
Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[13]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 2: Code-Review-1

(1 comment)

Missed one spot where we should conditionally return Asterisk Version.

https://gerrit.asterisk.org/#/c/60/2/main/asterisk.c
File main/asterisk.c:

Line 539:   return NULL;
We need to search for file, return ast_get_version() if it's found.  Return 
NULL if it's not found.


-- 
To view, visit https://gerrit.asterisk.org/60
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-HasComments: Yes

-- 
_
-- 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] 4391: Add blank line between headers and output for Command action response

[Corey Farrell]

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


So I think this looks pretty good.  Next steps:
* We've migrated to git.  Take a look at [1] for information on how to use 
gerrit to post a git review.  Don't worry you won't be facing the full review 
again, we've already dealt with the code issues.
* We need to prepare a patch for starpy [2].  I don't have time to work on this 
right now.  Do you know python or know someone who does and has 
time/willingness to help?
* Verify the full testsuite passes against the latest Asterisk with this change.
* Once starpy supports the change we will coordinate with Digium to ensure 
starpy is updated before we commit this to trunk.

I know this seems like a lot, but we are changing the protocol, so we need to 
make sure we do not break the testsuite.

[1] https://wiki.asterisk.org/wiki/display/AST/Gerrit+Usage
[2] https://github.com/asterisk/starpy/


/trunk/main/manager.c
https://reviewboard.asterisk.org/r/4391/#comment25852

Nit pick: Success or Error is already provided through a standard 
field, so this could be static - Message: Command Output Follows\r\n.  This 
would give a single value for Message that can be matched to determine that we 
successfully processed output, even if there are 0 lines or if the command 
provided the reason for failure in output.


- Corey Farrell


On April 13, 2015, 1 a.m., gareth wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4391/
 ---
 
 (Updated April 13, 2015, 1 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24730
 https://issues.asterisk.org/jira/browse/ASTERISK-24730
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch adds a blank line between the headers and the output in the 
 Command action response. The reason for this change is to make it easier to 
 determine where the headers end and the output from the command starts.
 
 Currently, to parse a response to a Command action, one has to:
 
 1. Read one line, if line is Response: Error, parse the remaining as a 
 standard AMI response and stop.
 2. Read one more line - or two if you used an ActionID - those lines are the 
 headers.
 3. Then read everything up to --END COMMAND--\r\n\r\n.
 
 That could be changed to:
 
 1. Read standard AMI response.
 2. If Response: Follows, then read everything up to --END 
 COMMAND--\r\n\r\n.
 
 The AMI version has been increased to 2.8.0 as this is a protocol change and 
 so that clients detect the new behavior.
 
 Adding a blank line should not cause older parsers to fail as they have to 
 read everything up to --END COMMAND--\r\n\r\n anyway.
 
 Adding a blank line will also not cause the AMI to HTML/XML encoder to fail 
 to separate the headers from the output as it checks for the presence of a 
 : character, which a blank line does not contain.
 
 
 Diffs
 -
 
   /trunk/main/manager.c 434448 
   /trunk/main/cli.c 434448 
   /trunk/include/asterisk/manager.h 434448 
   /trunk/UPGRADE.txt 434448 
 
 Diff: https://reviewboard.asterisk.org/r/4391/diff/
 
 
 Testing
 ---
 
 Connected to manager, issued 'core show uptime' command and verified that 
 there was a blank line between the headers and output.
 
 
 Thanks,
 
 gareth
 


-- 
_
-- 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] Change in asterisk[master]: git migration: Refactor the ASTERISK_FILE_VERSION macro

[Corey Farrell (Code Review)]

Gerrit-PatchSet: 5
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com

-- 
_
-- 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] Change in testsuite[master]: PEP8 fixes

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: PEP8 fixes
..


Patch Set 2:

(2 comments)

The PJSIP tests no longer fail.  I have a full run of the testsuite going now, 
so far no issues.

https://gerrit.asterisk.org/#/c/40/2/lib/python/asterisk/sip_dialog_test_condition.py
File lib/python/asterisk/sip_dialog_test_condition.py:

Line 217: if history_requirements[dialog][req] is False:
I'm unsure this was needed, but a == False is not the same thing as not a, 
for example when a is None.


https://gerrit.asterisk.org/#/c/40/2/lib/python/asterisk/test_case.py
File lib/python/asterisk/test_case.py:

Line 650: if self.passed is False:
This was causing the problem because it is possible for self.passed is None.


-- 
To view, visit https://gerrit.asterisk.org/40
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I55bcaab21c54f9040594f51c57f0efe30a219a62
Gerrit-PatchSet: 2
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- 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] 4586: AMI: Fix improper handling of lines that are exactly 1025 bytes long.

[Corey Farrell]

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

(Updated April 13, 2015, 7:59 a.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

When AMI receives a line that is 1025 bytes long, it sends two error messages.  
Copy the last byte in the buffer to the first postiion, set the length to 1.


Diffs
-

  /branches/11/main/manager.c 433967 

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


Testing
---

Testsuite - tests/manager against Asterisk 13.  All passed.
Manually tested against 11 using David's script posted to JIRA.  Verified a 
single error is received for lines 1025 or more, lines 1024 or less succeed.


Thanks,

Corey Farrell

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

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

[asterisk-dev] Change in asterisk[master]: AMI: Fix improper handling of lines that are exactly 1025 by...

[Corey Farrell (Code Review)]

Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/74

Change subject: AMI: Fix improper handling of lines that are exactly 1025 bytes 
long.
..

AMI: Fix improper handling of lines that are exactly 1025 bytes long.

When AMI receives a line that is 1025 bytes long, it sends two error
messages.  Copy the last byte in the buffer to the first postiion,
set the length to 1.

ASTERISK-20524 #close
Reported by: David M. Lee

Change-Id: Ifda403e2713b59582c715229814fd64a0733c5ea
---
M main/manager.c
1 file changed, 4 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/74/74/1

diff --git a/main/manager.c b/main/manager.c
index d2d9896..fe4d071 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -6239,9 +6239,11 @@
return 1;
}
if (s-session-inlen = maxlen) {
-   /* no crlf found, and buffer full - sorry, too long for us */
+   /* no crlf found, and buffer full - sorry, too long for us
+* keep the last character in case we are in the middle of a 
CRLF. */
ast_log(LOG_WARNING, Discarding message from %s. Line too 
long: %.25s...\n, ast_sockaddr_stringify_addr(s-session-addr), src);
-   s-session-inlen = 0;
+   src[0] = src[s-session-inlen - 1];
+   s-session-inlen = 1;
s-parsing = MESSAGE_LINE_TOO_LONG;
}
res = 0;

-- 
To view, visit https://gerrit.asterisk.org/74
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: Ifda403e2713b59582c715229814fd64a0733c5ea
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com

-- 
_
-- 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] Change in asterisk[master]: Optional API: Fix handling of sources that are both provider...

[Corey Farrell (Code Review)]

Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/73

Change subject: Optional API: Fix handling of sources that are both provider 
and user.
..

Optional API: Fix handling of sources that are both provider and user.

OPTIONAL_API has conditionals to define AST_OPTIONAL_API and
AST_OPTIONAL_API_ATTR differently based on if AST_API_MODULE is defined.
Unfortunately this is inside the include protection block, so only the
first status of AST_API_MODULE is respected.  For example res_monitor
is an optional API provider, but uses func_periodic_hook.  This makes
func_periodic_hook non-optional to res_monitor.

This changes optional_api.h so that AST_OPTIONAL_API and
AST_OPTIONAL_API_ATTR is redefined every time the header is included.

ASTERISK-17608 #close
Reported by: Warren Selby

Change-Id: I8fcf2a5e7b481893e17484ecde4f172c9ffb5679
---
M include/asterisk/optional_api.h
1 file changed, 54 insertions(+), 28 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/73/73/1

diff --git a/include/asterisk/optional_api.h b/include/asterisk/optional_api.h
index 394aed0..1118dc7 100644
--- a/include/asterisk/optional_api.h
+++ b/include/asterisk/optional_api.h
@@ -178,10 +178,7 @@
 
 #define AST_OPTIONAL_API_NAME(name) __##name
 
-#if defined(AST_API_MODULE)
-/* Module defining the API */
-
-#define AST_OPTIONAL_API_IMPL_INIT(name)   \
+#define AST_OPTIONAL_API_INIT_IMPL(name)   \
static void __attribute__((constructor)) __init__##name##_impl(void) { \
ast_optional_api_provide(#name, \
(ast_optional_fn)AST_OPTIONAL_API_NAME(name));  \
@@ -191,22 +188,7 @@
(ast_optional_fn)AST_OPTIONAL_API_NAME(name));  \
}
 
-#define AST_OPTIONAL_API(result, name, proto, stub)\
-   result AST_OPTIONAL_API_NAME(name) proto;   \
-   static attribute_unused typeof(AST_OPTIONAL_API_NAME(name)) * const \
-name = AST_OPTIONAL_API_NAME(name);\
-   AST_OPTIONAL_API_IMPL_INIT(name)
-
-#define AST_OPTIONAL_API_ATTR(result, attr, name, proto, stub) \
-   result  __attribute__((attr)) AST_OPTIONAL_API_NAME(name) proto; \
-   static attribute_unused typeof(AST_OPTIONAL_API_NAME(name)) * const \
-name = AST_OPTIONAL_API_NAME(name);\
-   AST_OPTIONAL_API_IMPL_INIT(name)
-
-#else
-/* Module using the API */
-
-#define AST_OPTIONAL_API_INIT(name)\
+#define AST_OPTIONAL_API_INIT_USER(name)   \
static void __attribute__((constructor)) __init__##name(void) { \
ast_optional_api_use(#name, (ast_optional_fn *)name,   \
(ast_optional_fn)__stub__##name,\
@@ -217,19 +199,31 @@
AST_MODULE);\
}
 
-#define AST_OPTIONAL_API(result, name, proto, stub)\
+#define AST_OPTIONAL_API_IMPL(result, name, proto, stub)   
\
+   result AST_OPTIONAL_API_NAME(name) proto;   \
+   static attribute_unused typeof(AST_OPTIONAL_API_NAME(name)) * const \
+name = AST_OPTIONAL_API_NAME(name);\
+   AST_OPTIONAL_API_INIT_IMPL(name)
+
+#define AST_OPTIONAL_API_USER(result, name, proto, stub)   
\
static result __stub__##name proto stub;\
static attribute_unused \
typeof(__stub__##name) * name;  \
-   AST_OPTIONAL_API_INIT(name)
+   AST_OPTIONAL_API_INIT_USER(name)
 
-#define AST_OPTIONAL_API_ATTR(result, attr, name, proto, stub) \
+
+/* AST_OPTIONAL_API_ATTR */
+#define AST_OPTIONAL_API_ATTR_IMPL(result, attr, name, proto, stub)
\
+   result  __attribute__((attr)) AST_OPTIONAL_API_NAME(name) proto; \
+   static attribute_unused typeof(AST_OPTIONAL_API_NAME(name)) * const \
+name = AST_OPTIONAL_API_NAME(name);\
+   AST_OPTIONAL_API_INIT_IMPL(name)
+
+#define AST_OPTIONAL_API_ATTR_USER(result, attr, name, proto, stub)
\
static __attribute__((attr)) result __stub__##name proto stub;  \
static attribute_unused __attribute__((attr))   \
typeof(__stub__##name) * name;  \
-   AST_OPTIONAL_API_INIT(name)
-
-#endif /* defined(AST_API_MODULE) */
+   AST_OPTIONAL_API_INIT_USER(name)
 
 #else /* defined(OPTIONAL_API) */
 
@@ -245,6 +239,38 @@
 
 #endif /* defined(OPTIONAL_API) */
 
-#undef AST_API_MODULE
-
 #endif /* __ASTERISK_OPTIONAL_API_H */
+
+/*
+ * Some Asterisk sources

[asterisk-dev] Change in asterisk[13]: res_monitor: Add dependency on func_periodic_hook.

[Corey Farrell (Code Review)]

Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/72

Change subject: res_monitor: Add dependency on func_periodic_hook.
..

res_monitor: Add dependency on func_periodic_hook.

OPTIONAL_API has conditionals to define AST_OPTIONAL_API and
AST_OPTIONAL_API_ATTR differently based on if AST_API_MODULE is defined.
Unfortunately this is inside the include protection block, so only the
first status of AST_API_MODULE is respected.  For example res_monitor
is an optional API provider, but uses func_periodic_hook.  This makes
func_periodic_hook non-optional to res_monitor.

ASTERISK-17608 #close
Reported by: Warren Selby

Change-Id: I8fcf2a5e7b481893e17484ecde4f172c9ffb5679
---
M res/res_monitor.c
1 file changed, 1 insertion(+), 0 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/72/72/1

diff --git a/res/res_monitor.c b/res/res_monitor.c
index 872f565..ebf9843 100644
--- a/res/res_monitor.c
+++ b/res/res_monitor.c
@@ -24,6 +24,7 @@
  */
 
 /*** MODULEINFO
+   depend type=modulefunc_periodic_hook/depend
support_levelcore/support_level
  ***/
  

-- 
To view, visit https://gerrit.asterisk.org/72
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I8fcf2a5e7b481893e17484ecde4f172c9ffb5679
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Corey Farrell g...@cfware.com

-- 
_
-- 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] 4590: Optional API does not work for sources that are both provider and user of optional API's.

[Corey Farrell]

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

(Updated April 13, 2015, 7:37 a.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers.


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


Repository: Asterisk


Description
---

OPTIONAL_API has conditionals to define AST_OPTIONAL_API and 
AST_OPTIONAL_API_ATTR differently based on if AST_API_MODULE is defined.  
Unfortunately this is inside the #ifndef __ASTERISK_OPTIONAL_API_H include 
protection block, so only the first status of AST_API_MODULE is respected.  For 
example res_monitor is a provider, but uses beep.h (func_periodic_hook).  This 
makes func_periodic_hook non-optional to res_monitor.

The patch applies cleanly to 13 and fixes the issue, but I'm not sure this 
should be fixed in 13.  Although there is no ABI change, it is a change in 
behaviour for sources that #include optional_api.h.  If this is wanted for 13 
please let me know, otherwise I will follow up with a patch for 13 to make 
res_monitor require func_periodic_hook.


Diffs
-

  /trunk/include/asterisk/optional_api.h 434423 

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


Testing
---

Verified I could load res_monitor with or without func_periodic_hook.  Ran a 
couple testsuite tests.


Thanks,

Corey Farrell

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

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

[asterisk-dev] Change in asterisk[master]: Optional API: Fix handling of sources that are both provider...

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: Optional API: Fix handling of sources that are both provider 
and user.
..


Patch Set 1:

This was previously posted for review at 
https://reviewboard.asterisk.org/r/4590/

-- 
To view, visit https://gerrit.asterisk.org/73
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I8fcf2a5e7b481893e17484ecde4f172c9ffb5679
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[13]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 3: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/60
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[11]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 3: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/61
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in testsuite[master]: PEP8 fixes

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: PEP8 fixes
..


Patch Set 2: -Code-Review

It appears that it's not this change that is broken, it's my computer.

-- 
To view, visit https://gerrit.asterisk.org/40
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I55bcaab21c54f9040594f51c57f0efe30a219a62
Gerrit-PatchSet: 2
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in testsuite[master]: PEP8 fixes

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: PEP8 fixes
..


Patch Set 2: Code-Review-1

Still working out an issue.

-- 
To view, visit https://gerrit.asterisk.org/40
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I55bcaab21c54f9040594f51c57f0efe30a219a62
Gerrit-PatchSet: 2
Gerrit-Project: testsuite
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[13]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/60
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 13
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[11]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 1: Code-Review+1

(1 comment)

So I'm a bit indifferent about the finding against main/asterisk.c, up to you 
if you want to address or not.  Please keep the functionality for 11 and 13 the 
same if you do address it.

https://gerrit.asterisk.org/#/c/61/1/main/asterisk.c
File main/asterisk.c:

Line 390:   return NULL;
Sorry I didn't think of this in the review for 13, maybe we should check for 
the file, return the Asterisk version if found.  If not found then return NULL.

This way if something uses this to check that a source is registered it will 
return NULL or not NULL the same as before.

On the other hand I'm not sure how useful this is, just thinking of edge cases 
where this could break compatibility.


-- 
To view, visit https://gerrit.asterisk.org/61
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-HasComments: Yes

-- 
_
-- 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] Change in asterisk[11]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/61/1/main/asterisk.c
File main/asterisk.c:

Line 390:   return NULL;
 If we did that, we'd have to keep the version field on the struct, and we'd
You misunderstand.  What I mean is:
printf(Version: %s\n, ast_file_version_find(chan_sip.c));
This would print Version: 11.17.0\n if chan_sip is loaded.

This way the result of this function for file=chan_sip.c is still not NULL, 
the result for file=chan_this_doesnt_exist.c is still NULL.


-- 
To view, visit https://gerrit.asterisk.org/61
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- 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] Change in asterisk[11]: git migration: Remove support for file versions

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Remove support for file versions
..


Patch Set 1: -Code-Review

(1 comment)

https://gerrit.asterisk.org/#/c/61/1/main/asterisk.c
File main/asterisk.c:

Line 390:   return NULL;
 That's a very good idea.
Agreed on the CLI command.

The AMI action is to check if a module is loaded, so even without the file 
version functionality it is still useful.  Additionally I'm thinking that in 
trunk we could add a version string to the module info structure, so the module 
check response would provide a useful version again.  Built-in modules would 
just use the version of Asterisk they were built against, 3rd party modules 
would have the option to provide their own version string.

So for the AMI action in 11/13 we should note in UPGRADE that the Version field 
is no longer useful.


-- 
To view, visit https://gerrit.asterisk.org/61
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ia932d3c64cd18a14a3c894109baa657ec0a85d28
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: 11
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- 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] 4108: Weak Proxy Objects

[Corey Farrell]

 On April 10, 2015, 7:05 p.m., rmudgett wrote:
 

I'm posting my next diff here, then I will discard this review then post the 
same change to gerrit.  This way you can look at reviewboard to see the changes 
between patches.


 On April 10, 2015, 7:05 p.m., rmudgett wrote:
  /trunk/main/astobj2.c, lines 492-494
  https://reviewboard.asterisk.org/r/4108/diff/5-6/?file=71780#file71780line492
 
  The comment doesn't make sense.  How is destructor_fn supposed to 
  access values in the weak proxy?  The real object is no longer linked with 
  the weakproxy.
 
 Corey Farrell wrote:
 The following is allowed and a reasonable thing to do:
 weakproxy-name = ast_strdup(name);
 realobj-name = weakproxy-name;
 
 The idea is that weakproxy has to own storage of any shared fields since 
 realobj could be destroyed at any time.  So if the destructor uses 
 realobj-name, it is accessing memory that is owned by the weakproxy.

While addressing the other finding about the ref count games, I realized this 
comment no longer applies.  ao2_ref(user_data, -1) happens inside the lock of 
weakproxy, and ao2_ref(weakproxy, -1) must happen after it's unlocked.  
Originally it wasn't clear without a comment that the weakproxy release 
couldn't be moved higher in internal_ao2_ref.


 On April 10, 2015, 7:05 p.m., rmudgett wrote:
  /trunk/main/astobj2.c, lines 455-463
  https://reviewboard.asterisk.org/r/4108/diff/5-6/?file=71780#file71780line455
 
  Rather than play games with the real object's ref count.  How about 
  replacing the indicated code with this:
  --
  /* Unlink the obj from the weak proxy */
  internal_weakproxy-priv_data.weakptr = NULL;
  obj-priv_data.weakptr = NULL;
  
  /* Notify the subscribers that obj is about to die. */
  weakproxy_run_callbacks(weakproxy);
  
  /*
   * Weak is already unlinked from obj so this won't recurse.
   * This will destroy obj.
   */
  ao2_ref(user_data, -1);
  --
  
  The test to release the weakproxy ref below needs to be adjusted to:
  if (current_value == 1)
  and that code moved to just after the unlock of weakproxy.

Retested, although the last two reference lines are out of order this is 
tolerated by refcounter.py.


- Corey


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


On April 3, 2015, 12:58 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4108/
 ---
 
 (Updated April 3, 2015, 12:58 p.m.)
 
 
 Review request for Asterisk Developers, George Joseph and rmudgett.
 
 
 Bugs: ASTERISK-24936
 https://issues.asterisk.org/jira/browse/ASTERISK-24936
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This implements weak references.  The weakproxy object is a real ao2 with 
 normal reference counting of its own.  When a weakproxy is pointed to a 
 normal object they hold references to each other.  The normal object is 
 automatically freed when a single reference remains (the weakproxy).  The 
 weakproxy also supports subscriptions that will notify callbacks when it does 
 not point to any real object.
 
 
 Diffs
 -
 
   /trunk/tests/test_astobj2_weaken.c PRE-CREATION 
   /trunk/main/astobj2.c 433964 
   /trunk/include/asterisk/astobj2.h 433964 
 
 Diff: https://reviewboard.asterisk.org/r/4108/diff/
 
 
 Testing
 ---
 
 Ran the included test with REF_DEBUG enabled under valgrind.  No reference 
 leaks or improper memory access.  Though this does not test for races, I 
 don't know of an automated way to do that.
 
 
 Thanks,
 
 Corey Farrell
 


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

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

Re: [asterisk-dev] [Code Review] 4108: Weak Proxy Objects

[Corey Farrell]

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

(Updated April 12, 2015, 5:24 a.m.)


Review request for Asterisk Developers, George Joseph and rmudgett.


Changes
---

Addressed findings.

Moved AO2_DEBUG section next to the other atomic increments.


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


Repository: Asterisk


Description
---

This implements weak references.  The weakproxy object is a real ao2 with 
normal reference counting of its own.  When a weakproxy is pointed to a normal 
object they hold references to each other.  The normal object is automatically 
freed when a single reference remains (the weakproxy).  The weakproxy also 
supports subscriptions that will notify callbacks when it does not point to any 
real object.


Diffs (updated)
-

  /trunk/tests/test_astobj2_weaken.c PRE-CREATION 
  /trunk/main/astobj2.c 434545 
  /trunk/include/asterisk/astobj2.h 434545 

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


Testing
---

Ran the included test with REF_DEBUG enabled under valgrind.  No reference 
leaks or improper memory access.  Though this does not test for races, I don't 
know of an automated way to do that.


Thanks,

Corey Farrell

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

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

[asterisk-dev] Change in asterisk[master]: astobj2: Add support for weakproxy objects.

[Corey Farrell (Code Review)]

Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/56

Change subject: astobj2: Add support for weakproxy objects.
..

astobj2: Add support for weakproxy objects.

This implements weak references.  The weakproxy object is a real ao2 with
normal reference counting of its own.  When a weakproxy is pointed to a normal
object they hold references to each other.  The normal object is automatically
freed when a single reference remains (the weakproxy).  The weakproxy also
supports subscriptions that will notify callbacks when it does not point
to any real object.

ASTERISK-24936 #close
Reported by: Corey Farrell

Change-Id: Ib9f73c02262488d314d9d9d62f58165b9ec43c67
---
M include/asterisk/astobj2.h
M main/astobj2.c
A tests/test_astobj2_weaken.c
3 files changed, 640 insertions(+), 2 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/56/56/1

diff --git a/include/asterisk/astobj2.h b/include/asterisk/astobj2.h
index 692cc7c..1c3c2e8 100644
--- a/include/asterisk/astobj2.h
+++ b/include/asterisk/astobj2.h
@@ -19,6 +19,7 @@
 
 #include asterisk/compat.h
 #include asterisk/lock.h
+#include asterisk/linkedlists.h
 
 /*! \file
  * \ref AstObj2
@@ -578,6 +579,155 @@
 
 /*! @} */
 
+/*! \brief ao2_weakproxy
+ *
+ * @{
+ */
+struct ao2_weakproxy_notification;
+typedef void (*ao2_weakproxy_notification_cb)(void *weakproxy, void *data);
+
+/*! \brief This struct should be opaque, but it's size is needed. */
+struct ao2_weakproxy {
+   AST_LIST_HEAD_NOLOCK(, ao2_weakproxy_notification) destroyed_cb;
+};
+
+/*! \brief Macro which must be used at the beginning of weakproxy capable 
objects.
+ *
+ * \note The primary purpose of user defined fields on weakproxy objects is to 
hold
+ *   immutable container keys for the real object.
+ */
+#define AO2_WEAKPROXY() struct ao2_weakproxy __weakproxy##__LINE__
+
+/*!
+ * \since 14.0.0
+ * \brief Allocate an ao2_weakproxy object
+ *
+ * \param data_size The sizeof() of the user-defined structure.
+ * \param destructor_fn The destructor function (can be NULL)
+ *
+ * \note struct ao2_weakproxy must be the first field of any object.
+ *   This can be done by using AO2_WEAKPROXY to declare your structure.
+ */
+void *__ao2_weakproxy_alloc(size_t data_size, ao2_destructor_fn destructor_fn,
+   const char *tag, const char *file, int line, const char *func) 
attribute_warn_unused_result;
+
+#define ao2_weakproxy_alloc(data_size, destructor_fn) \
+   __ao2_weakproxy_alloc(data_size, destructor_fn, , __FILE__, __LINE__, 
__PRETTY_FUNCTION__)
+
+#define ao2_t_weakproxy_alloc(data_size, destructor_fn, tag) \
+   __ao2_weakproxy_alloc(data_size, destructor_fn, tag, __FILE__, 
__LINE__, __PRETTY_FUNCTION__)
+
+/*!
+ * \since 14.0.0
+ * \brief Associate weakproxy with obj.
+ *
+ * \param weakproxy An object created by ao2_weakproxy_alloc.
+ * \param obj An ao2 object not created by ao2_weakproxy_alloc.
+ * \param flags OBJ_NOLOCK to avoid locking weakproxy.
+ *
+ * \retval 0 Success
+ * \retval -1 Failure
+ *
+ * \note obj must be newly created, this procedure is not thread safe
+ *   if any other code can reach obj before this procedure ends.
+ *
+ * \note weakproxy may be previously existing, but must not currently
+ *   have an object set.
+ *
+ * \note The only way to unset an object is for it to be destroyed.
+ *   Any call to this function while an object is already set will fail.
+ */
+int __ao2_weakproxy_set_object(void *weakproxy, void *obj, int flags,
+   const char *tag, const char *file, int line, const char *func);
+
+#define ao2_weakproxy_set_object(weakproxy, obj, flags) \
+   __ao2_weakproxy_set_object(weakproxy, obj, flags, , __FILE__, 
__LINE__, __PRETTY_FUNCTION__)
+
+#define ao2_t_weakproxy_set_object(weakproxy, obj, flags, tag) \
+   __ao2_weakproxy_set_object(weakproxy, obj, flags, tag, __FILE__, 
__LINE__, __PRETTY_FUNCTION__)
+
+/*!
+ * \since 14.0.0
+ * \brief Get the object associated with weakproxy.
+ *
+ * \param weakproxy The weakproxy to read from.
+ * \param flags OBJ_NOLOCK to avoid locking weakproxy.
+ *
+ * \return A reference to the object previously set by 
ao2_weakproxy_set_object.
+ * \retval NULL Either no object was set or the previously set object has been 
freed.
+ */
+void *__ao2_weakproxy_get_object(void *weakproxy, int flags,
+   const char *tag, const char *file, int line, const char *func) 
attribute_warn_unused_result;
+
+#define ao2_weakproxy_get_object(weakproxy, flags) \
+   __ao2_weakproxy_get_object(weakproxy, flags, , __FILE__, __LINE__, 
__PRETTY_FUNCTION__)
+
+#define ao2_t_weakproxy_get_object(weakproxy, flags, tag) \
+   __ao2_weakproxy_get_object(weakproxy, flags, tag, __FILE__, __LINE__, 
__PRETTY_FUNCTION__)
+
+/*!
+ * \since 14.0.0
+ * \brief Request notification when weakproxy points to NULL.
+ *
+ * \param weakproxy The weak object
+ * \param cb

Re: [asterisk-dev] [Code Review] 4108: Weak Proxy Objects

[Corey Farrell]

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

(Updated April 12, 2015, 5:29 a.m.)


Status
--

This change has been discarded.


Review request for Asterisk Developers, George Joseph and rmudgett.


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


Repository: Asterisk


Description
---

This implements weak references.  The weakproxy object is a real ao2 with 
normal reference counting of its own.  When a weakproxy is pointed to a normal 
object they hold references to each other.  The normal object is automatically 
freed when a single reference remains (the weakproxy).  The weakproxy also 
supports subscriptions that will notify callbacks when it does not point to any 
real object.


Diffs
-

  /trunk/tests/test_astobj2_weaken.c PRE-CREATION 
  /trunk/main/astobj2.c 434545 
  /trunk/include/asterisk/astobj2.h 434545 

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


Testing
---

Ran the included test with REF_DEBUG enabled under valgrind.  No reference 
leaks or improper memory access.  Though this does not test for races, I don't 
know of an automated way to do that.


Thanks,

Corey Farrell

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

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

[asterisk-dev] Change in asterisk[master]: git migration: Refactor the ASTERISK_FILE_VERSION macro

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Refactor the ASTERISK_FILE_VERSION macro
..


Patch Set 3: Code-Review-1

(9 comments)

https://gerrit.asterisk.org/#/c/54/3//COMMIT_MSG
Commit Message:

Line 22: Other facilities other
Remove one of the other's.


https://gerrit.asterisk.org/#/c/54/3/include/asterisk.h
File include/asterisk.h:

Line 164: void ast_register_file(const char *file);
Given that this is meant for use through a macro only, shouldn't it be 
__ast_register_file?  Ditto for ast_unregister_file.


Line 203:  * ASTERISK_REGISTER_FILE(__FILE__, \$Revision\$)
Update parameters.


Line 206: * \note The dollar signs above have been protected with backslashes 
to keep
:  * SVN from modifying them in this file; under normal circumstances 
they would
:  * not be present and SVN would expand the Revision keyword into the 
file's
:  * revision number.
No longer applies.


Line 213: #define ASTERISK_REGISTER_FILE(file) \
Couldn't we drop the 'file' parameter too, just use __FILE__ inside the macro?


https://gerrit.asterisk.org/#/c/54/3/main/manager.c
File main/manager.c:

Line 5998:  snprintf(cut, (sizeof(filename) - strlen(filename)) - 1, .c);
 :  ast_debug(1,  ModuleCheck .c file %s\n, filename);
Since we're no longer checking the source file registration I think this should 
be removed.  Also it's the wrong filename for C++ modules (like chan_vpb.cc).


https://gerrit.asterisk.org/#/c/54/3/utils/check_expr.c
File utils/check_expr.c:

Line 155: //void ast_register_file(const char *file, const char *version);
: //void ast_unregister_file(const char *file);
Delete..


Line 164: void ast_register_file(const char *file, const char *version);  
Tailing whitespace


Line 167: int ast_add_profile(const char *x, uint64_t scale) { return 0;} 
Tailing whitespace


-- 
To view, visit https://gerrit.asterisk.org/54
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Id146f24e9d968dccd64cb09dbfa1da3d9ac719f6
Gerrit-PatchSet: 3
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- 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] Change in asterisk[master]: main/editline: Add .gitignore.

[Corey Farrell (Code Review)]

Corey Farrell has uploaded a new change for review.

  https://gerrit.asterisk.org/57

Change subject: main/editline: Add .gitignore.
..

main/editline: Add .gitignore.

This patch adds a .gitignore for main/editline to ignore all build results.

Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
---
A main/editline/.gitignore
1 file changed, 13 insertions(+), 0 deletions(-)


  git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/57/57/1

diff --git a/main/editline/.gitignore b/main/editline/.gitignore
new file mode 100644
index 000..d3bb06b
--- /dev/null
+++ b/main/editline/.gitignore
@@ -0,0 +1,13 @@
+*.o_a
+Makefile
+common.h
+config.cache
+config.h
+editline.c
+emacs.h
+fcns.c
+fcns.h
+help.c
+help.h
+makelist
+vi.h

-- 
To view, visit https://gerrit.asterisk.org/57
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: newchange
Gerrit-Change-Id: I68c7bf375ea46282689e5a706534b69fca233b5d
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Corey Farrell g...@cfware.com

-- 
_
-- 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] Change in asterisk[master]: git migration: Refactor the ASTERISK_FILE_VERSION macro

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: git migration: Refactor the ASTERISK_FILE_VERSION macro
..


Patch Set 1: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/58
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I6cf0ff280e1668bf4957dc21f32a5ff43444a40e
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[master]: Add .gitignore and .gitreview files

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: Add .gitignore and .gitreview files
..


Patch Set 2: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/42
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[master]: Add .gitignore and .gitreview files

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: Add .gitignore and .gitreview files
..


Patch Set 1:

(2 comments)

Couple minor things then this is good to go.

https://gerrit.asterisk.org/#/c/42/1/.gitignore
File .gitignore:

Line 6: # *~
I suspect we do want to ignore *~


Line 10: **.so
Please add *.eo for embedded objects.  Also *.ii, *.oo for C++ modules.

Honestly I'm not sure the purpose of the two stars in **.so or others like 
this.  Why not just use *.so?  That would apply to all dirs.


-- 
To view, visit https://gerrit.asterisk.org/42
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I842a1588ff27d8a0189f12d597f0a7af033d6c69
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: George Joseph george.jos...@fairview5.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: Yes

-- 
_
-- 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] Change in asterisk[master]: sounds: Add a .gitignore file for downloaded sound tarballs

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: sounds: Add a .gitignore file for downloaded sound tarballs
..


Patch Set 1: Code-Review-1

Any reason we can't just add to the root .gitignore:
*.gz

This way if we eventually add a 'make dist' target it will be covered.

-- 
To view, visit https://gerrit.asterisk.org/55
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-HasComments: No

-- 
_
-- 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] Change in asterisk[master]: .gitignore: Ignore tarballs (*.gz)

[Corey Farrell (Code Review)]

Corey Farrell has posted comments on this change.

Change subject: .gitignore: Ignore tarballs (*.gz)
..


Patch Set 2: Code-Review+1

-- 
To view, visit https://gerrit.asterisk.org/55
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: Ie84f085cc0fa51262209e7bfc1b1ba8c04a1ef59
Gerrit-PatchSet: 2
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Matt Jordan mjor...@digium.com
Gerrit-Reviewer: Corey Farrell g...@cfware.com
Gerrit-Reviewer: Matt Jordan mjor...@digium.com
Gerrit-HasComments: No

-- 
_
-- 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] 4108: Weak Proxy Objects

[Corey Farrell]

 On April 10, 2015, 7:05 p.m., rmudgett wrote:
  /trunk/main/astobj2.c, lines 492-494
  https://reviewboard.asterisk.org/r/4108/diff/5-6/?file=71780#file71780line492
 
  The comment doesn't make sense.  How is destructor_fn supposed to 
  access values in the weak proxy?  The real object is no longer linked with 
  the weakproxy.

The following is allowed and a reasonable thing to do:
weakproxy-name = ast_strdup(name);
realobj-name = weakproxy-name;

The idea is that weakproxy has to own storage of any shared fields since 
realobj could be destroyed at any time.  So if the destructor uses 
realobj-name, it is accessing memory that is owned by the weakproxy.


- Corey


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


On April 3, 2015, 12:58 p.m., Corey Farrell wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4108/
 ---
 
 (Updated April 3, 2015, 12:58 p.m.)
 
 
 Review request for Asterisk Developers, George Joseph and rmudgett.
 
 
 Bugs: ASTERISK-24936
 https://issues.asterisk.org/jira/browse/ASTERISK-24936
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This implements weak references.  The weakproxy object is a real ao2 with 
 normal reference counting of its own.  When a weakproxy is pointed to a 
 normal object they hold references to each other.  The normal object is 
 automatically freed when a single reference remains (the weakproxy).  The 
 weakproxy also supports subscriptions that will notify callbacks when it does 
 not point to any real object.
 
 
 Diffs
 -
 
   /trunk/tests/test_astobj2_weaken.c PRE-CREATION 
   /trunk/main/astobj2.c 433964 
   /trunk/include/asterisk/astobj2.h 433964 
 
 Diff: https://reviewboard.asterisk.org/r/4108/diff/
 
 
 Testing
 ---
 
 Ran the included test with REF_DEBUG enabled under valgrind.  No reference 
 leaks or improper memory access.  Though this does not test for races, I 
 don't know of an automated way to do that.
 
 
 Thanks,
 
 Corey Farrell
 


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

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

Re: [asterisk-dev] [Code Review] 4391: Add blank line between headers and output for Command action response

[Corey Farrell]

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



/trunk/main/cli.c
https://reviewboard.asterisk.org/r/4391/#comment25830

These two lines should be replaced with 'goto done;'.  This way we will 
return RESULT_FAILURE.


- Corey Farrell


On April 9, 2015, 1:05 a.m., gareth wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4391/
 ---
 
 (Updated April 9, 2015, 1:05 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24730
 https://issues.asterisk.org/jira/browse/ASTERISK-24730
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch adds a blank line between the headers and the output in the 
 Command action response. The reason for this change is to make it easier to 
 determine where the headers end and the output from the command starts.
 
 Currently, to parse a response to a Command action, one has to:
 
 1. Read one line, if line is Response: Error, parse the remaining as a 
 standard AMI response and stop.
 2. Read one more line - or two if you used an ActionID - those lines are the 
 headers.
 3. Then read everything up to --END COMMAND--\r\n\r\n.
 
 That could be changed to:
 
 1. Read standard AMI response.
 2. If Response: Follows, then read everything up to --END 
 COMMAND--\r\n\r\n.
 
 The AMI version has been increased to 2.8.0 as this is a protocol change and 
 so that clients detect the new behavior.
 
 Adding a blank line should not cause older parsers to fail as they have to 
 read everything up to --END COMMAND--\r\n\r\n anyway.
 
 Adding a blank line will also not cause the AMI to HTML/XML encoder to fail 
 to separate the headers from the output as it checks for the presence of a 
 : character, which a blank line does not contain.
 
 
 Diffs
 -
 
   /trunk/main/manager.c 434448 
   /trunk/main/cli.c 434448 
   /trunk/include/asterisk/manager.h 434448 
   /trunk/UPGRADE.txt 434448 
 
 Diff: https://reviewboard.asterisk.org/r/4391/diff/
 
 
 Testing
 ---
 
 Connected to manager, issued 'core show uptime' command and verified that 
 there was a blank line between the headers and output.
 
 
 Thanks,
 
 gareth
 


-- 
_
-- 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] 4391: Add blank line between headers and output for Command action response

[Corey Farrell]

 On April 3, 2015, 6:37 a.m., Corey Farrell wrote:
  /trunk/main/manager.c, line 4873
  https://reviewboard.asterisk.org/r/4391/diff/3/?file=72904#file72904line4873
 
  If we successfully ran the command, it seems unsafe to claim failure.  
  We have to assume the the caller doesn't actually care about any of the CLI 
  output, they only care about having the command perform an action.  So I 
  think we need to respond with success if the command ran.  I'm leaning 
  towards thinking that this error should be provided through a single 
  Output: Command response construction error\r\n, so move astman_start_ack 
  to just below ast_cli_command.
  
  On a related issue, there are a couple errors that can occur in 
  ast_cli_command_full which print error messages and return success.  I 
  don't know if it's safe to modify ast_cli_command_full to return errors for 
  more situations, it might be worth looking at to allow us to trust the 
  return value of ast_cli_command_full.  CLI commands themselves can return 
  an error, but this error is not returned by ast_cli_command_full.  It would 
  be nice if this action could use the return value from ast_cli_command_full 
  to determine if it should respond success or failure.
 
 gareth wrote:
 If the caller is executing any of the 'module show ...' commands then 
 they likely care about the output. In this case, I think it would be better 
 to provide a more descriptive error message to the caller so they can detect 
 if the command was executed.
 
 Yes, ast_cli_commmand_full should indicate whether the command failed, I 
 will modify it so that an Error response can sent if the command fails. There 
 don't appear to be any callers of that function who check the return code.

I do not feel this issue is resolved.  If a command has side-effects, we should 
respond with Success/Failure based on if the command ran - regardless of our 
(in)ability to process output.  Since there is no way to determine if a CLI 
command is meant to retrieve information or manipulate it, we have to assume 
that it is manipulating something.  So I agree that descriptive error messages 
are useful, we need to avoid lying to the caller by claiming that the command 
failed to run if it did run.

For times where people are just retrieving information, couldn't we respond 
with success, put the error in 'Message:', and provide no 'Output:' headers?  
This way if you care about the output, you can detect the lack of output.


- Corey


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


On April 9, 2015, 1:05 a.m., gareth wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4391/
 ---
 
 (Updated April 9, 2015, 1:05 a.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24730
 https://issues.asterisk.org/jira/browse/ASTERISK-24730
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 This patch adds a blank line between the headers and the output in the 
 Command action response. The reason for this change is to make it easier to 
 determine where the headers end and the output from the command starts.
 
 Currently, to parse a response to a Command action, one has to:
 
 1. Read one line, if line is Response: Error, parse the remaining as a 
 standard AMI response and stop.
 2. Read one more line - or two if you used an ActionID - those lines are the 
 headers.
 3. Then read everything up to --END COMMAND--\r\n\r\n.
 
 That could be changed to:
 
 1. Read standard AMI response.
 2. If Response: Follows, then read everything up to --END 
 COMMAND--\r\n\r\n.
 
 The AMI version has been increased to 2.8.0 as this is a protocol change and 
 so that clients detect the new behavior.
 
 Adding a blank line should not cause older parsers to fail as they have to 
 read everything up to --END COMMAND--\r\n\r\n anyway.
 
 Adding a blank line will also not cause the AMI to HTML/XML encoder to fail 
 to separate the headers from the output as it checks for the presence of a 
 : character, which a blank line does not contain.
 
 
 Diffs
 -
 
   /trunk/main/manager.c 434448 
   /trunk/main/cli.c 434448 
   /trunk/include/asterisk/manager.h 434448 
   /trunk/UPGRADE.txt 434448 
 
 Diff: https://reviewboard.asterisk.org/r/4391/diff/
 
 
 Testing
 ---
 
 Connected to manager, issued 'core show uptime' command and verified that 
 there was a blank line between the headers and output.
 
 
 Thanks,
 
 gareth
 


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

asterisk-dev mailing list
To UNSUBSCRIBE or update

Re: [asterisk-dev] [Code Review] 4608: res_pjsip_phoneprov_provider: Fix reference leak on unload

[Corey Farrell]

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

Ship it!


Ship It!

- Corey Farrell


On April 9, 2015, 3:57 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4608/
 ---
 
 (Updated April 9, 2015, 3:57 p.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Bugs: ASTERISK-24935
 https://issues.asterisk.org/jira/browse/ASTERISK-24935
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 res_pjsip_phoneprov_provider was leaking references to phoneprov objects due 
 to a missing OBJ_NODATA in an ao2_callback in load_users().  Rather than 
 adding the OBJ_NODATA, I changed load_users to use a more straightforward 
 ao2_iterator.  This plugged the leak but exposed an unload order issue 
 between res_pjsip_phoneprov_provider, res_phoneprov and res_pjsip.
 
 res_pjsip_phoneprov_provider unloads first, then res_phoneprov, then 
 res_pjsip.  Since res_pjsip_phoneprov_provider uses res_pjsip's sorcery 
 instance, when it unloads, it's objects are still in the sorcery instance.  
 When res_pjsip unloads, it destroys all its objects including 
 res_pjsip_phoneprov_provider's.  The phoneprov destructor then attempts to 
 unregister the extension from res_phoneprov but because res_phoneprov is 
 already cleaned up, its users container is gone and we get a FRACK.
 
 Simple solution, check for the NULL users container before attempting to 
 remove the entry. Duh.
 
 
 Diffs
 -
 
   branches/13/res/res_pjsip_phoneprov_provider.c 434447 
   branches/13/res/res_phoneprov.c 434447 
 
 Diff: https://reviewboard.asterisk.org/r/4608/diff/
 
 
 Testing
 ---
 
 Ran tests/res_phoneprov/res_phoneprov_provider.  No leaks in 
 res_pjsip_phoneprov_provider and no FRACKs.
 
 
 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] 4608: res_pjsip_phoneprov_provider: Fix reference leak on unload

[Corey Farrell]

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


I think I still prefer ao2_callback over ao2_iterator for taking an action 
against every object in a container, but don't feel strongly enough to hold 
this up.  Minor nit with the return value, then we can ship this.


branches/13/res/res_pjsip_phoneprov_provider.c
https://reviewboard.asterisk.org/r/4608/#comment25828

If we're sticking with the ao2_iterator lets loose the 'int' return, just 
make this function void.



branches/13/res/res_pjsip_phoneprov_provider.c
https://reviewboard.asterisk.org/r/4608/#comment25827

Remove..



branches/13/res/res_pjsip_phoneprov_provider.c
https://reviewboard.asterisk.org/r/4608/#comment25826

To record our brief discussion about this change:
coreyfarrell gtjoseph: so one thing I don't see how using an ao2_iterator 
instead of ao2_callback in users_apply_handler makes it more straight forward..
coreyfarrell I don't want to hold up the fix, but that change doesn't 
make much sense to me
gtjoseph there's no worry about the callback return value, OBJ_NODATA, 
etc.  Get the container and iterate over it.   It also eliminates a lot of 
overhead in the ao2_traversal code.
gtjoseph unless the callback is controlling the traversal with it's 
return code, there's no point.


- Corey Farrell


On April 9, 2015, 3:20 p.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4608/
 ---
 
 (Updated April 9, 2015, 3:20 p.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Bugs: ASTERISK-24935
 https://issues.asterisk.org/jira/browse/ASTERISK-24935
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 res_pjsip_phoneprov_provider was leaking references to phoneprov objects due 
 to a missing OBJ_NODATA in an ao2_callback in load_users().  Rather than 
 adding the OBJ_NODATA, I changed load_users to use a more straightforward 
 ao2_iterator.  This plugged the leak but exposed an unload order issue 
 between res_pjsip_phoneprov_provider, res_phoneprov and res_pjsip.
 
 res_pjsip_phoneprov_provider unloads first, then res_phoneprov, then 
 res_pjsip.  Since res_pjsip_phoneprov_provider uses res_pjsip's sorcery 
 instance, when it unloads, it's objects are still in the sorcery instance.  
 When res_pjsip unloads, it destroys all its objects including 
 res_pjsip_phoneprov_provider's.  The phoneprov destructor then attempts to 
 unregister the extension from res_phoneprov but because res_phoneprov is 
 already cleaned up, its users container is gone and we get a FRACK.
 
 Simple solution, check for the NULL users container before attempting to 
 remove the entry. Duh.
 
 
 Diffs
 -
 
   branches/13/res/res_pjsip_phoneprov_provider.c 434447 
   branches/13/res/res_phoneprov.c 434447 
 
 Diff: https://reviewboard.asterisk.org/r/4608/diff/
 
 
 Testing
 ---
 
 Ran tests/res_phoneprov/res_phoneprov_provider.  No leaks in 
 res_pjsip_phoneprov_provider and no FRACKs.
 
 
 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] 4604: loader/main: Don't set ast_fully_booted until deferred reloads are processed

[Corey Farrell]

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

Ship it!


Ship It!

- Corey Farrell


On April 9, 2015, 10:57 a.m., George Joseph wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4604/
 ---
 
 (Updated April 9, 2015, 10:57 a.m.)
 
 
 Review request for Asterisk Developers and Corey Farrell.
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 Until we have a true module management facility it's sometimes necessary for 
 one module to force a reload on another before its own load is complete.  If 
 Asterisk isn't fully booted yet, these reloads are deferred.  The problem is 
 that asterisk reports fully booted before processing the deferred reloads 
 which means Asterisk really isn't quite ready when it says it is.
 
 This patch moves the report of fully booted after the processing of the 
 deferred reloads is complete.
 
 
 Diffs
 -
 
   branches/13/main/loader.c 434447 
   branches/13/main/asterisk.c 434447 
 
 Diff: https://reviewboard.asterisk.org/r/4604/diff/
 
 
 Testing
 ---
 
 Since the pjsip stack has the most number of related modules, I ran the 
 channels/pjsip testsuite to make sure there aren't any issues.  All tests 
 passed.
 
 
 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