date:20140923


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

Ship it!


Ship It!

- Joshua Colp


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


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

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

Re: [asterisk-dev] [Code Review] 4016: chan_sip: Unref outbound proxy structure on dialog(pvt) struct


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



/branches/1.8/channels/chan_sip.c
https://reviewboard.asterisk.org/r/4016/#comment23871

This is actually incorrect. p-outboundproxy may point to a statically 
allocated outbound proxy. To ensure both cases are met use ref_proxy to set it 
to NULL. This will unref if it should and otherwise not unref.


- Joshua Colp


On Sept. 22, 2014, 8:30 p.m., wdoekes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4016/
 ---
 
 (Updated Sept. 22, 2014, 8:30 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24063
 https://issues.asterisk.org/jira/browse/ASTERISK-24063
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 In ASTERISK-24063 Damian Ivereigh wants to add outboundproxy support
 to OPTIONS qualify requests.
 
 The addition clearly shows that dialog-set proxies were not unreffed,
 creating a (very slow) memory leak if proxies are frequently changed.
 
 This should fix things.
 
 
 Diffs
 -
 
   /branches/1.8/channels/chan_sip.c 423724 
 
 Diff: https://reviewboard.asterisk.org/r/4016/diff/
 
 
 Testing
 ---
 
 Tested with the patch from:
 https://reviewboard.asterisk.org/r/3948/
 because that patch makes sure a proxy is reffed for every OPTIONS.
 But without that patch, the problem applies to most other dialog
 creation as well (calling, mwi, ...).
 
 sip.conf:
 
 [general]
 
 [whoever]
 type=peer
 host=127.0.0.1
 port=5062
 qualify=yes
 outboundproxy=127.0.0.1:5065
 
 
 Without this patch, 'memory atexit list' listed proxy_from_config(),
 with this patch, the atexit list was free of any chan_sip related stuff.
 
 
 Thanks,
 
 wdoekes
 


-- 
_
-- 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] 4016: chan_sip: Unref outbound proxy structure on dialog(pvt) struct

2014-09-23 Thread wdoekes


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

(Updated Sept. 23, 2014, 12:18 p.m.)


Review request for Asterisk Developers.


Changes
---

Thanks Josh. Updated as suggested.


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


Repository: Asterisk


Description
---

In ASTERISK-24063 Damian Ivereigh wants to add outboundproxy support
to OPTIONS qualify requests.

The addition clearly shows that dialog-set proxies were not unreffed,
creating a (very slow) memory leak if proxies are frequently changed.

This should fix things.


Diffs (updated)
-

  /branches/1.8/channels/chan_sip.c 423782 

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


Testing
---

Tested with the patch from:
https://reviewboard.asterisk.org/r/3948/
because that patch makes sure a proxy is reffed for every OPTIONS.
But without that patch, the problem applies to most other dialog
creation as well (calling, mwi, ...).

sip.conf:

[general]

[whoever]
type=peer
host=127.0.0.1
port=5062
qualify=yes
outboundproxy=127.0.0.1:5065


Without this patch, 'memory atexit list' listed proxy_from_config(),
with this patch, the atexit list was free of any chan_sip related stuff.


Thanks,

wdoekes

-- 
_
-- 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] 4016: chan_sip: Unref outbound proxy structure on dialog(pvt) struct


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

Ship it!


Ship It!

- Joshua Colp


On Sept. 23, 2014, 12:18 p.m., wdoekes wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://reviewboard.asterisk.org/r/4016/
 ---
 
 (Updated Sept. 23, 2014, 12:18 p.m.)
 
 
 Review request for Asterisk Developers.
 
 
 Bugs: ASTERISK-24063
 https://issues.asterisk.org/jira/browse/ASTERISK-24063
 
 
 Repository: Asterisk
 
 
 Description
 ---
 
 In ASTERISK-24063 Damian Ivereigh wants to add outboundproxy support
 to OPTIONS qualify requests.
 
 The addition clearly shows that dialog-set proxies were not unreffed,
 creating a (very slow) memory leak if proxies are frequently changed.
 
 This should fix things.
 
 
 Diffs
 -
 
   /branches/1.8/channels/chan_sip.c 423782 
 
 Diff: https://reviewboard.asterisk.org/r/4016/diff/
 
 
 Testing
 ---
 
 Tested with the patch from:
 https://reviewboard.asterisk.org/r/3948/
 because that patch makes sure a proxy is reffed for every OPTIONS.
 But without that patch, the problem applies to most other dialog
 creation as well (calling, mwi, ...).
 
 sip.conf:
 
 [general]
 
 [whoever]
 type=peer
 host=127.0.0.1
 port=5062
 qualify=yes
 outboundproxy=127.0.0.1:5065
 
 
 Without this patch, 'memory atexit list' listed proxy_from_config(),
 with this patch, the atexit list was free of any chan_sip related stuff.
 
 
 Thanks,
 
 wdoekes
 


