Allegro.cc - Online Community

Allegro.cc Forums » Allegro Development » Allegro 5.1.5 source static analysis

This thread is locked; no one can reply to it. rss feed Print
Allegro 5.1.5 source static analysis
Max Savenkov
Member #4,613
May 2004
avatar

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'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.

So, here's what I found:

1) addons\audio\kcm_stream.c:

/* Function: al_get_available_audio_stream_fragments
 */
unsigned int al_get_available_audio_stream_fragments(
   const ALLEGRO_AUDIO_STREAM *stream)
{
   unsigned int i;
   ASSERT(stream);

   for (i = 0; stream->used_bufs[i] && i < stream->buf_count; i++)
      ;
   return i;
}

It's safer to check for "i < stream->buf_count" first, because if it's 0 this code may crash. The same code is repeated on lines 322, 518, 554 in this file.

2) wsystem.c:

static void win_shutdown(void)

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).

3) addons\audio\dsound.c:

extra->desc.lpwfxFormat = &format;

The pointer to a local variable 'format' is being saved into 'extra' structure, which outlives 'format'. 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?

4) allegro\examples\ex_bitmap_target.c:

               case ALLEGRO_EVENT_KEY_DOWN:
                  if (event.keyboard.keycode == ALLEGRO_KEY_ESCAPE)
                     quit = true;
                     goto done;
                  if (event.keyboard.keycode == ALLEGRO_KEY_SPACE)
                     goto done;
                  break;

The second if is never reached. Should be corrected to:

               case ALLEGRO_EVENT_KEY_DOWN:
                  if (event.keyboard.keycode == ALLEGRO_KEY_ESCAPE)
                  {
                     quit = true;
                     goto done;
                  }
                  if (event.keyboard.keycode == ALLEGRO_KEY_SPACE)
                     goto done;
                  break;

And that's all, folks! A noticeable improvement from 14 issues reported last year.

I feel I should try to give more back to Allegro, because I used this library since DJGPP days, but I don't know how. I don't know much about graphic or sound programming (I mostly do game logic and UI at work), but I'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.

Peter Wang
Member #23
April 2000

Thanks, Max. I'll fix these.

I don't know much about graphic or sound programming (I mostly do game logic and UI at work), but I'm good at debugging.

Don't worry, just jump in!

Go to: