Re: [asterisk-dev] Asterisk Media Performance
On Jan 6, 2014, at 12:20 PM, Joshua Colp wrote: > David Lee (digium) wrote: >> >> Synchronizing the notifications with the format change may be tricky. If >> we’re not careful, we could end up in the situation where 1) the format >> changes but some bit of code gets the notification about the change too >> late or 2) we end up with extra locks/contention in the media path due >> to format change notifications. > > I think we can actually do it using a control frame. The stuff that cares > about the format would be in that path, and the control frame could just be > in front of the frame with the new format. As it passes through stuff can > change accordingly (change formats, smoother, etc). Works for me :-) -- David M. Lee Digium, Inc. | Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: www.digium.com & www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3106: res_pjsip_session: If a reinvite without an SDP is received, unhold the call.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3106/ --- Review request for Asterisk Developers, Joshua Colp and Mark Michelson. Repository: Asterisk Description --- Some devices apparently perform unhold by simply issuing a reinvite on the dialog without an SDP. This patch adds that functionality to our PJSIP session control. It might be worthwhile to add some method of not queing unhold if the call isn't already on hold in the first place, but my testing so far hasn't revealed it as being necessary. Diffs - /trunk/res/res_pjsip_session.c 404854 Diff: https://reviewboard.asterisk.org/r/3106/diff/ Testing --- Created a phone_B.xml similar to the ones used for this case in review 3105 (phone_B_unhold_sans_sdp.xml). This would hold the call then unhold the call by sending a reinvite without an SDP pjsip endpoint: [sippbert] type=aor contact=sip:sippbert@127.0.0.3:5065 [sippbert] type=endpoint aors=sippbert context=default disallow=all allow=ulaw direct_media=no With the patch in place, this held and unheld the call as it should have. Without the patch, the hold would work (naturally since it's just a normal hold), but the invite without the SDP would not unhold the call. It's worth noting that if an unhold in this fashion is issued while the call is already active/not on hold will still produce unhold events. I'm unsure if this is a problem that needs to be addressed. Testing hasn't revealed this being a problem yet, but I don't believe many test scenarios exist which use reinvites without SDP. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3105: Testsuite: Test PJSIP hold and unhold for various conditions for INVITE SDPs
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3105/ --- Review request for Asterisk Developers, Joshua Colp, Matt Jordan, and Mark Michelson. Repository: testsuite Description --- Creates tests similar to the channels/SIP/sip_hold tests. Using the PJSIP channel driver, the following scenarios are tested: 1. Put a call on hold by setting media attributes to sendonly. Unhold by setting media back to sendrecv 2. (new) as above, but unhold by simply not including an SDP (some devices are known to do this apparently and a patch is on reviewboard to handle that scenario in PJSIP). 3. Set on hold by setting the contact IP to 0.0.0.0, return to normal by providing normal SDP 4. Combine both the IP hold and media restriction hold methods. There are a few noteworthy differences here to the original tests in SIP hold that might require some additional attention. First, simply running the SIPP scenarios as is with PJSIP yielded an inability to match the new INVITES to the existing PJSIP dialogs. In addition to one invite from each scenario appearing to point to the wrong peer in the invite used to resume/unhold the call, the appropriate To tag was left out of the reinvites for both holding and unholding. I'm not sure how this worked in the first place. Second, I noticed that verifying for hold/unhold behavior in the test script is performed by checking for Music On Hold start and stop events. This mostly works for hold, but for unhold in particular it's unreliable since music on hold stop events will also be issued if the call simply ends. Because of that, I went ahead and added listeners for Hold and Unhold events which are required in a similar fashion to the existing MOH start and stop events. I'm willing to apply these changes to the SIP hold test, but this does appear to be a noteworthy variance in behavior which needs to be taken note of... particularly the differences in the SIPP scenarios that needed to be made. Diffs - /asterisk/trunk/tests/channels/pjsip/tests.yaml 4539 /asterisk/trunk/tests/channels/pjsip/hold/test-config.yaml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_unhold_sans_sdp.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_media_restrict.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_restrict.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_B_IP_media_restrict.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/sipp/phone_A.xml PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/sipp/inject.csv PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/run-test PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/pjsip.conf PRE-CREATION /asterisk/trunk/tests/channels/pjsip/hold/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3105/diff/ Testing --- Ran the tests. Checked output of user events in Asterisk. Confirmed that without the patch in 3106 that this test fails due to not receiving an Unhold event for the second scenario described above. Thanks, Jonathan Rose -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3051: TestSuite: Add chan_pjsip path support tests
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3051/ --- (Updated Jan. 6, 2014, 3:53 p.m.) Review request for Asterisk Developers. Changes --- Tweak the SIPp scenario to fail more consistently under off-nominal conditions. Bugs: ASTERISK-21084 https://issues.asterisk.org/jira/browse/ASTERISK-21084 Repository: testsuite Description --- This adds a test which covers path support for outbound registrations, inbound registrations, and outbound requests following an inbound registration. Diffs (updated) - asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/tests.yaml 4485 asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/path/test-config.yaml PRE-CREATION asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/path/sipp/ua1_register.xml PRE-CREATION asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/path/sipp/ua1_invite_recv.xml PRE-CREATION asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/path/sipp/registrar.xml PRE-CREATION asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/path/configs/ast1/pjsip.conf PRE-CREATION asterisk/trunk/tests/channels/pjsip/registration/inbound/nominal/path/configs/ast1/extensions.conf PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3051/diff/ Testing --- Ensured the tests behaved as expected. Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3050: PJSIP: Add Path header support
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3050/ --- (Updated Jan. 6, 2014, 3:52 p.m.) Review request for Asterisk Developers. Changes --- Address Matt's and Joshua's comments. Bugs: ASTERISK-21084 https://issues.asterisk.org/jira/browse/ASTERISK-21084 Repository: Asterisk Description --- This adds Path support to chan_pjsip in res_pjsip_path.c with minimal additions in res_pjsip_registrar.c to store the path and additions in res_pjsip_outbound_registration.c to enable advertisement of path support to registrars and intervening proxies. Path information is stored on contacts and is enabled via Address of Record (AoRs) and Registration configuration sections. While adding path support, it became necessary to be able to add SIP supplements that handled messages outside of sessions, so a framework for handling these types of hooks was added in parallel to the already-existing session supplements and several senders of out-of-dialog requests were refactored as a result. Diffs (updated) - /trunk/res/res_pjsip_t38.c 405005 /trunk/res/res_pjsip_session.c 405005 /trunk/res/res_pjsip_registrar.c 405005 /trunk/res/res_pjsip_refer.c 405005 /trunk/res/res_pjsip_path.c PRE-CREATION /trunk/res/res_pjsip_outbound_registration.c 405005 /trunk/res/res_pjsip_notify.c 405005 /trunk/res/res_pjsip_nat.c 405005 /trunk/res/res_pjsip_mwi.c 405005 /trunk/res/res_pjsip_messaging.c 405005 /trunk/res/res_pjsip_header_funcs.c 405005 /trunk/res/res_pjsip_diversion.c 405005 /trunk/res/res_pjsip_caller_id.c 405005 /trunk/res/res_pjsip/pjsip_options.c 405005 /trunk/res/res_pjsip/pjsip_distributor.c 405005 /trunk/res/res_pjsip/location.c 405005 /trunk/res/res_pjsip.c 405005 /trunk/include/asterisk/res_pjsip_session.h 405005 /trunk/include/asterisk/res_pjsip.h 405005 /trunk/channels/chan_pjsip.c 405005 Diff: https://reviewboard.asterisk.org/r/3050/diff/ Testing --- This passes the tests covered by the review at https://reviewboard.asterisk.org/r/3051/ These tests were converted to work with chan_pjsip from their chan_sip counterparts. Thanks, opticron -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3104: PJSIP CLI Part 2
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3104/ --- Review request for Asterisk Developers. Repository: Asterisk Description --- Adds identify, transport and registration support to the CLI. Creates 3 additional callbacks, one for an iterator, one for a comparator and one for a container. This eliminates the link dependency from higher level modules to lower level ones. Eliminates duplicate sorting. Cleans up output formatting. Pushes cli registration down to the implementing source file. Adds several ast_sip_destroy_sorcery functions to complement existing ast_sip_sorcery_initialize functions. The destroy functions unregister cli commands and cli formatters. Diffs - branches/12/res/res_pjsip_outbound_registration.c 405000 branches/12/res/res_pjsip_endpoint_identifier_ip.c 405000 branches/12/res/res_pjsip/pjsip_configuration.c 405000 branches/12/res/res_pjsip/pjsip_cli.c 405000 branches/12/res/res_pjsip/location.c 405000 branches/12/res/res_pjsip/config_transport.c 405000 branches/12/res/res_pjsip/config_auth.c 405000 branches/12/include/asterisk/res_pjsip_cli.h 405000 branches/12/include/asterisk/res_pjsip.h 405000 Diff: https://reviewboard.asterisk.org/r/3104/diff/ Testing --- Thanks, George Joseph -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] [Code Review] 3102: res_pjsip_multihomed: Add multihomed support
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3102/ --- Review request for Asterisk Developers. Bugs: ASTERISK-23020 https://issues.asterisk.org/jira/browse/ASTERISK-23020 Repository: Asterisk Description --- This module adds messaging rewriting for multihomed situations. When messages are sent out it determines what interface and transport the message is going out on. The IP address for this is then placed into the message. I've written this as a module since it does add a bit of additional logic for outgoing messages. Diffs - /branches/12/res/res_pjsip_multihomed.c PRE-CREATION Diff: https://reviewboard.asterisk.org/r/3102/diff/ Testing --- Sent and received calls/traffic in two configurations. 1. Two transports each bound to specific interfaces 2. One transport bound to 0.0.0.0 Confirmed that the correct IP address for each interface went into the message. Some additional testing by others would be nice. Thanks, Joshua Colp -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] [Code Review] 3094: res_pjsip_messaging: potential for field values in from/to headers to be missing
---
This is an automatically generated e-mail. To reply, visit:
https://reviewboard.asterisk.org/r/3094/
---
(Updated Jan. 6, 2014, 12:49 p.m.)
Review request for Asterisk Developers.
Changes
---
Changed formatting, when possible, of uri on incoming messages to remove
redundent 'pjsip:https://issues.asterisk.org/jira/browse/ASTERISK-22924
Repository: Asterisk
Description
---
Added in ability to specify display name format ("name" )
for a given URI and made sure it was fully propagated to the outgoing message.
This mainly occurred when sending to a direct URI (using the default outbound
endpoint). MESSAGEs sent to/from a valid endpoint will use that endpoint's
AOR/contact to fill in the appropriate information.
Diffs (updated)
-
branches/12/res/res_pjsip_messaging.c 404949
branches/12/res/res_pjsip.c 404949
Diff: https://reviewboard.asterisk.org/r/3094/diff/
Testing
---
Some manual testing and also ran the current testsuite tests that exists to
make sure everything still worked as expected.
Thanks,
Kevin Harwell
--
_
-- 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 Media Performance
David Lee (digium) wrote: Synchronizing the notifications with the format change may be tricky. If we’re not careful, we could end up in the situation where 1) the format changes but some bit of code gets the notification about the change too late or 2) we end up with extra locks/contention in the media path due to format change notifications. I think we can actually do it using a control frame. The stuff that cares about the format would be in that path, and the control frame could just be in front of the frame with the new format. As it passes through stuff can change accordingly (change formats, smoother, etc). Another option would be structuring ast_format so that comparisons are cheap. If it’s a refcounted immutable object, we might even be able to do it simply with a pointer comparison (or, at the very least, the compare function can be fast if comparing an ast_format with itself). Agreed. I think regardless of the above reactive versus notification this has to be fast. Of course, I say that without actually looking at the code. Maybe there are other reasons to switch from reactive to notifications, or my concerns about extra locks and late notifications are just FUD. Just something to think about. By responding to your email I can confirm that I have thought about it at least briefly. -- Joshua Colp Digium, Inc. | Senior Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: www.digium.com & www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
Re: [asterisk-dev] Asterisk Media Performance
On Jan 6, 2014, at 11:54 AM, Joshua Colp wrote: > 1. Make ast_format an ao2 object > > I think what needs to happen is that ast_format becomes an immutable ao2 > reference counted object. Copying becomes bumping the reference count and > returning it. > > Additional related stuff can be attached and guaranteed that it will be > disposed of. This can include an actual list of attributes, and a pointer to > the format negotiatior. As a result operations become faster to do and memory > usage goes down. +1 for immutable objects. > 2. Audit format usage > > Times have changed and what we can do with Asterisk has also. We need to look > at how we are using formats internally and improve/optimize/change them. A > perfect example is the format one I used previously. Even though copying an > ast_format would become cheap there is no need to do it on every read frame. > Everything format related should be fast and quick. Agreed. In the media path, every little bit helps. > 3. Be less reactive > > We need to determine something has changed once (such as the format of > incoming media) and notify everything else involved. Reacting using the same > (or potentially more expensive) comparison logic at different points in the > chain is not needed. Synchronizing the notifications with the format change may be tricky. If we’re not careful, we could end up in the situation where 1) the format changes but some bit of code gets the notification about the change too late or 2) we end up with extra locks/contention in the media path due to format change notifications. Another option would be structuring ast_format so that comparisons are cheap. If it’s a refcounted immutable object, we might even be able to do it simply with a pointer comparison (or, at the very least, the compare function can be fast if comparing an ast_format with itself). Of course, I say that without actually looking at the code. Maybe there are other reasons to switch from reactive to notifications, or my concerns about extra locks and late notifications are just FUD. Just something to think about. -- David M. Lee Digium, Inc. | Software Developer 445 Jan Davis Drive NW - Huntsville, AL 35806 - USA Check us out at: www.digium.com & www.asterisk.org -- _ -- Bandwidth and Colocation Provided by http://www.api-digital.com -- asterisk-dev mailing list To UNSUBSCRIBE or update options visit: http://lists.digium.com/mailman/listinfo/asterisk-dev
[asterisk-dev] Asterisk Media Performance
Greetings everyone, I've been doing some profiling over the past few days on Asterisk 12 with a focus on media. I've uncovered some stuff but before I get into things let's go back in time a bit... to when things were simpler. In the old times media formats were represented using a bit field. This was fast but had a few limitations. We were limited on how many there were and they also did not include any attribute information. It was strictly a "this is ulaw". This was changed and ast_format was created, which is a structure that contains additional information. Additionally ast_format_cap was created to act as a container and another mechanism was added to allow logic to be registered which performed format negotiation. Everywhere throughout the codebase the code was changed to use this strategy but unfortunately this came at a cost... 1. Copying formats is no longer cheap. The ast_format structure is approximately 320 bytes in size. This unfortunately stems from the fact that the structure is not a reference counted astobj2 object. You can't attach other data to the structure with the expectation that it will be disposed of, thus there is a reasonable amount of room for storing arbitrary attribute information. You might think... "gee, we don't copy formats that often". You would be incorrect. Copying formats can be done 5 or more times when passing a media frame through Asterisk. 2. Comparing formats is no longer cheap. The act of comparing two formats now includes a hash table lookup to determine if additional logic should be called for a deeper attribute comparison. Yet again you might think... "gee, we don't compare formats that often". Code throughout Asterisk is reactive in many places to format changes. Instead of the ingress indicating the incoming format has changed multiple places make the determination by doing a format comparison. This can occur 4 or more times when passing a media frame through Asterisk. The reason the lookup is done when the comparison is done is because you can't attach the underlying logic to the format itself, since no guarantee exists it would be disposed of. 3. Finding associated information using a format is no longer cheap. As discussed above comparing formats is not cheap so using them as a key for something hit constantly is expensive. This happens in the RTP layer when going between RTP payload and Asterisk format. 4. Code assumes the above is cheap A lot of code is written under the assumption that the above operations are cheap, and thus some things are done in an unoptimized fashion. Instead of setting up data structures with the correct information when allocating them they are continually set. An example of this are the format modules. When reading from a ulaw file each individual frame results in a format copy, instead of the format being copied once when the file is opened. This also includes translation. Every time a translated frame is returned an ast_format_copy happens. Put all this together and you have an impact on CPU consumption when passing frames, and even when setting up calls since that involves format negotiation. To give some scope of just how much of an impact all this makes: on my quad core machine with 100 simultaneous channels doing a Playback (without transcoding but with bidirectional media) I was able to reduce the CPU usage by 8-10% by reducing format copies, reducing format comparisons, and tweaking RTP<->Format lookups. This wasn't with completely changing everything, which I'm sure would reduce CPU usage even more. So how can we improve this and make it better? 1. Make ast_format an ao2 object I think what needs to happen is that ast_format becomes an immutable ao2 reference counted object. Copying becomes bumping the reference count and returning it. Additional related stuff can be attached and guaranteed that it will be disposed of. This can include an actual list of attributes, and a pointer to the format negotiatior. As a result operations become faster to do and memory usage goes down. 2. Audit format usage Times have changed and what we can do with Asterisk has also. We need to look at how we are using formats internally and improve/optimize/change them. A perfect example is the format one I used previously. Even though copying an ast_format would become cheap there is no need to do it on every read frame. Everything format related should be fast and quick. 3. Be less reactive We need to determine something has changed once (such as the format of incoming media) and notify everything else involved. Reacting using the same (or potentially more expensive) comparison logic at different points in the chain is not needed. All of the above spans the entire code base. It's big. What we gain though is a faster media architecture that is slimmed down. I think it's worth it. What about everyone else? What do you think? Side note: After readi
Re: [asterisk-dev] [Code Review] 3061: External MWI core support with AMI using it.
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3061/ --- (Updated Jan. 6, 2014, 11:50 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Repository: Asterisk Description --- This patch can be broken into two main components: 1) The core resource support for external MWI providers are in the following files: apps/app_voicemail.c, include/asterisk/res_mwi_external.h, configs/sorcery.conf.sample, res/res_mwi_external.c, and res/res_mwi_external.exports.in. * The core external MWI resource provides for MWI message counts persistence using sorcery. With sorcery, the user is able to configure which sorcery wizzard backend to use if the default astdb is not desired. * The core external MWI resoruce provides some debugging CLI commands enabled by defining MWI_DEBUG_CLI. The debugging CLI commands are: "mwi delete all", "mwi delete like ", "mwi delete mailbox ", "mwi list all", "mwi list like ", "mwi show mailbox ", and "mwi update mailbox [ []]". 2) The AMI component of external MWI is in: res/res_mwi_external_ami.c * The external MWI AMI interface provides a thin wrapper around the core external MWI resource. The resource adds the following AMI actions: MWIGet, MWIDelete, and MWIUpdate. Diffs - /trunk/res/res_mwi_external_ami.c PRE-CREATION /trunk/res/res_mwi_external.exports.in PRE-CREATION /trunk/res/res_mwi_external.c PRE-CREATION /trunk/include/asterisk/res_mwi_external.h PRE-CREATION /trunk/configs/sorcery.conf.sample 404401 /trunk/apps/app_voicemail.c 404401 Diff: https://reviewboard.asterisk.org/r/3061/diff/ Testing --- Used the CLI "database show" along with the new CLI commands and AMI actions to test adding/updating, getting, and deleting external MWI counts. 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] no ast_channel_varshead in 1.8 [was: Re: [svn-commits] sgriepentrog: branch 1.8 r404674 - /branches/1.8/funcs/func_strings.c]
Thanks for catching that. I've updated my build script to set
TEST_FRAMEWORK so I don't miss that in the future. Patched in r404941, and
double checked to insure tests pass.
On Sat, Jan 4, 2014 at 9:46 AM, Tzafrir Cohen wrote:
> On Thu, Jan 02, 2014 at 07:32:24PM -, SVN commits to the Digium
> repositories wrote:
> > Author: sgriepentrog
> > Date: Thu Jan 2 13:32:20 2014
> > New Revision: 404674
> >
> > URL: http://svnview.digium.com/svn/asterisk?view=rev&rev=404674
> > Log:
> > func_strings: use memmove to prevent overlapping memory on strcpy
> >
> > When calling REPLACE() with an empty replace-char argument, strcpy
> > is used to overwrite the the matching . However as the
> > src and dest arguments to strcpy must not overlap, it causes other
> > parts of the string to be overwritten with adjacent characters and
> > the result is mangled. Patch replaces call to strcpy with memmove
> > and adds a test suite case for REPLACE.
> >
> > (closes issue ASTERISK-22910)
> > Reported by: Gareth Palmer
> > Patches:
> > func_strings.patch uploaded by Gareth Palmer (license 5169)
> >
> >
> > Modified:
> > branches/1.8/funcs/func_strings.c
> >
> > Modified: branches/1.8/funcs/func_strings.c
> > URL:
> http://svnview.digium.com/svn/asterisk/branches/1.8/funcs/func_strings.c?view=diff&rev=404674&r1=404673&r2=404674
> >
> ==
> > --- branches/1.8/funcs/func_strings.c (original)
> > +++ branches/1.8/funcs/func_strings.c Thu Jan 2 13:32:20 2014
> [snip]
>
> >* directly there */
> > if (strchr(find, *strptr)) {
> > if (ast_strlen_zero(replace)) {
> > - /* Remove character */
> > - strcpy(strptr, strptr + 1); /* SAFE */
> > + memmove(strptr, strptr + 1, strlen(strptr
> + 1) + 1);
> > strptr--;
> > } else {
> > /* Replace character */
> > @@ -1619,6 +1618,76 @@
> > return res;
> > }
> >
> > +AST_TEST_DEFINE(test_REPLACE)
>
> [snip]
>
> > + }
> > +
> > + AST_LIST_INSERT_HEAD(ast_channel_varshead(chan), var,
> entries);
>
> ast_channel_varshead does not exist in 1.8. This fails the builds.
>
> s/ast_channel_varshead(chan)/&chan->varshead/ ?
>
> http://svn.asterisk.org/svn/asterisk/trunk@357542
>
> > +
> > + snprintf(expression, sizeof(expression),
> "${REPLACE(%s,%s,%s)}", var->name, test_args[i].find_chars,
> test_args[i].replace_char);
> > + ast_str_substitute_variables(&str, 0, chan, expression);
> > +
> > + AST_LIST_REMOVE(ast_channel_varshead(chan), var, entries);
>
> likewise.
>
> --
>Tzafrir Cohen
> icq#16849755 jabber:tzafrir.co...@xorcom.com
> +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
>
--
[image: 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 317 507 4029
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] 3101: rasterisk needlessly prints the AST-2013-007 warning
On 01/05/2014 04:03 PM, Tzafrir Cohen wrote: On Sun, Jan 05, 2014 at 02:34:35PM -0500, Sean Darcy wrote: I agree on asterisk -rx, but as someone who just recently stopped living dangerously I suggest you keep it for a remote login. I never see asterisk start up. It's part of the systemd process. I would only see it on a remote login. And only after saw it for some number of times, did I actually get motivated. I may not be alone in needing prodding to fix things, especially where it's not the problem I'm immediately trying to fix. It's only one line. There's a lot of lower hanging fruit spamming the log file. (It's already commited) Technically it would be a trivial change to the patch. Anyway, if I decided to live dangerously, it would be annoying. This is not the only overly-verbose message Asterisk has, and users learn to ignore loud noise. Well, I wouldn't suggest we have the warning if someone explicitly opts to live dangerously, just if it's the default behavior. sean -- _ -- 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] 3101: rasterisk needlessly prints the AST-2013-007 warning
--- This is an automatically generated e-mail. To reply, visit: https://reviewboard.asterisk.org/r/3101/ --- (Updated Jan. 6, 2014, 10:38 a.m.) Status -- This change has been marked as submitted. Review request for Asterisk Developers. Bugs: ASTERISK-23084 https://issues.asterisk.org/jira/browse/ASTERISK-23084 Repository: Asterisk Description --- Even since the fixes of AST-2013-007, Asterisk prints the following warning on startup if the user decided to live dangerously: Privilege escalation protection disabled! See https://wiki.asterisk.org/wiki/x/1gKfAQ for more details. I believe that this warning is intended to be one-time and thus makes no sense when asterisk is run as a remote terminal. It certainly makes no sense when Asterisk is run to issue a remote command (asterisk -rx). This patch disables the warning on those two cases. Diffs - /branches/1.8/main/asterisk.c 404783 Diff: https://reviewboard.asterisk.org/r/3101/diff/ Testing --- Checked with asterisk 11 Thanks, Tzafrir Cohen -- _ -- 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