-- 
_
-- 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] beta2 compile failure


On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com wrote:

 Paul Albrecht wrote:
 
 Asterisk 13 beta2 compile fails:
 
 .
 .
 .
 [CC] chan_pjsip.c - chan_pjsip.o
 [CC] pjsip/dialplan_functions.c - pjsip/dialplan_functions.o
 [LD] chan_pjsip.o pjsip/dialplan_functions.o - chan_pjsip.so
 /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../x86_64-pc-linux-gnu/bin/ld:
 /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a(sip_inv.o):
 relocation R_X86_64_32S against `.rodata' can not be used when making a
 shared object; recompile with -fPIC
 /usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a:
 could not read symbols: Bad value
 collect2: ld returned 1 exit status
 make[1]: *** [chan_pjsip.so] Error 1
 make: *** [channels] Error 2
 
 Your environment does not seem have a suitable pjproject. It should not 
 be trying to link in any static. Did you follow the wiki to build 
 pjproject? Do you have an old install as well as a new?

Oops, my bad, I didn’t setup pjproject. However, the configure script didn’t 
complain so I assumed I was good to go and proceeded to make asterisk.  
Shouldn’t the configure script catch this and not allow someone to proceed to 
make? Thought that was a function of the configure script, that is, it verifies 
software dependancies before the build step.

Another thing … the README int the top level directory doesn’t seem to have 
been updated in a while and doesn’t mention pjproject. If there’s an external 
requirement like pjproject shouldn’t that go in the README? 

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

-- 
_
-- 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] beta2 compile failure

Paul Albrecht wrote:

On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com
mailto:jc...@digium.com wrote:

Paul Albrecht wrote:

Asterisk 13 beta2 compile fails:

.
.
.
[CC] chan_pjsip.c - chan_pjsip.o
[CC] pjsip/dialplan_functions.c - pjsip/dialplan_functions.o
[LD] chan_pjsip.o pjsip/dialplan_functions.o - chan_pjsip.so
/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../x86_64-pc-linux-gnu/bin/ld:
/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a(sip_inv.o):
relocation R_X86_64_32S against `.rodata' can not be used when making a
shared object; recompile with -fPIC
/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a:
could not read symbols: Bad value
collect2: ld returned 1 exit status
make[1]: *** [chan_pjsip.so] Error 1
make: *** [channels] Error 2

Your environment does not seem have a suitable pjproject. It should not
be trying to link in any static. Did you follow the wiki to build
pjproject? Do you have an old install as well as a new?

Oops, my bad, I didn’t setup pjproject. However, the configure script
didn’t complain so I assumed I was good to go and proceeded to make
asterisk. Shouldn’t the configure script catch this and not allow
someone to proceed to make? Thought that was a function of the configure
script, that is, it verifies software dependancies before the build step.

The configure script checks for pjproject but it is not currently
specific enough to look for only shared. (Not sure off the top of my
head how we could make it)

Another thing … the README int the top level directory doesn’t seem to
have been updated in a while and doesn’t mention pjproject. If there’s
an external requirement like pjproject shouldn’t that go in the README?

These days almost everything is documented on the wiki. The README
itself only covers the basics to get Asterisk up and going. It could be
extended to include dependencies for the various things.

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

Re: [asterisk-dev] [Code Review] 4014: Changes to CDR and CEL unit tests to prevent FRACKs.

2014-09-23 Thread Mark Michelson


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

(Updated Sept. 23, 2014, 9:29 a.m.)


Status
--

This change has been marked as submitted.


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Committed in revision 423783


Repository: Asterisk


Description
---

In Asterisk 13, the format rewrite caused CDR and CEL unit tests to have FRACKs 
because they have no formats on the channels they create. In revision 423414 of 
Asterisk 13, I introduced an independent change that is intended to prevent 
unnecessary FRACKs by returning early when attempting to set up translation 
paths if any channel has no formats set. Unfortunately, this resulted in huge 
breakage of the CDR and CEL unit tests.

The change introduced in this review is to set the ulaw format on channels 
created by the CEL and CDR unit tests. This way, the tests are passing again. 
Event better, they now pass FRACK-free.


Diffs
-

  /branches/13/tests/test_cel.c 423656 
  /branches/13/tests/test_cdr.c 423656 

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


Testing
---

Ran CDR and CEL unit tests with and without the change. Without this patch, 
things are pretty disastrous in the CEL tests (i.e. Asterisk crashes). And the 
CDR tests have FRACKs in them. With the patch, all CDR and CEL unit tests pass 
with no FRACKs.


