<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/1773/">https://reviewboard.asterisk.org/r/1773/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On March 1st, 2012, 4:45 p.m., <b>Tilghman Lesher</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/1773/diff/2/?file=25075#file25075line751" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/channel_internal_api.c</a>
<span style="font-weight: normal;">
(Diff revision 2)
</span>
</th>
</tr>
</thead>
<tbody style="background-color: #e4d9cb; padding: 4px 8px; text-align: center;">
<tr>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<td colspan="2"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">struct timeval *ast_channel_whentohangup(struct ast_channel *chan)</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">751</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">varshead</span> <span class="o">*</span><span class="n">ast_channel_varshead</span><span class="p">(</span><span class="k">struct</span> <span class="n">ast_channel</span> <span class="o">*</span><span class="n">chan</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">752</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">753</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="k">return</span> <span class="o">&</span><span class="n">chan</span><span class="o">-></span><span class="n">__do_not_use_varshead</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">754</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="p">}</span></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;">Hopefully, this API is going to be revisited at some point before a major release is made. There's always been some discussion about the inefficiency of a linked list for variables, and this change, although making channel opaque, does not actually allow the backend of channel variable storage to be changed in any real way.
It needs additional accessor functions, e.g. ast_channel_variable_name(), ast_channel_variable_value(), ast_channel_variable_attribute(), ast_channel_variable_lookup_next(), etc., with an opaque variable container, so that the implementation can change to e.g. a hash table backend.
My concern is that while we're going through the pain of changing the core channel API, we should be ensuring this API actually makes the storage of these values really opaque, instead of just hiding a pointer, as this is now.</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;">This is just the first stage. The point being to make as few semantic changes as humanly possible to get to an opaque structure. After things are opaque, it becomes easier to make the semantic changes a little at a time without worrying about introducing bugs in the middle of a 10k line patch. The next stage is to do more than just making a one-to-one mapping to the internal structure, but instead make the API actually make sense.</pre>
<br />
<p>- Terry</p>
<br />
<p>On February 27th, 2012, 5:47 p.m., Terry Wilson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.org/media/rb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Terry Wilson.</div>
<p style="color: grey;"><i>Updated Feb. 27, 2012, 5:47 p.m.</i></p>
<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;">Opaquify lists and remaining structs. I had do a lot of manual fixing in this one. I also added a SWAP macro in chan_local because I was tired of typing and saw some copy/paste code that could use it. I can type things out if people really want me to. :-p Setters for the structs are just wrappers around memcpy. For the most part I could have done things like *ast_channel_caller(chan1) = *ast_channel_caller(chan2), but ick.</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;">Unit tests pass.</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>/trunk/addons/chan_ooh323.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_alarmreceiver.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_amd.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_confbridge.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_dial.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_disa.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_dumpchan.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_fax.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_followme.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_macro.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_meetme.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_minivm.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_mixmonitor.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_osplookup.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_parkandannounce.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_privacy.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_queue.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_readexten.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_rpt.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_setcallerid.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_sms.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_stack.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_talkdetect.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_voicemail.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_while.c <span style="color: grey">(357092)</span></li>
<li>/trunk/apps/app_zapateller.c <span style="color: grey">(357092)</span></li>
<li>/trunk/bridges/bridge_builtin_features.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_agent.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_console.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_dahdi.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_gtalk.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_h323.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_iax2.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_jingle.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_local.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_mgcp.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_misdn.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_oss.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_phone.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_sip.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_skinny.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_unistim.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_usbradio.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/chan_vpb.cc <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/console_video.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/sig_analog.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/sig_pri.c <span style="color: grey">(357092)</span></li>
<li>/trunk/channels/sig_ss7.c <span style="color: grey">(357092)</span></li>
<li>/trunk/funcs/func_blacklist.c <span style="color: grey">(357092)</span></li>
<li>/trunk/funcs/func_callerid.c <span style="color: grey">(357092)</span></li>
<li>/trunk/funcs/func_dialplan.c <span style="color: grey">(357092)</span></li>
<li>/trunk/funcs/func_strings.c <span style="color: grey">(357092)</span></li>
<li>/trunk/funcs/func_timeout.c <span style="color: grey">(357092)</span></li>
<li>/trunk/include/asterisk/channel.h <span style="color: grey">(357092)</span></li>
<li>/trunk/include/asterisk/utils.h <span style="color: grey">(357092)</span></li>
<li>/trunk/main/abstract_jb.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/app.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/autochan.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/ccss.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/cdr.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/cel.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/channel_internal_api.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/cli.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/dial.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/features.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/file.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/manager.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/message.c <span style="color: grey">(357092)</span></li>
<li>/trunk/main/pbx.c <span style="color: grey">(357092)</span></li>
<li>/trunk/pbx/pbx_lua.c <span style="color: grey">(357092)</span></li>
<li>/trunk/res/res_agi.c <span style="color: grey">(357092)</span></li>
<li>/trunk/res/res_fax.c <span style="color: grey">(357092)</span></li>
<li>/trunk/res/snmp/agent.c <span style="color: grey">(357092)</span></li>
<li>/trunk/tests/test_substitution.c <span style="color: grey">(357092)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1773/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>