<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/2241/">https://reviewboard.asterisk.org/r/2241/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On December 10th, 2012, 3:29 a.m., <b>wdoekes</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/2241/diff/1/?file=32350#file32350line4629" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/main/pbx.c</a>
<span style="font-weight: normal;">
(Diff revision 1)
</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 int pbx_extension_helper(struct ast_channel *c, struct ast_context *con,</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4629</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">tmp</span><span class="p">[</span><span class="mi">80</span><span class="p">],</span> <span class="n">tmp2</span><span class="p">[</span><span class="mi">80</span><span class="p">],</span> <span class="n">tmp3</span><span class="p">[</span><span class="n">EXT_DATA_SIZE</span><span class="p">];</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4628</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_verb</span><span class="p">(</span><span class="mi">3</span><span class="p">,</span> <span class="s">"Executing [%s@%s:%d] %s%s%s(</span><span class="se">\"</span><span class="s">%s%s%s</span><span class="se">\"</span><span class="s">, </span><span class="se">\"</span><span class="s">%s%s%s</span><span class="se">\"</span><span class="s">) %s</span><span class="se">\n</span><span class="s">"</span><span class="p">,</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4630</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_verb</span><span class="p">(</span><span class="mi">3</span><span class="p">,</span> <span class="s">"Executing [%s@%s:%d] %s(</span><span class="se">\"</span><span class="s">%s</span><span class="se">\"</span><span class="s">, </span><span class="se">\"</span><span class="s">%s</span><span class="se">\"</span><span class="s">) %s</span><span class="se">\n</span><span class="s">"</span><span class="p">,</span></pre></td>
<th bgcolor="#ebb1ba" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2"></font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4631</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">exten</span><span class="p">,</span> <span class="n">context</span><span class="p">,</span> <span class="n">priority</span><span class="p">,</span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4629</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">exten</span><span class="p">,</span> <span class="n">context</span><span class="p">,</span> <span class="n">priority</span><span class="p">,</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4632</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">term_color</span><span class="p">(</span><span class="n">tmp</span><span class="p">,</span> <span class="n">app</span><span class="o">-></span><span class="n">name</span><span class="p">,</span> <span class="n">COLOR_BRCYAN</span><span class="p">,</span> <span class="mi">0</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">tmp</span><span class="p">)),</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4630</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_term_color</span><span class="p">(</span><span class="n">COLOR_BRCYAN</span><span class="p">,</span> <span class="mi">0</span><span class="p">),</span> <span class="n">app</span><span class="o">-></span><span class="n">name</span><span class="p">,</span> <span class="n">ast_term_reset</span><span class="p">(),</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4633</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">term_color</span><span class="p">(</span><span class="n">tmp2</span><span class="p">,</span> <span class="n">ast_channel_name</span><span class="p">(</span><span class="n">c</span><span class="p">),</span> <span class="n">COLOR_BRMAGENTA</span><span class="p">,</span> <span class="mi">0</span><span class="p">,</span> <span class="k">sizeof</span><span class="p">(</span><span class="n">tmp2</span><span class="p">)),</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">4631</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">ast_term_color</span><span class="p">(</span><span class="n">COLOR_BRMAGENTA</span><span class="p">,</span> <span class="mi">0</span><span class="p">),</span> <span class="n">ast_channel_name</span><span class="p">(</span><span class="n">c</span><span class="p">),</span> <span class="n">ast_term_reset</span><span class="p">(),</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">4634</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n">term_color</span><span class="p">(</span><span class="n"><span class="hl">tmp3</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">passdata</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n">COLOR_BRMAGENTA</span><span class="p">,</span> <span class="mi">0</span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="k"><span class="hl">sizeof</span></span><span class="p"><span class="hl">(</span></span><span class="n"><span class="hl">tmp3</span></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">4632</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "> <span class="n"><span class="hl">ast_</span>term_color</span><span class="p">(</span><span class="n">COLOR_BRMAGENTA</span><span class="p">,</span> <span class="mi">0</span><span class="p"><span class="hl">),</span></span><span class="hl"> </span><span class="n"><span class="hl">passdata</span></span><span class="p"><span class="hl">,</span></span><span class="hl"> </span><span class="n"><span class="hl">ast_term_reset</span></span><span class="p"><span class="hl">(</span>),</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;">Some macros might relieve that.
Instead of:
ast_verb(3, "Bla: %s%d%s: %s%s%s",
ast_term_color(COLOR_BRCYAN, 0), number, ast_term_reset(),
ast_term_color(COLOR_BRCYAN, 0), name, ast_term_reset());
Do:
ast_verb(3, "Bla: %" AST_C(d) ": %" AST_C(s),
AST_COLORED(COLOR_BRCYAN, 0, number),
AST_COLORED(COLOR_BRCYAN, 0, name));
Where AST_C(d) would become "s%d%s" and AST_COLORED would expand to the three arguments.</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 honestly don't see much use for anything other than a substitution of the full "%s%s%s" and the 3 arguments. I've looked through the code and haven't seen anything like a colorization of an integer. Cite one, and I'll put forth the extra effort to implement it.</pre>
<br />
<p>- Tilghman</p>
<br />
<p>On December 9th, 2012, 9:19 p.m., Tilghman Lesher 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 Tilghman Lesher.</div>
<p style="color: grey;"><i>Updated Dec. 9, 2012, 9:19 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;">The core module related to coloring terminal output was old and needed some love. The main thing here was an attempt to get rid of the obscene number of stack-local buffers that were allocated for no other reason than to colorize some output. Instead, this uses a simple trick to allocate several buffers within threadlocal storage, then automatically rotates between them, so that you can make multiple calls to the colorization routine within one function and not need to allocate multiple buffers.</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;">The code works, however, I haven't extended the change to all parts of Asterisk, simply because I wanted to get other eyes on the code and agree on the method employed.</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/apps/app_queue.c <span style="color: grey">(377511)</span></li>
<li>/trunk/include/asterisk/term.h <span style="color: grey">(377511)</span></li>
<li>/trunk/main/asterisk.c <span style="color: grey">(377511)</span></li>
<li>/trunk/main/data.c <span style="color: grey">(377511)</span></li>
<li>/trunk/main/logger.c <span style="color: grey">(377511)</span></li>
<li>/trunk/main/manager.c <span style="color: grey">(377511)</span></li>
<li>/trunk/main/pbx.c <span style="color: grey">(377511)</span></li>
<li>/trunk/main/term.c <span style="color: grey">(377511)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2241/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>