Thanks,

Mark Michelson

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

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

Re: [asterisk-dev] beta2 compile failure

On Sep 23, 2014, at 9:25 AM, Joshua Colp jc...@digium.com wrote:

Paul Albrecht wrote:

On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com
mailto:jc...@digium.com wrote:

Paul Albrecht wrote:

Asterisk 13 beta2 compile fails:

Your environment does not seem have a suitable pjproject. It should not
be trying to link in any static. Did you follow the wiki to build
pjproject? Do you have an old install as well as a new?

The configure script checks for pjproject but it is not currently
specific enough to look for only shared. (Not sure off the top of my
head how we could make it)

So your response is that you agree the configure script should check for the
pjproject dependency, but you don’t know how to do that? Really?

These days almost everything is documented on the wiki. The README
itself only covers the basics to get Asterisk up and going. It could be
extended to include dependencies for the various things.

The README no longer covers the basics to get asterisk up and running because
it doesn’t mention pjproject, which is now a required external dependency. If
you don’t think the README file is the right place for installation
instructions, why not add an “INSTALL” file in the top level directory like
other projects?

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

--
_
-- 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] beta2 compile failure

On Tue, Sep 23, 2014 at 10:11 AM, Paul Albrecht palbre...@glccom.com
wrote:

On Sep 23, 2014, at 9:25 AM, Joshua Colp jc...@digium.com wrote:

Paul Albrecht wrote:

On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com
mailto:jc...@digium.com wrote:

Paul Albrecht wrote:

Asterisk 13 beta2 compile fails:

.
.
.
[CC] chan_pjsip.c - chan_pjsip.o
[CC] pjsip/dialplan_functions.c - pjsip/dialplan_functions.o
[LD] chan_pjsip.o pjsip/dialplan_functions.o - chan_pjsip.so

/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../x86_64-pc-linux-gnu/bin/ld:

