<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 />
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Is changing AST_MAX_UNIQUEID and MAX_CHANNEL_ID from 150 really a good idea? The length was potentialy dependent upon the choice of the system name. With the impending change of the user choosing the channel uniqueid value it could be longer. Users have been known to pack other information into strings.</pre>
<br />
<div>
<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/2/?file=53918#file53918line4337" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/include/asterisk/channel.h</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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">int ast_channel_move(struct ast_channel *dest, struct ast_channel *source);</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4319</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * in uniqueid land.</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4334</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cm"> * for the purposes call records.</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">...purposes of call...</pre>
</div>
<br />
<div>
<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/2/?file=53922#file53922line835" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/main/cel.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event)</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">835</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_copy_string</span><span class="p">(</span><span class="n">channelid</span><span class="p">.</span><span class="n">unique_id</span><span class="p">,</span> <span class="n">record</span><span class="p">.</span><span class="n">unique_id</span><span class="p">,</span> <span class="n">AST_MAX_UNIQUEID</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Use sizeof(channelid.unique_id) instead of AST_MAX_UNIQUEID.</pre>
</div>
<br />
<div>
<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/2/?file=53922#file53922line839" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/main/cel.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">struct ast_channel *ast_cel_fabricate_channel_from_event(const struct ast_event *event)</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">839</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">ast_copy_string</span><span class="p">(</span><span class="n">channelid</span><span class="p">.</span><span class="n">unique_id</span><span class="p">,</span> <span class="n">record</span><span class="p">.</span><span class="n">linked_id</span><span class="p">,</span> <span class="n">AST_MAX_UNIQUEID</span><span class="p">);</span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Use sizeof(channelid.unique_id) instead of AST_MAX_UNIQUEID.</pre>
</div>
<br />
<div>
<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/2/?file=53923#file53923line6489" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/main/channel.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">static void channel_do_masquerade(struct ast_channel *original, struct ast_channel *clonechan)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">6488</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>tmp_id = <span class="hl">ast_strdupa</span>(ast_channel_uniqueid(clonechan));</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">6455</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>tmp_id = <span class="hl">*</span>(ast_channel_uniqueid(clonechan));</pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Parentheses are not necessary.
tmp_id = *ast_channel_uniqueid(clonechan);</pre>
</div>
<br />
<div>
<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/2/?file=53923#file53923line10210" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/main/channel.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">struct ast_channel *ast_channel_yank(struct ast_channel *yankee)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">10208</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>my_vars.linkedid = <span class="hl">ast_strdupa</span>(ast_channel_linkedid(yankee));</pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">10175</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span>my_vars.linkedid = <span class="hl">*</span>(ast_channel_linkedid(yankee));</pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">Parenthese are not neede here also.</pre>
</div>
<br />
<div>
<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/2/?file=53943#file53943line334" style="color: black; font-weight: bold; text-decoration: underline;">/branches/12/res/res_stasis_snoop.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="4"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">struct ast_channel *stasis_app_control_snoop(struct ast_channel *chan,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">332</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">strcpy</span><span class="p">(</span><span class="n">snoop</span><span class="o">-></span><span class="n">uniqueid</span><span class="p">,</span> <span class="n">ast_channel_uniqueid</span><span class="p">(</span><span class="n">chan</span><span class="p">)<span class="hl">);</span></span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">334</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="tb"> </span><span class="n">strcpy</span><span class="p">(</span><span class="n">snoop</span><span class="o">-></span><span class="n">uniqueid</span><span class="p">,</span> <span class="n">ast_channel_uniqueid</span><span class="p">(</span><span class="n">chan</span><span class="p">)</span><span class="o"><span class="hl">-></span></span><span class="n"><span class="hl">unique_id</span></span><span class="p"><span class="hl">);</span></span><span class="hl"> </span><span class="cm"><span class="hl">/* safe */</span></span></pre></td>
</tr>
</tbody>
</table>
<pre style="margin-left: 2em; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;">This really should be using ast_copy_string().
It was most definitely not safe before this patch and this module should not be assuming that uniqueid is going to fit.</pre>
</div>
<br />
<p>- rmudgett</p>
<br />
<p>On February 13th, 2014, 2:55 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. 13, 2014, 2:55 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">(408019)</span></li>
<li>/branches/12/tests/test_cel.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/tests/test_cdr.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/stasis/control.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/stasis/app.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/snmp/agent.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_stasis_snoop.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_stasis_recording.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_stasis_playback.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_stasis.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_pjsip_refer.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_musiconhold.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_monitor.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_fax.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/res_agi.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/parking/parking_bridge_features.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/parking/parking_applications.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/res/ari/resource_channels.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/stasis_channels.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/stasis_bridges.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/pbx.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/manager.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/features.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/endpoints.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/core_unreal.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/channel_internal_api.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/channel.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/cel.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/main/bridge_channel.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/include/asterisk/rtp_engine.h <span style="color: grey">(408019)</span></li>
<li>/branches/12/include/asterisk/channel_internal.h <span style="color: grey">(408019)</span></li>
<li>/branches/12/include/asterisk/channel.h <span style="color: grey">(408019)</span></li>
<li>/branches/12/funcs/func_channel.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_unistim.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_skinny.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_sip.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_pjsip.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_phone.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_oss.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_multicast_rtp.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_motif.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_mgcp.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_jingle.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_iax2.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_gtalk.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_console.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/channels/chan_alsa.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_voicemail.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_queue.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_minivm.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_followme.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_dumpchan.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_confbridge.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/apps/app_chanspy.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/addons/chan_ooh323.c <span style="color: grey">(408019)</span></li>
<li>/branches/12/addons/chan_mobile.c <span style="color: grey">(408019)</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>