<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/1441/">https://reviewboard.asterisk.org/r/1441/</a>
</td>
</tr>
</table>
<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/1441/diff/2/?file=20675#file20675line90" style="color: black; font-weight: bold; text-decoration: underline;">trunk/include/asterisk/astmm.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; ">void __ast_mm_init(void);</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#define ast_free(a) \</span></pre></td>
<th bgcolor="#e9eaa8" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">90</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">#define ast_free(a) ({ __ast_free((void *) a,__FILE__, __LINE__, __PRETTY_FUNCTION__); a = NULL; })</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#ebb1ba" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">91</font></th>
<td bgcolor="#ffc5ce" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="cp">        __ast_free(a,__FILE__, __LINE__, __PRETTY_FUNCTION__)</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>
</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;">You should leave this as two lines. Max suggested line length in the guidelines is 90.</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/1441/diff/2/?file=20677#file20677line3748" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/ccss.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 int cc_monitor_failed(void *data)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3748</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_free</span><span class="p">(<span class="hl">(</span></span><span class="kt"><span class="hl">char</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="n">failure_data</span><span class="o">-></span><span class="n">de<span class="hl">vice_name</span></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">3748</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_free</span><span class="p">(</span><span class="n">failure_data</span><span class="o">-></span><span class="n">de<span class="hl">bug</span></span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3749</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_free</span><span class="p">(<span class="hl">(</span></span><span class="kt"><span class="hl">char</span></span><span class="hl"> </span><span class="o"><span class="hl">*</span></span><span class="p"><span class="hl">)</span></span><span class="hl"> </span><span class="n">failure_data</span><span class="o">-></span><span class="n">debug</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">3749</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="n">ast_free</span><span class="p">(</span><span class="n">failure_data</span><span class="o">-></span><span class="n">debug</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;">Oops cut and paste inadvertent change error. First should be failure_data->device_name</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/1441/diff/2/?file=20678#file20678line1671" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/config.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 inclfile_destroy(void *obj)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1671</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">const</span> <span class="k">struct</span> <span class="n">inclfile</span> <span class="o">*</span><span class="n">o</span> <span class="o">=</span> <span class="n">obj</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">1671</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="k">const</span> <span class="k">struct</span> <span class="n">inclfile</span> <span class="o">*</span><span class="n">o</span> <span class="o">=</span> <span class="n">obj</span><span class="p">;</span></pre></td>
</tr>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1672</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">1672</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">1673</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; "><span class="hl">        </span><span class="n"><span class="hl">ast_</span>free</span><span class="p">(</span><span class="n">o</span><span class="o">-></span><span class="n">fname</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">1673</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <span class="n">free</span><span class="p">(</span><span class="n">o</span><span class="o">-></span><span class="n">fname</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;">Since this is an ao2 destructor callback, the o type does not need to be be const.</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/1441/diff/2/?file=20682#file20682line326" style="color: black; font-weight: bold; text-decoration: underline;">trunk/main/frame.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 __frame_free(struct ast_frame *fr, int cache)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">326</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">(</span><span class="n">fr</span><span class="o">-></span><span class="n">data</span><span class="p">.</span><span class="n">ptr</span><span class="p">)</span><span class="ew"> </span></pre></td>
<th bgcolor="#f0f0f0" style="border-left: 1px solid #C0C0C0; border-right: 1px solid #C0C0C0;" align="right"><font size="2">326</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                <span class="k">if</span> <span class="p">(</span><span class="n">fr</span><span class="o">-></span><span class="n">data</span><span class="p">.</span><span class="n">ptr</span><span class="p">)</span><span class="ew"> </span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">327</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>free</span><span class="p">(</span><span class="n">fr</span><span class="o">-></span><span class="n">data</span><span class="p">.</span><span class="n">ptr</span> <span class="o">-</span> <span class="n">fr</span><span class="o">-></span><span class="n">offset</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">327</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                        <span class="n">free</span><span class="p">(</span><span class="n">fr</span><span class="o">-></span><span class="n">data</span><span class="p">.</span><span class="n">ptr</span> <span class="o">-</span> <span class="n">fr</span><span class="o">-></span><span class="n">offset</span><span class="p">);</span></pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#f0f0f0" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">328</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <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">328</font></th>
<td bgcolor="#ffffff" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">        <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;">How about putting the calculated address into a temporary pointer for the macro to set to NULL.</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/1441/diff/2/?file=20685#file20685line3196" style="color: black; font-weight: bold; text-decoration: underline;">trunk/res/res_agi.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; ">int AST_OPTIONAL_API_NAME(ast_agi_unregister)(struct ast_module *mod, agi_command *cmd)</pre></td>
</tr>
</tbody>
<tbody>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3196</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>free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span> <span class="n">e</span><span class="o">-></span><span class="n">summary</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">3196</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="n">free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span><span class="n">e</span><span class="o">-></span><span class="n">summary</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3197</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>free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span> <span class="n">e</span><span class="o">-></span><span class="n">usage</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">3197</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="n">free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span><span class="n">e</span><span class="o">-></span><span class="n">usage</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3198</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>free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span> <span class="n">e</span><span class="o">-></span><span class="n">syntax</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">3198</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="n">free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span><span class="n">e</span><span class="o">-></span><span class="n">syntax</span><span class="p">);</span></pre></td>
</tr>
<tr>
<th bgcolor="#e9eaa8" style="border-right: 1px solid #C0C0C0;" align="right"><font size="2">3199</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>free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span> <span class="n">e</span><span class="o">-></span><span class="n">seealso</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">3199</font></th>
<td bgcolor="#fdfebc" width="50%"><pre style="font-size: 8pt; line-height: 140%; margin: 0; ">                                <span class="n">free</span><span class="p">((</span><span class="kt">char</span> <span class="o">*</span><span class="p">)</span><span class="n">e</span><span class="o">-></span><span class="n">seealso</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;">A temporary variable could come to the rescue here as well. However, it does undo the purpose of your patch in this instance.
char *doomed;
doomed = (char *) e->summary;
ast_free(doomed);
...
Also ast_free must be used if ast_malloc/ast_calloc/ast_strdup was used because if MALLOC_DEBUG is enabled, ast_free does not degenerate to free anymore.</pre>
</div>
<br />
<p>- rmudgett</p>
<br />
<p>On September 15th, 2011, 5:14 p.m., schmidts 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 David Vossel.</div>
<div>By schmidts.</div>
<p style="color: grey;"><i>Updated Sept. 15, 2011, 5:14 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;">first of all, this is just an idea so be friendly ;)
i have thought about setting a pointer to NULL after it is freed to
avoid segfaults and double free corruptions.
the most important change is to replace the define of ast_free to a
macro which just looks like this:
#define ast_free(x) ({ free((void *)x); x = NULL; })
and also to keep it working proberly change the define of ast_free_ptr
to free instead of ast_free.
the same change happens in astmm.h for malloc debugging calls:
#define ast_free(a) ({ __ast_free((void *) a,__FILE__, __LINE__, __PRETTY_FUNCTION__); a = NULL; })
after doing this changes i have seen a lot of places where a pointer is
cast at the ast_free call to remove the const state of it.
you can see in the diff all this places where this happens but i think
just cast it into void within the macro makes more sense.
because of this small changes the diff is so big, but its the same change on every part of it, except utils.h and astmm.h
only on three places i dont know how to handle this things:
in res_agi.c i had to replace ast_free with free to make it compile, i am sure there would be another way to solve this but i need some help with this.
in config.c i have the same problem like in res_agi.c
format_cap.c is allready cleared with dvossel
</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;">it compiles with and without MALLOC_DEBUG,
it starts
and it do what it should do, the pointer is set to NULL after ast_free</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_dial.c <span style="color: grey">(336161)</span></li>
<li>trunk/apps/app_voicemail.c <span style="color: grey">(336161)</span></li>
<li>trunk/channels/chan_sip.c <span style="color: grey">(336161)</span></li>
<li>trunk/include/asterisk/astmm.h <span style="color: grey">(336161)</span></li>
<li>trunk/include/asterisk/utils.h <span style="color: grey">(336161)</span></li>
<li>trunk/main/ccss.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/config.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/datastore.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/event.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/features.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/frame.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/indications.c <span style="color: grey">(336161)</span></li>
<li>trunk/main/taskprocessor.c <span style="color: grey">(336161)</span></li>
<li>trunk/res/res_agi.c <span style="color: grey">(336161)</span></li>
</ul>
<p><a href="https://reviewboard.asterisk.org/r/1441/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>