<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/1164/">https://reviewboard.asterisk.org/r/1164/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 15th, 2011, 5:17 a.m., <b>Russell Bryant</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 code is mostly fine here ... but I'm not entirely sure it's something that makes sense to go into Asterisk. This is the kind of change that I would make (and have made many times) while debugging an issue, but just throw away when I'm done. If we did it for this function, why not every API call in Asterisk? Why not _every_ function?
Most cases where we have it today in the upstream code are for the purposes of resource tracking. For example, track the real file/func/line that allocated memory.</pre>
</blockquote>
<p>On April 15th, 2011, 5:30 a.m., <b>Olle E Johansson</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;">Because in so many cases, the hangup is a critical issue and we need to know why this happened and who issued it. I think it's important. Otherwise I would have thrown this away too, like many other debugging patches...</pre>
</blockquote>
<p>On May 9th, 2011, 5:22 p.m., <b>jrose</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;">I really don't like the idea of adding many arguments to a function that takes one argument strictly for the sake of debugging. Maybe you could add a debug-specific function that invokes this one as a helper method or something along those lines. I guess it wouldn't be as convenient if you couldn't immediately see what the invoking function/channel were, but you might be able to get around that by using some fancy preprocessor definitions to automatically add some of that data to that data to the function and function call if some in specific debugging compilation flag is enabled.</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;">jrose: you need to consider ast_mutex_init() in the context of that argument, which ostensibly takes a single argument, but actually takes 6.
Olle: given that this is such a common utility function, I think it would be better served with a parallel call to ast_log describing the reason for the hangup. Debug only gets us so far, but the reason would be better specified in the code. At the time that ast_hangup is called, we're usually well beyond the point at which a reason can be discerned; it's just cleaning up at that point.</pre>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On April 15th, 2011, 5:17 a.m., <b>Russell Bryant</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/1164/diff/1/?file=16064#file16064line1381" style="color: black; font-weight: bold; text-decoration: underline;">/trunk/include/asterisk/channel.h</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; ">int ast_channel_trace_serialize(struct ast_channel *chan, struct ast_str **out);</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1381</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">int</span> <span class="n">ast_hangup</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>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1381</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#define ast_hangup(chan)        __ast_hangup(chan, __func__, __FILE__, __LINE__)</span></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">1382</font></th>
<td bgcolor="#c5ffc4" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="kt">int</span> <span class="n">__ast_hangup</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> <span class="k">const</span> <span class="kt">char</span> <span class="o">*</span><span class="n">func</span><span class="p">,</span> <span class="k">const</span> <span class="kt">char</span> <span class="o">*</span><span class="n">file</span><span class="p">,</span> <span class="k">const</span> <span class="kt">int</span> <span class="n">line</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;">For consistency, I think most other instances of this do it in the order of file, func, line.
Also, most code uses __PRETTY_FUNCTION__ instead of __func__. I don't know what the difference is off of the top of my head though.</pre>
</blockquote>
<p>On April 15th, 2011, 5:30 a.m., <b>Olle E Johansson</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;">Ok.</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;">The difference in pure C code is minor, but if you use hybrid C++ modules, the __PRETTY_FUNCTION__ macro removes the extra cruft added to the function name by the compiler.</pre>
<br />
<p>- Tilghman</p>
<br />
<p>On April 7th, 2011, 6:21 a.m., Olle E Johansson wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('https://reviewboard.asterisk.orgrb/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 Olle E Johansson.</div>
<p style="color: grey;"><i>Updated 2011-04-07 06:21:53</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;">In many cases when I develop crazy code, I get weird hangups. It may only happen to me, but it does happen. I need to know when a hangup process is initiated and by whom. This small patch is a first step towards that goal.</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;">Tested in various versions of Asterisk. Seems to give me the output I need.</pre>
</td>
</tr>
</table>
<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/view.php?id=19080">19080</a>
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/include/asterisk/channel.h <span style="color: grey">(313005)</span></li>
<li>/trunk/main/channel.c <span style="color: grey">(313005)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1164/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>