/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a(sip_inv.o):
relocation R_X86_64_32S against `.rodata' can not be used when making
a
shared object; recompile with -fPIC

/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../lib64/libpjsip-ua-x86_64-pc-linux-gnu.a:
could not read symbols: Bad value
collect2: ld returned 1 exit status
make[1]: *** [chan_pjsip.so] Error 1
make: *** [channels] Error 2

Your environment does not seem have a suitable pjproject. It should not
be trying to link in any static. Did you follow the wiki to build
pjproject? Do you have an old install as well as a new?

The configure script checks for pjproject but it is not currently
specific enough to look for only shared. (Not sure off the top of my
head how we could make it)

So your response is that you agree the configure script should check for
the pjproject dependency, but you don’t know how to do that? Really?

Josh was trying to help. If you'd like to provide a patch to the configure
script to make it more robust in detecting the shared object libraries,
that would be appreciated. However, as Asterisk is installed on a wide
variety of platforms, such solutions are often very difficult to craft
correctly.

The configure script currently uses the AST_PKG_CONFIG_CHECK macro to
verify pjproject is installed. That will use the output of pkg-config to
verify whether or not the shared object libraries are installed. If, for
whatever reason, that returns success, then I would expect chan_pjsip to
enabled.

The config.log would illustrate why it returned success in your case.

These days almost everything is documented on the wiki. The README
itself only covers the basics to get Asterisk up and going. It could be
extended to include dependencies for the various things.

The README no longer covers the basics to get asterisk up and running
because it doesn’t mention pjproject, which is now a required external
dependency. If you don’t think the README file is the right place for
installation instructions, why not add an “INSTALL” file in the top level
directory like other projects?

That isn't what Josh said. pjproject is not a required external dependency.
You can run Asterisk just fine without it.

As for the suggestion of adding an INSTALL file: this is really just the
same as a README. Personally, we've found that the wiki provides a good
place to collect that information. Major items are still listed in the
README; as pjproject is not required for Asterisk, I'm not sure it has to
be there.

Matt

--
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com http://asterisk.org
--
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

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

Re: [asterisk-dev] [Code Review] 4013: Alembic: Add 'outgoing' enum value to sippeers directmedia enumerator

2014-09-23 Thread Jonathan Rose


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

(Updated Sept. 23, 2014, 10:41 a.m.)


Review request for Asterisk Developers and Matt Jordan.


Changes
---

Hey hey, so instead of dropping the tables I was able to do a safe altering of 
the tables by doing some postgres specific shenanigans. I've also made sure it 
works against mysql.
Basically, going forward everything is fine.  Going backwards, we change any 
instances of 'outgoing' in the table to NULL prior to changing the type.


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


Repository: Asterisk


Description
---

The SIP directmedia value 'outgoing' was overlooked when creating the table and 
needs to be added. Unfortunately this means dropping the column and rebuilding 
it with a new enum.


Diffs (updated)
-

  
/branches/12/contrib/ast-db-manage/config/versions/10aedae86a32_add_outgoing_enum_va.py
 PRE-CREATION 

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


Testing
---

upgraded, checked to see if I could put 'outgoing' in the directmedia column. 
Downgraded, upgraded again to make sure things didn't explode.

All on a postgres database


Thanks,

Jonathan Rose

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

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

[asterisk-dev] Opinions Needed: Case sensitivity in config file section names

I've been working on some changes for config.c and in the process I've
found 5 instances of someone attempting to do cat-name == category_name
instead of strcmp(cat-name, category_name).Example:

/* try exact match first, then case-insensitive match */
for (cat = config-root; cat; cat = cat-next) {
if (cat-name == category_name  (ignored || !cat-ignored))
return cat;
}

for (cat = config-root; cat; cat = cat-next) {
if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
return cat;
}

The result is that the case sensitive match never succeeds and it's always
the case insensitive match that's run.

My question is...  Should I fix these so the case sensitive match works and
runs first or just remove the first loop so the match is always
case-insensitive?   I'm hoping the latter not only because it makes the
code simpler but because that's how it's worked for years and suddenly
making the match case sensitive might cause unexpected problems.

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] Opinions Needed: Case sensitivity in config file section names

On Tue, Sep 23, 2014 at 9:45 AM, George Joseph george.jos...@fairview5.com
wrote:

 I've been working on some changes for config.c and in the process I've
 found 5 instances of someone attempting to do cat-name == category_name
 instead of strcmp(cat-name, category_name).Example:

 /* try exact match first, then case-insensitive match */
 for (cat = config-root; cat; cat = cat-next) {
 if (cat-name == category_name  (ignored || !cat-ignored))
 return cat;
 }

 for (cat = config-root; cat; cat = cat-next) {
 if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
 return cat;
 }

 The result is that the case sensitive match never succeeds and it's always
 the case insensitive match that's run.

 My question is...  Should I fix these so the case sensitive match works
 and runs first or just remove the first loop so the match is always
 case-insensitive?   I'm hoping the latter not only because it makes the
 code simpler but because that's how it's worked for years and suddenly
 making the match case sensitive might cause unexpected problems.

 Thoughts?

 Forgot to mention...There are other places in the code where the
comparison is always case-insensitive.
-- 
_
-- 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] Opinions Needed: Case sensitivity in config file section names

On Tue, Sep 23, 2014 at 10:45 AM, George Joseph george.jos...@fairview5.com
 wrote:

 I've been working on some changes for config.c and in the process I've
 found 5 instances of someone attempting to do cat-name == category_name
 instead of strcmp(cat-name, category_name).Example:

 /* try exact match first, then case-insensitive match */
 for (cat = config-root; cat; cat = cat-next) {
 if (cat-name == category_name  (ignored || !cat-ignored))
 return cat;
 }

 for (cat = config-root; cat; cat = cat-next) {
 if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
 return cat;
 }

 The result is that the case sensitive match never succeeds and it's always
 the case insensitive match that's run.

 My question is...  Should I fix these so the case sensitive match works
 and runs first or just remove the first loop so the match is always
 case-insensitive?   I'm hoping the latter not only because it makes the
 code simpler but because that's how it's worked for years and suddenly
 making the match case sensitive might cause unexpected problems.



My vote is for case insensitive.

In fact, practically speaking, I'm not sure how this isn't always case
insensitive.

Given contexts [foo] and [Foo], searching for foo:

* If foo matches context foo directly, return foo.
* If foo matches context Foo, return Foo.

Either way, you'll end up matching on Foo eventually.

If there's a risk here that I'm missing, then I'd vote trunk only for
changes.

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com  http://asterisk.org
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

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

Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names

2014-09-23 Thread Richard Mudgett

On Tue, Sep 23, 2014 at 10:51 AM, George Joseph george.jos...@fairview5.com
 wrote:

 On Tue, Sep 23, 2014 at 9:45 AM, George Joseph 
 george.jos...@fairview5.com wrote:

 I've been working on some changes for config.c and in the process I've
 found 5 instances of someone attempting to do cat-name == category_name
 instead of strcmp(cat-name, category_name).Example:

 /* try exact match first, then case-insensitive match */
 for (cat = config-root; cat; cat = cat-next) {
 if (cat-name == category_name  (ignored || !cat-ignored))
 return cat;
 }

 for (cat = config-root; cat; cat = cat-next) {
 if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
 return cat;
 }

 The result is that the case sensitive match never succeeds and it's
 always the case insensitive match that's run.

 My question is...  Should I fix these so the case sensitive match works
 and runs first or just remove the first loop so the match is always
 case-insensitive?   I'm hoping the latter not only because it makes the
 code simpler but because that's how it's worked for years and suddenly
 making the match case sensitive might cause unexpected problems.

 Thoughts?

 Forgot to mention...There are other places in the code where the
 comparison is always case-insensitive.


I was under the impression that the code was done this way
initially to avoid doing a string comparison.  Though, I'm not
sure avoiding the string comparison really buys much in the
way of performance anyway.

Another reason is if you have several sections named [foo]
and you need to resume with the same [foo] section as last
time.

Richard
-- 
_
-- 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] beta2 compile failure

On Sep 23, 2014, at 10:24 AM, Matthew Jordan mjor...@digium.com wrote:

On Tue, Sep 23, 2014 at 10:11 AM, Paul Albrecht palbre...@glccom.com wrote:

On Sep 23, 2014, at 9:25 AM, Joshua Colp jc...@digium.com wrote:

Paul Albrecht wrote:

On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com
mailto:jc...@digium.com wrote:

Paul Albrecht wrote:

Asterisk 13 beta2 compile fails:

Your environment does not seem have a suitable pjproject. It should not
be trying to link in any static. Did you follow the wiki to build
pjproject? Do you have an old install as well as a new?

The configure script checks for pjproject but it is not currently
specific enough to look for only shared. (Not sure off the top of my
head how we could make it)

So your response is that you agree the configure script should check for the
pjproject dependency, but you don’t know how to do that? Really?

Josh was trying to help. If you'd like to provide a patch to the configure
script to make it more robust in detecting the shared object libraries, that
would be appreciated. However, as Asterisk is installed on a wide variety of
platforms, such solutions are often very difficult to craft correctly.

The configure script currently uses the AST_PKG_CONFIG_CHECK macro to verify
pjproject is installed. That will use the output of pkg-config to verify
whether or not the shared object libraries are installed. If, for whatever
reason, that returns success, then I would expect chan_pjsip to enabled.

The config.log would illustrate why it returned success in your case.

I’m just a user reporting a bug and I don’t care why the asterisk configure
script doesn’t work. You’re the engineering manager so you can prioritize
problems or define them out of existence as you see fit.

These days almost everything is documented on the wiki. The README
itself only covers the basics to get Asterisk up and going. It could be
extended to include dependencies for the various things.

The README no longer covers the basics to get asterisk up and running
because it doesn’t mention pjproject, which is now a required external
dependency. If you don’t think the README file is the right place for
installation instructions, why not add an “INSTALL” file in the top level
directory like other projects?

That isn't what Josh said. pjproject is not a required external dependency.
You can run Asterisk just fine without it.

No, you can’t run asterisk without pjproject. I know because I’ve tried. If you
think you can, please provide me with a detailed set instructions for doing so
because I’d like to try that.

As for the suggestion of adding an INSTALL file: this is really just the same
as a README. Personally, we've found that the wiki provides a good place to
collect that information. Major items are still listed in the README; as
pjproject is not required for Asterisk, I'm not sure it has to be there.

I don’t disagree that the wiki is a good source of information and should be
used. That’s really a non-issue. What I do think is a problem is the fact the
README hasn’t been kept up to date with the later asterisk releases that
require pjproject. I don’t get the resistance to a simple suggestion to make
it easier for asterisk users to compile and install asterisk.

Matt

asterisk-dev

Re: [asterisk-dev] [Code Review] 4017: chan_pjsip: Don't attempt to apply formats if there aren't any capabilities defined when creating a new channel

2014-09-23 Thread Jonathan Rose


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

(Updated Sept. 23, 2014, 11:32 a.m.)


Review request for Asterisk Developers, Joshua Colp and Mark Michelson.


Changes
---

Poke mmichelson's finding


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


Repository: Asterisk


Description
---

This is the test scenario:

A has disallow=all and no allow lines
B has disallow=all, allow=ulaw

A dials an extension that 

Prior to Asterisk 13, calls like this would actually start dialing and the call 
would only hangup after answering. In 13, some major formats rework went in and 
this has led to some differences from that behavior. Namely the asserts 
mentioned in the issue, but another issue that this patch doesn't address is 
that in Asterisk 13, no outbound dial occurs in this case either... that might 
actually be a good thing.


Diffs (updated)
-

  /branches/13/channels/chan_pjsip.c 423661 

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


Testing
---

Dialed from A to B in the above scenario in 12, 13 (unpatched) and 13 with the 
patch. Made sure the channels involved were hung up properly and that no 
asserts occurred when the patch was in place.


Example output in Asterisk 12:
*CLI [Sep 22 15:59:50] WARNING[9969]: channel.c:834 ast_best_codec: Don't know 
any of (nothing) formats
-- Executing [1603@default:1] Dial(PJSIP/1601-0004, PJSIP/1603) in 
new stack
-- Called PJSIP/1603
-- PJSIP/1603-0005 is ringing
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5128 ast_write: Codec 
mismatch on channel PJSIP/1601-0004 setting write format to ulaw from 
unknown native formats (nothing)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5365 set_format: Unable 
to find a codec translation path from (nothing) to (ulaw)
[Sep 22 15:59:52] WARNING[10087][C-0003]: chan_pjsip.c:639 
chan_pjsip_write: Asked to transmit frame type ulaw, while native formats is 
(nothing) (read/write = unknown/unknown)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5128 ast_write: Codec 
mismatch on channel PJSIP/1601-0004 setting write format to ulaw from 
unknown native formats (nothing)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5365 set_format: Unable 
to find a codec translation path from (nothing) to (ulaw)
[Sep 22 15:59:52] WARNING[10087][C-0003]: chan_pjsip.c:639 
chan_pjsip_write: Asked to transmit frame type ulaw, while native formats is 
(nothing) (read/write = unknown/unknown)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5128 ast_write: Codec 
mismatch on channel PJSIP/1601-0004 setting write format to ulaw from 
unknown native formats (nothing)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5365 set_format: Unable 
to find a codec translation path from (nothing) to (ulaw)
[Sep 22 15:59:52] WARNING[10087][C-0003]: chan_pjsip.c:639 
chan_pjsip_write: Asked to transmit frame type ulaw, while native formats is 
(nothing) (read/write = unknown/unknown)
-- PJSIP/1603-0005 answered PJSIP/1601-0004
-- Channel PJSIP/1603-0005 joined 'simple_bridge' basic-bridge 
cca32573-8057-4791-8261-b0270821b33f
-- Channel PJSIP/1601-0004 joined 'simple_bridge' basic-bridge 
cca32573-8057-4791-8261-b0270821b33f
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5365 set_format: Unable 
to find a codec translation path from (nothing) to (ulaw)
[Sep 22 15:59:52] WARNING[10087][C-0003]: bridge.c:957 
bridge_make_compatible: Failed to set channel PJSIP/1601-0004 to read 
format ulaw
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5128 ast_write: Codec 
mismatch on channel PJSIP/1601-0004 setting write format to ulaw from 
unknown native formats (nothing)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5365 set_format: Unable 
to find a codec translation path from (nothing) to (ulaw)
[Sep 22 15:59:52] WARNING[10087][C-0003]: chan_pjsip.c:639 
chan_pjsip_write: Asked to transmit frame type ulaw, while native formats is 
(nothing) (read/write = unknown/unknown)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5128 ast_write: Codec 
mismatch on channel PJSIP/1601-0004 setting write format to ulaw from 
unknown native formats (nothing)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5365 set_format: Unable 
to find a codec translation path from (nothing) to (ulaw)
[Sep 22 15:59:52] WARNING[10087][C-0003]: chan_pjsip.c:639 
chan_pjsip_write: Asked to transmit frame type ulaw, while native formats is 
(nothing) (read/write = unknown/unknown)
[Sep 22 15:59:52] WARNING[10087][C-0003]: channel.c:5128 ast_write: Codec 
mismatch on channel PJSIP/1601-0004 setting write format to ulaw from 
unknown native formats (nothing)
[Sep 22 15:59:52]

Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names

2014-09-23 Thread Paul Belanger

On Tue, Sep 23, 2014 at 11:45 AM, George Joseph
george.jos...@fairview5.com wrote:
 I've been working on some changes for config.c and in the process I've found
 5 instances of someone attempting to do cat-name == category_name instead
 of strcmp(cat-name, category_name).Example:

 /* try exact match first, then case-insensitive match */
 for (cat = config-root; cat; cat = cat-next) {
 if (cat-name == category_name  (ignored || !cat-ignored))
 return cat;
 }

 for (cat = config-root; cat; cat = cat-next) {
 if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
 return cat;
 }

 The result is that the case sensitive match never succeeds and it's always
 the case insensitive match that's run.

 My question is...  Should I fix these so the case sensitive match works and
 runs first or just remove the first loop so the match is always
 case-insensitive?   I'm hoping the latter not only because it makes the code
 simpler but because that's how it's worked for years and suddenly making the
 match case sensitive might cause unexpected problems.

 Thoughts?

For me, case sensitive.  Because I config files that do have:

[Foo]

[foo]

[fOO]

don't ask, long story.

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

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

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

Re: [asterisk-dev] beta2 compile failure

2014-09-23 Thread Jason Parker

-BEGIN PGP SIGNED MESSAGE-
Hash: SHA1

On 09/23/2014 11:19 AM, Paul Albrecht wrote:
 
 I’m just a user reporting a bug
 

No you aren't.  If you were reporting a bug, you would be at
https://issues.asterisk.org/, instead.
-BEGIN PGP SIGNATURE-
Version: GnuPG v1

iEYEARECAAYFAlQhp9YACgkQ5UEwSbgOpICNUQCfdSEH6vgyACQdHaitc0pplMCb
kCUAniJYsiVS1wjvtMe/OyNcjXuARNLI
=IAaL
-END PGP SIGNATURE-

-- 
_
-- 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] Opinions Needed: Case sensitivity in config file section names

On Tue, Sep 23, 2014 at 11:29 AM, Paul Belanger 
paul.belan...@polybeacon.com wrote:

 On Tue, Sep 23, 2014 at 11:45 AM, George Joseph
 george.jos...@fairview5.com wrote:
  I've been working on some changes for config.c and in the process I've
 found
  5 instances of someone attempting to do cat-name == category_name
 instead
  of strcmp(cat-name, category_name).Example:
 
  /* try exact match first, then case-insensitive match */
  for (cat = config-root; cat; cat = cat-next) {
  if (cat-name == category_name  (ignored || !cat-ignored))
  return cat;
  }
 
  for (cat = config-root; cat; cat = cat-next) {
  if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
  return cat;
  }
 
  The result is that the case sensitive match never succeeds and it's
 always
  the case insensitive match that's run.
 
  My question is...  Should I fix these so the case sensitive match works
 and
  runs first or just remove the first loop so the match is always
  case-insensitive?   I'm hoping the latter not only because it makes the
 code
  simpler but because that's how it's worked for years and suddenly making
 the
  match case sensitive might cause unexpected problems.
 
  Thoughts?
 
 For me, case sensitive.  Because I config files that do have:

 [Foo]

 [foo]

 [fOO]

 don't ask, long story.


And it currently works? :-D

(Just kidding)

If that is the case, then obviously any change we make here should be done
to trunk only.

I'll leave the impending case-war to others... :-)

-- 
Matthew Jordan
Digium, Inc. | Engineering Manager
445 Jan Davis Drive NW - Huntsville, AL 35806 - USA
Check us out at: http://digium.com  http://asterisk.org
-- 
_
-- Bandwidth and Colocation Provided by http://www.api-digital.com --

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

Re: [asterisk-dev] beta2 compile failure

On Tue, Sep 23, 2014 at 11:19 AM, Paul Albrecht palbre...@glccom.com
wrote:

On Sep 23, 2014, at 10:24 AM, Matthew Jordan mjor...@digium.com wrote:

On Tue, Sep 23, 2014 at 10:11 AM, Paul Albrecht palbre...@glccom.com
wrote:

On Sep 23, 2014, at 9:25 AM, Joshua Colp jc...@digium.com wrote:

Paul Albrecht wrote:

On Sep 22, 2014, at 3:47 PM, Joshua Colp jc...@digium.com
mailto:jc...@digium.com wrote:

Paul Albrecht wrote:

Asterisk 13 beta2 compile fails:

.
.
.
[CC] chan_pjsip.c - chan_pjsip.o
[CC] pjsip/dialplan_functions.c - pjsip/dialplan_functions.o
[LD] chan_pjsip.o pjsip/dialplan_functions.o - chan_pjsip.so

/usr/lib/gcc/x86_64-pc-linux-gnu/4.5.3/../../../../x86_64-pc-linux-gnu/bin/ld:

Your environment does not seem have a suitable pjproject. It should
not
be trying to link in any static. Did you follow the wiki to build
pjproject? Do you have an old install as well as a new?

Oops, my bad, I didn’t setup pjproject. However, the configure script
didn’t complain so I assumed I was good to go and proceeded to make
asterisk. Shouldn’t the configure script catch this and not allow
someone to proceed to make? Thought that was a function of the
configure
script, that is, it verifies software dependancies before the build
step.

The configure script checks for pjproject but it is not currently
specific enough to look for only shared. (Not sure off the top of my
head how we could make it)

So your response is that you agree the configure script should check for
the pjproject dependency, but you don’t know how to do that? Really?

The config.log would illustrate why it returned success in your case.

I’m just a user reporting a bug and I don’t care why the asterisk
configure script doesn’t work. You’re the engineering manager so you can
prioritize problems or define them out of existence as you see fit.

That isn't how the Asterisk project works.

First, if this really was just a 'bug report', then as Jason pointed out,
the appropriate place to go is issues.asterisk.org. If that's the case,
then we can continue the troubleshooting there. I'm inclined to think that
Josh's initial assessment is correct: you have a version of pjproject
installed on your system that has static libraries but not the dynamic
libraries. It would be interesting to see what the config.log spat out. If
you feel that's worthy of a bug report, then please go make an issue on the
issue tracker and attach the config.log there, along with sufficient steps
to reproduce the configuration issue on a different machine.

Beyond that, I don't make any decision about what the developers in the
Asterisk project choose to fix or provide patches for. That is up to each
individual developer. If someone wants to provide a patch for the build
system that makes it better in some regard, that's great. Generally, if
someone provides a patch with an issue, those issues tend to get fixed
faster.

As the project lead, the only time I would weigh in is when there is
disagreement over a proposed patch. Those situations are very rare.

I do have some influence on what Digium developers do with respect to the
Asterisk project, but that has no bearing on what others choose to do.

These days almost everything is documented on the wiki. The README
itself only covers the basics to get Asterisk up and going. It could be
extended to include dependencies for the various things.

The README no longer covers the basics to get asterisk up and running
because it doesn’t mention pjproject, which is now a required external
dependency. If you don’t think the README file is the right place for
installation instructions,

Re: [asterisk-dev] Opinions Needed: Case sensitivity in config file section names

On Tue, Sep 23, 2014 at 10:05 AM, Richard Mudgett rmudg...@digium.com
wrote:



 On Tue, Sep 23, 2014 at 10:51 AM, George Joseph 
 george.jos...@fairview5.com wrote:

 On Tue, Sep 23, 2014 at 9:45 AM, George Joseph 
 george.jos...@fairview5.com wrote:

 I've been working on some changes for config.c and in the process I've
 found 5 instances of someone attempting to do cat-name == category_name
 instead of strcmp(cat-name, category_name).Example:

 /* try exact match first, then case-insensitive match */
 for (cat = config-root; cat; cat = cat-next) {
 if (cat-name == category_name  (ignored || !cat-ignored))
 return cat;
 }

 for (cat = config-root; cat; cat = cat-next) {
 if (!strcasecmp(cat-name, category_name)  (ignored || !cat-ignored))
 return cat;
 }

 The result is that the case sensitive match never succeeds and it's
 always the case insensitive match that's run.

 My question is...  Should I fix these so the case sensitive match works
 and runs first or just remove the first loop so the match is always
 case-insensitive?   I'm hoping the latter not only because it makes the
 code simpler but because that's how it's worked for years and suddenly
 making the match case sensitive might cause unexpected problems.

 Thoughts?

 Forgot to mention...There are other places in the code where the
 comparison is always case-insensitive.


 I was under the impression that the code was done this way
 initially to avoid doing a string comparison.  Though, I'm not
 sure avoiding the string comparison really buys much in the
 way of performance anyway.

 Another reason is if you have several sections named [foo]
 and you need to resume with the same [foo] section as last
 time.

 Richard

 This works only in ast_category_browse where the pointer you're passing in
is almost always a pointer you received from the last call to
ast_category_browse.It's almost never going to work in ast_category_get
because you're probably passing in a constant or a pointer to some other
memory location housing the name.
-- 
_
-- 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] Opinions Needed: Case sensitivity in config file section names

On Tue, Sep 23, 2014 at 11:13 AM, Matthew Jordan mjor...@digium.com wrote:



 On Tue, Sep 23, 2014 at 11:29 AM, Paul Belanger 
 paul.belan...@polybeacon.com wrote:

 On Tue, Sep 23, 2014 at 11:45 AM, George Joseph
 george.jos...@fairview5.com wrote:
  I've been working on some changes for config.c and in the process I've
 found
  5 instances of someone attempting to do cat-name == category_name
 instead
  of strcmp(cat-name, category_name).Example:
 

 snip

  My question is...  Should I fix these so the case sensitive match works
 and
  runs first or just remove the first loop so the match is always
  case-insensitive?   I'm hoping the latter not only because it makes the
 code
  simpler but because that's how it's worked for years and suddenly
 making the
  match case sensitive might cause unexpected problems.
 
  Thoughts?
 
 For me, case sensitive.  Because I config files that do have:

 [Foo]

 [foo]

 [fOO]

 don't ask, long story.



Which file?  I'd like to test a before and after scenario to make sure
whatever I change still works.


 And it currently works? :-D

 (Just kidding)

 If that is the case, then obviously any change we make here should be done
 to trunk only.

 I'll leave the impending case-war to others... :-)


Ok, I think I'll fix the ones that are definitely broken so they do case
sensitive first then fall back to case insensitive.   That should preserve
the current behavior.
-- 
_
-- 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] beta2 compile failure