<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/2825/">https://reviewboard.asterisk.org/r/2825/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On September 5th, 2013, 5:08 a.m. UTC, <b>Tilghman Lesher</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;">It's not clear why adding a magic value for cli output is the answer here.</pre>
</blockquote>
<p>On September 6th, 2013, 4:12 p.m. UTC, <b>Kevin Harwell</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;">There must be a way to tell if incoming text on a remote console are log messages vs. cli output.
Say for instance the log level was 3 on the remote console and a few lines of log messages had been sent to it. At this point the last known log level was 3. Now a cli output message is sent to the remote console and since it has no "magic" the last known log level was assumed and it would print the cli output using the verbose level 3 prefix.
By adding a "magic" value to the cli output going to a remote console it can now tell the difference between incoming log messages and cli output and print them appropriately.</pre>
</blockquote>
<p>On September 6th, 2013, 4:20 p.m. UTC, <b>Tilghman Lesher</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;">A magic value is only supposed to be good until a newline. Once you hit the newline, you reset back to cli output unless you see another magic value directly after the newline character.</pre>
</blockquote>
<p>On September 6th, 2013, 4:39 p.m. UTC, <b>Kevin Harwell</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;">There are some scenarios where that would fail, thus the discrepancies. For instance, if someone did the following:
ast_verb(3, "hello\nworld\n");
That would only output the level 3 prefix for the "hello" and then no prefix for "world". Another example would be if there is no newline:
ast_verb(3, "hello world");
If cli output is received the first line would be on the same line as the log message. If another verbose log message, say at level 2, was issued it would also show up appended to the level 3 message.</pre>
</blockquote>
<p>On September 6th, 2013, 4:52 p.m. UTC, <b>Tilghman Lesher</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;">If either of those cases existed in the code, they should definitely be fixed. It's kind of hard to argue, "Well, if there are bugs in the code, then things don't work correctly, so we should code around it, instead of fixing those bugs." Think about it.</pre>
</blockquote>
<p>On September 6th, 2013, 5:56 p.m. UTC, <b>Matt Jordan</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;">We've never had a limitation against putting multi-line output in a single verbose message before. (There's probably even more debug messages that do this). I'm not against that limitation - I personally don't care for log messages that are multiple lines - but that doesn't mean there aren't plenty of instances of it littered throughout the source code. In fact, Asterisk even leads off with one:
#define WELCOME_MESSAGE \
ast_verbose("Asterisk %s, Copyright (C) 1999 - 2013 Digium, Inc. and others.\n" \
"Created by Mark Spencer <markster@digium.com>\n" \
"Asterisk comes with ABSOLUTELY NO WARRANTY; type 'core show warranty' for details.\n" \
"This is free software, with components licensed under the GNU General Public\n" \
"License version 2 and other licenses; you are welcome to redistribute it under\n" \
"certain conditions. Type 'core show license' for details.\n" \
"=========================================================================\n", ast_get_version()) \
We'd certainly have to publicize "Don't do this, you screw up remote consoles" for all contributors. Finding all of these would certainly be a joy as well.
The fact that we have to massage the output as it goes to the remote console in such a fashion is the result of the feature in Asterisk 11. I'd prefer to get that tolerant of as many scenarios as possible *and* make a note not to ever allow messages like that again.</pre>
</blockquote>
</blockquote>
<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 also add that possible multi-line strings can come from an embedded string or a string created from another function/module/etc. One still has to check and possibly parse any string given to a log statement since it may contain multiple lines.
I don't believe the second case is a bug either. There may be instances (if some are not already in Asterisk) where a developer may want/need to write out part of a log message and then finish that line later.</pre>
<br />
<p>- Kevin</p>
<br />
<p>On September 6th, 2013, 4:14 p.m. UTC, Kevin Harwell 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.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for Asterisk Developers.</div>
<div>By Kevin Harwell.</div>
<p style="color: grey;"><i>Updated Sept. 6, 2013, 4:14 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-22450">ASTERISK-22450</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 a follow up to https://reviewboard.asterisk.org/r/2798/
The remote console continued to have issues with its output. In this case CLI command output would either not show up (if verbose level = 0) or would contain verbose prefixes (if verbose level > 0) once log messages were sent to the remote console. The fix now distinguishes between verbose log messages and cli command output sent to a remote console.</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;">Duplicated issues. Then ran scenarios again and observed that the issues were no longer occurring. Also tried several other combinations of log levels and cli commands between consoles (local/remote).</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/11/include/asterisk/cli.h <span style="color: grey">(398227)</span></li>
<li>branches/11/main/asterisk.c <span style="color: grey">(398227)</span></li>
<li>branches/11/main/cli.c <span style="color: grey">(398227)</span></li>
<li>branches/11/main/logger.c <span style="color: grey">(398227)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/2825/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>