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