<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/2040/">https://reviewboard.asterisk.org/r/2040/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 24th, 2012, 11:17 a.m., <b>opticron</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/2040/diff/5/?file=30356#file30356line137" style="color: black; font-weight: bold; text-decoration: underline;">trunk/channels/chan_skinny.c</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="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; ">static int skinnydebug = 0;</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">137</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#define SKINNY_DEVMODE</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">137</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">char</span> <span class="n">global_buf</span><span class="p">[</span><span class="mi">256</span><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;">I'd prefer the locations that use global_buf to have individual buffers (defined in dev mode only if you want) to avoid multiple threads trying to access the buffer simultaneously.</pre>
</blockquote>
<p>On July 24th, 2012, 3:47 p.m., <b>wedhorn</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;">The buffer is used in three places, part of the skinny device register process (basically once per device), a new call (every call), and skinny debug show cli (when used). The most likely cause of multiple threads accessing the buffer at the same time would be two simultaneous calls (on different devices), both of which would be using the same new buffer anyway.
Basically this stuff is purely for dev work, and personally i can live with the odd bit of garbage on the cli (only in dev mode) which would be preferably to something like adding locks which could result in slightly different functionality between dev mode and normal mode.
Happy to do the change, but not sure of the value added.</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;">I'm more concerned about the person doing debugging getting the wrong messages which most certainly not help with debugging.</pre>
<br />
<p>- opticron</p>
<br />
<p>On July 17th, 2012, 8:44 p.m., wedhorn 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 and Matt Jordan.</div>
<div>By wedhorn.</div>
<p style="color: grey;"><i>Updated July 17, 2012, 8:44 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;">Rewrite of skinny debugging.
Debugging messages and associated controls only compiled in if configured with --enable-dev-mode
Debug messages provide more detail (including thread id) and are grouped so the user/dev can limit the type of messages displayed.
Functionally no real change to chan_skinny.
While the diff is fairly large, there are basically 3 components,
- definition of SKINNY_DEBUG at lines 134-152
- control functions at lines 3238-3374
- numerous uses of SKINNY_DEBUG throughout</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;">Been running for a while with --enable-dev-mode. Compiled and tested basic functionality without --enable-dev-mode.</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/build_tools/cflags-devmode.xml <span style="color: grey">(369904)</span></li>
<li>trunk/channels/chan_skinny.c <span style="color: grey">(369904)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2040/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>