[Freeswitch-dev] [PATCH] fixing conference dial flexibly, fixing bad pointer op
Anthony Minessale
anthony.minessale at gmail.com
Wed Oct 22 09:02:07 EDT 2008
Travis,
Thank you for the patch.
Can you please submit the patch as a svn diff from latest trunk (svn diff
src/file.c > my.diff) to http://jira.freeswitch.org and attach the diff.
The mailing list makes a mess out of patch files.
Be sure to add your name to the top of the file under contributors before
generating the diff.
We will do code review on it and add it if there are no issues.
On Wed, Oct 22, 2008 at 1:20 AM, Travis Cross <tc at travislists.com> wrote:
> Greetings,
>
> I noticed that the following currently fails in FreeSWITCH:
>
> freeswitch at domain> conference 3001 dial
> {foo=bar}sofia/internal/2001%domain 3001 XML default Conference 3001 30
>
> 2008-10-22 04:01:14 [ERR] switch_core_session.c:249
> switch_core_session_outgoing_channel() Could not locate channel type
> {foo=bar}sofia
> 2008-10-22 04:01:14 [ERR] switch_ivr_originate.c:1025
> switch_ivr_originate() Cannot create outgoing channel of type
> [{foo=bar}sofia] cause: [CHAN_NOT_IMPLEMENTED]
> API CALL [conference(3001 dial {foo=bar}sofia/internal/2001%domain 3001 XML
> default Conference 3001 30)] output:
> Call Requested: result: [CHAN_NOT_IMPLEMENTED]
>
> The example looks contrived, but this functionality is fairly critical
> when you need to add, for example, P-Asserted-Identity or Diversion
> headers for the call to go outbound.
>
> Interestingly, I found that conference bgdial works correctly, which
> got me looking at the source code.
>
> Here is the offender, conference_outcall(...) in mod_conference.c:
>
> char *dialstr =
> switch_mprintf("{ignore_early_media=true}%s", bridgeto);
> status = switch_ivr_originate(NULL, &peer_session, cause,
> dialstr, 60, NULL, cid_name, cid_num, NULL, SOF_NONE);
>
> This means that {ignore_early_media=true}{foo=bar}sofia/... is getting
> passed to switch_ivr_originate. Initially I looked for a better way
> to pass channel variables to switch_ivr_originate, but I didn't see a
> good way to do it without changing the interface around.
>
> Then I thought, why shouldn't switch_ivr_originate simply accept
> multiple sets of braces? This feature could be useful in other
> contexts: scripts, for example, that need to add channel variables and
> then include a dialstring from another application layer, just like
> conference_outcall is doing.
>
> So I came up with the following patch, which also fixes a bug in the
> current code that I'll note below the patch:
>
> ---
> Author: Travis Cross <tc at traviscross.com>
> Date: Sat Oct 18 00:40:05 2008 +0000
>
> Allow multiple sets of channel variable brackets on originate.
>
> This change is intended in part to prevent channel variables that we
> add in the C code from interfering with the user's ability to use this
> feature. This occurs, for example, in conference_outcall.
>
> diff --git a/src/switch_ivr_originate.c b/src/switch_ivr_originate.c
> index 02c1a37..57e002b 100644
> --- a/src/switch_ivr_originate.c
> +++ b/src/switch_ivr_originate.c
> @@ -577,6 +577,8 @@ SWITCH_DECLARE(switch_status_t)
> switch_ivr_originate(switch_core_session_t *sess
> switch_call_cause_t reason = SWITCH_CAUSE_NONE;
> uint8_t to = 0;
> char *var_val, *vars = NULL;
> + int var_block_count = 0;
> + char *e = NULL;
> const char *ringback_data = NULL;
> switch_codec_t *read_codec = NULL;
> uint8_t sent_ring = 0, early_ok = 1, return_ring_ready = 0, progress
> = 0;
> @@ -611,18 +613,41 @@ SWITCH_DECLARE(switch_status_t)
> switch_ivr_originate(switch_core_session_t *sess
> data++;
> }
>
> - if (*data == '{') {
> - char *e = switch_find_end_paren(data, '{', '}');
> -
> - if (e) {
> + /* extract channel variables, allowing multiple sets of braces */
> + while (*data == '{') {
> + if (!var_block_count) {
> + e = switch_find_end_paren(data, '{', '}');
> + if (!e || !*e) {
> + goto var_extract_error;
> + }
> vars = data + 1;
> - *e++ = '\0';
> - data = e;
> + *e = '\0';
> + data = e + 1;
> } else {
> - switch_log_printf(SWITCH_CHANNEL_LOG,
> SWITCH_LOG_ERROR, "Parse Error!\n");
> - status = SWITCH_STATUS_GENERR;
> - goto done;
> + if (e) {
> + *e = ',';
> + }
> + e = switch_find_end_paren(data, '{', '}');
> + if (!e || !*e) {
> + goto var_extract_error;
> + }
> + /* swallow the opening bracket */
> + int i = 0, j = 0;
> + while ((data + i) && *(data + i)) {
> + j = i; i++;
> + /* note that this affects vars[] */
> + data[j] = data[i];
> + }
> + *(--e) = '\0';
> + data = e + 1;
> }
> + var_block_count++;
> + continue;
> +
> + var_extract_error:
> + switch_log_printf(SWITCH_CHANNEL_LOG, SWITCH_LOG_ERROR,
> "Parse Error!\n");
> + status = SWITCH_STATUS_GENERR;
> + goto done;
> }
>
> /* strip leading spaces (again) */
> ---
>
> After applying the patch, conference dial works as expected. It is
> now supported and well defined to use multiple contiguous sets of
> curly brackets in a dial string to pass channel variables.
>
> The patch also corrects the following code from the original source:
>
> char *e = switch_find_end_paren(data, '{', '}');
>
> if (e) {
> vars = data + 1;
> *e++ = '\0';
> data = e;
>
> If data does not contain a closing bracket, then switch_find_end_paren
> will return the address of the end of the string. The code above
> would then set data to the first address in memory after the end of
> the string, which would be very undefined.
>
> Cheers,
>
> -- tc
>
> _______________________________________________
> Freeswitch-dev mailing list
> Freeswitch-dev at lists.freeswitch.org
> http://lists.freeswitch.org/mailman/listinfo/freeswitch-dev
> UNSUBSCRIBE:http://lists.freeswitch.org/mailman/options/freeswitch-dev
> http://www.freeswitch.org
>
--
Anthony Minessale II
FreeSWITCH http://www.freeswitch.org/
ClueCon http://www.cluecon.com/
AIM: anthm
MSN:anthony_minessale at hotmail.com <MSN%3Aanthony_minessale at hotmail.com>
GTALK/JABBER/PAYPAL:anthony.minessale at gmail.com<PAYPAL%3Aanthony.minessale at gmail.com>
IRC: irc.freenode.net #freeswitch
FreeSWITCH Developer Conference
sip:888 at conference.freeswitch.org <sip%3A888 at conference.freeswitch.org>
iax:guest at conference.freeswitch.org/888
googletalk:conf+888 at conference.freeswitch.org<googletalk%3Aconf%2B888 at conference.freeswitch.org>
pstn:213-799-1400
-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://lists.freeswitch.org/pipermail/freeswitch-dev/attachments/20081022/e5080470/attachment-0001.html
More information about the Freeswitch-dev
mailing list