<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://reviewboard.asterisk.org/r/3191/">https://reviewboard.asterisk.org/r/3191/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On February 19th, 2014, 5:23 p.m. CST, <b>Matt Jordan</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<table width="100%" border="0" bgcolor="white" style="border: 1px solid #C0C0C0; border-collapse: collapse; margin: 2px padding: 2px;">
<thead>
<tr>
<th colspan="4" bgcolor="#F0F0F0" style="border-bottom: 1px solid #C0C0C0; font-size: 9pt; padding: 4px 8px; text-align: left;">
<a href="https://reviewboard.asterisk.org/r/3191/diff/5/?file=54211#file54211line178" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/include/asterisk/channel.h</a>
<span style="font-weight: normal;">
(Diff revision 5)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">extern "C" {</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">178</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm">/*!</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">179</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * \brief Channel UniqueId structure</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">180</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * \note channel creation time used for determining LinkedId Propagation</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">181</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> */</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">182</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="k">struct</span> <span class="n">ast_channel_id</span> <span class="p">{</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">183</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="kt">time_t</span> <span class="n">creation_time</span><span class="p">;</span> <span class="cm">/*!< Creation time */</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">184</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="kt">int</span> <span class="n">creation_unique</span><span class="p">;</span> <span class="cm">/*!< sub-second unique value */</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">185</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="kt">char</span> <span class="n">unique_id</span><span class="p">[</span><span class="n">AST_MAX_UNIQUEID</span><span class="p">];</span> <span class="cm">/*!< Unique Identifier - can be set on originate */</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">186</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">};</span></pre></td>
</tr>
<tr>
<th bgcolor="#b1ebb0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#b1ebb0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">187</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
</table>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This struct should be opaque, in the same way that all other channel internals are also opaque.
That means directly accessing the unique_id field should not be allowed. Instead, calling ast_channel_uniqueid or ast_channel_linkedid should still return a const char *, that happens to map to the unique_id contained in the structure.
The fact that we now have a structure should be - almost entirely - an implementation detail of channel.c and internal_channel.c, and not exposed to users of the channel. That's a large part of why we opaquified the channel.
That will, unfortunately, undo some of the changes you made - but I'd hate to undo the channel opaquification that was done by others in Asterisk 11.</pre>
</blockquote>
<p>On February 19th, 2014, 8:47 p.m. CST, <b>rmudgett</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">ast_channel is still opaque. The ast_channel_id is doing the same thing as the ast_party_caller, ast_party_connected_line, ast_party_dialed, ast_party_redirecting, and some other structs that are in ast_channel. Though I suppose the ast_channel_id could itself be made opaque with an accessor function for the uniqueid.
Another way would be to make ast_channel_uniqueid() and ast_channel_linkedid() return the associated ast_channel_id.unique_id so the callers won't need to be changed. Then another ast_channel accessor function could be created to return a pointer to the ast_channel_id if it is needed. Or the ast_channel_id struct may not need to be exposed outside of channel_internal_api.c.</pre>
</blockquote>
</blockquote>
<pre style="margin-left: 1em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Hiding the existence of the time structures to anything outside channel_internal can be done, but would require: a) copy of linkedid from one channel to another to be located in channel_internal as a function, b) the function would have to look up the channel structure by the char*linkedid being passed around to get the entire structure again to copy it, unless the caller has the ptr to the channel structure itself (bridge linkedid propagation does, but creating new channel inheriting another's linkedid doesn't).</pre>
<br />
<p>- Scott</p>
<br />
<p>On February 19th, 2014, 3:38 p.m. CST, Scott Griepentrog wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/static/rb/images/review_request_box_top_bg.ab6f3b1072c9.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Scott Griepentrog.</div>
<p style="color: grey;"><i>Updated Feb. 19, 2014, 3:38 p.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Bugs: </b>
<a href="https://issues.asterisk.org/jira/browse/ASTERISK-23120">ASTERISK-23120</a>
</div>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
Asterisk
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This is the first phase of channel uniqueid changes for ASTERISK-23120.
* ast_channel_uniqueid structure replaces ast_string values uniqueid and linkedid in channel structure
struct ast_channel_id {
char unique_id[AST_MAX_UNIQUEID]; /*!< Unique Identifier - can be set on originate */
time_t creation_time; /*!< Creation time */
int creation_unique; /*!< sub-second unique value */
}
* ast_channel_linkedid() and ast_channel_uniqueid() now return ptr to struct, not char *
* all references to uniqueid & linkedid updated to either pass entire structure because full uniqueid with time must be propagated, or just the ->unique_id string element.
* an issue with argument order to ast_channel_alloc() in chan_mgcp.c was corrected [BUGFIX].
* an issue with argument order to ast_channel_alloc() in chan_gtalk.c was corrected [BUGFIX].
* there should be a slight performance improvement by removing the ast_string handling of id's, but at the cost of +~250 bytes to the channel structure.
* defines for AST_MAX_UNIQUEID (channel.h) and MAX_CHANNEL_ID (rtp_engine.h) have been changed from 150 to 128 to reduce structure alignment issues (and also just because 150 is ridiculously large)
</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Ran new linkedid_check test and received same results. Also ran some bridge tests to check for asserts.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/branches/12/tests/test_substitution.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/tests/test_cel.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/tests/test_cdr.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/stasis/control.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/stasis/app.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/snmp/agent.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_stasis_snoop.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_stasis_recording.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_stasis_playback.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_stasis.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_pjsip_refer.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_musiconhold.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_monitor.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_fax.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/res_agi.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/parking/parking_bridge_features.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/parking/parking_applications.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/res/ari/resource_channels.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/stasis_channels.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/stasis_bridges.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/pbx.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/manager.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/features.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/endpoints.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/core_unreal.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/channel_internal_api.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/channel.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/cel.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/main/bridge_channel.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/include/asterisk/rtp_engine.h <span style="color: grey">(408446)</span></li>
<li>/branches/12/include/asterisk/channel_internal.h <span style="color: grey">(408446)</span></li>
<li>/branches/12/include/asterisk/channel.h <span style="color: grey">(408446)</span></li>
<li>/branches/12/funcs/func_channel.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_vpb.cc <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_unistim.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_skinny.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_sip.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_pjsip.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_phone.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_oss.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_nbs.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_multicast_rtp.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_motif.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_misdn.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_mgcp.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_jingle.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_iax2.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_h323.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_gtalk.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_dahdi.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_console.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/channels/chan_alsa.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_voicemail.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_queue.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_minivm.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_followme.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_dumpchan.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_confbridge.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/apps/app_chanspy.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/addons/chan_ooh323.c <span style="color: grey">(408446)</span></li>
<li>/branches/12/addons/chan_mobile.c <span style="color: grey">(408446)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/3191/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>