<?xml version="1.0"?>
<rss version="2.0">
	<channel>
		<title>Allegro 5.1.5 source static analysis</title>
		<link>http://www.allegro.cc/forums/view/611934</link>
		<description>Allegro.cc Forum Thread</description>
		<webMaster>matthew@allegro.cc (Matthew Leverton)</webMaster>
		<lastBuildDate>Mon, 28 Jan 2013 05:20:33 +0000</lastBuildDate>
	</channel>
	<item>
		<description><![CDATA[<div class="mockup v2"><p>I had a little free time on my hands, and ran static analysis on Allegro sources again. Last year, I found a few interesting bugs this way. This time, I&#39;m happy to report, both tools used (cppcheck and PVS-Studio, latest versions) showed only a few suspicious places, none of which are really dangerous.</p><p>So, here&#39;s what I found:</p><p>1) addons\audio\kcm_stream.c:</p><div class="source-code snippet"><div class="inner"><pre><span class="c">/* Function: al_get_available_audio_stream_fragments</span>
<span class="c"> */</span>
<span class="k1">unsigned</span> <span class="k1">int</span> <a href="http://www.allegro.cc/manual/al_get_available_audio_stream_fragments"><span class="a">al_get_available_audio_stream_fragments</span></a><span class="k2">(</span>
   <span class="k1">const</span> <a href="http://www.allegro.cc/manual/ALLEGRO_AUDIO_STREAM"><span class="a">ALLEGRO_AUDIO_STREAM</span></a> <span class="k3">*</span>stream<span class="k2">)</span>
<span class="k2">{</span>
   <span class="k1">unsigned</span> <span class="k1">int</span> i<span class="k2">;</span>
   <a href="http://www.allegro.cc/manual/ASSERT"><span class="a">ASSERT</span></a><span class="k2">(</span>stream<span class="k2">)</span><span class="k2">;</span>

   <span class="k1">for</span> <span class="k2">(</span>i <span class="k3">=</span> <span class="n">0</span><span class="k2">;</span> stream-&gt;used_bufs<span class="k2">[</span>i<span class="k2">]</span> <span class="k3">&amp;</span><span class="k3">&amp;</span> i <span class="k3">&lt;</span> stream-&gt;buf_count<span class="k2">;</span> i<span class="k3">+</span><span class="k3">+</span><span class="k2">)</span>
      <span class="k2">;</span>
   <span class="k1">return</span> i<span class="k2">;</span>
<span class="k2">}</span>
</pre></div></div><p>

It&#39;s safer to check for &quot;i &lt; stream-&gt;buf_count&quot; first, because if it&#39;s 0 this code may crash. The same code is repeated on lines 322, 518, 554 in this file.</p><p>2) wsystem.c:</p><div class="source-code snippet"><div class="inner"><pre><span class="k1">static</span> <span class="k1">void</span> win_shutdown<span class="k2">(</span><span class="k1">void</span><span class="k2">)</span>
</pre></div></div><p>

ASSERT(vt) probably should be moved before the first access to vt, or removed entirely, if we are sure vt is always valid (we are not).</p><p>3) addons\audio\dsound.c:</p><div class="source-code snippet"><div class="inner"><pre>extra-&gt;desc.lpwfxFormat <span class="k3">=</span> <span class="k3">&amp;</span>format<span class="k2">;</span>
</pre></div></div><p>

The pointer to a local variable &#39;format&#39; is being saved into &#39;extra&#39; structure, which outlives &#39;format&#39;. May not be an error, since this field is not being used anywhere else, but storing an invalid pointer is never a good thing to do. Maybe set it to NULL after call to CreateCaptureBuffer?</p><p>4) allegro\examples\ex_bitmap_target.c:</p><div class="source-code snippet"><div class="inner"><pre>               <span class="k1">case</span> ALLEGRO_EVENT_KEY_DOWN:
                  <span class="k1">if</span> <span class="k2">(</span>event.keyboard.keycode <span class="k3">=</span><span class="k3">=</span> ALLEGRO_KEY_ESCAPE<span class="k2">)</span>
                     quit <span class="k3">=</span> <span class="k1">true</span><span class="k2">;</span>
                     <span class="k1">goto</span> done<span class="k2">;</span>
                  <span class="k1">if</span> <span class="k2">(</span>event.keyboard.keycode <span class="k3">=</span><span class="k3">=</span> ALLEGRO_KEY_SPACE<span class="k2">)</span>
                     <span class="k1">goto</span> done<span class="k2">;</span>
                  <span class="k1">break</span><span class="k2">;</span>
</pre></div></div><p>

The second if is never reached. Should be corrected to:</p><div class="source-code snippet"><div class="inner"><pre>               <span class="k1">case</span> ALLEGRO_EVENT_KEY_DOWN:
                  <span class="k1">if</span> <span class="k2">(</span>event.keyboard.keycode <span class="k3">=</span><span class="k3">=</span> ALLEGRO_KEY_ESCAPE<span class="k2">)</span>
                  <span class="k2">{</span>
                     quit <span class="k3">=</span> <span class="k1">true</span><span class="k2">;</span>
                     <span class="k1">goto</span> done<span class="k2">;</span>
                  <span class="k2">}</span>
                  <span class="k1">if</span> <span class="k2">(</span>event.keyboard.keycode <span class="k3">=</span><span class="k3">=</span> ALLEGRO_KEY_SPACE<span class="k2">)</span>
                     <span class="k1">goto</span> done<span class="k2">;</span>
                  <span class="k1">break</span><span class="k2">;</span>
</pre></div></div><p>

And that&#39;s all, folks! A noticeable improvement from 14 issues reported last year.</p><p>I feel I should try to give more back to Allegro, because I used this library since DJGPP days, but I don&#39;t know how. I don&#39;t know much about graphic or sound programming (I mostly do game logic and UI at work), but I&#39;m good at debugging. Unfortunately, I did very little work with iOS and Android, to which most of current bugs in tracker seems to be related.
</p></div>]]>
		</description>
		<author>no-reply@allegro.cc (Max Savenkov)</author>
		<pubDate>Sun, 27 Jan 2013 18:34:29 +0000</pubDate>
	</item>
	<item>
		<description><![CDATA[<div class="mockup v2"><p>Thanks, Max. I&#39;ll fix these.</p><div class="quote_container"><div class="title"><a href="http://www.allegro.cc/forums/thread/611934/975618#target">Max Savenkov</a> said:</div><div class="quote"><p>
I don&#39;t know much about graphic or sound programming (I mostly do game logic and UI at work), but I&#39;m good at debugging.
</p></div></div><p>

Don&#39;t worry, just jump in!
</p></div>]]>
		</description>
		<author>no-reply@allegro.cc (Peter Wang)</author>
		<pubDate>Mon, 28 Jan 2013 05:20:33 +0000</pubDate>
	</item>
</rss>
