Subtle C Equality Bug I didn't See

λ October 1, 2019
Tags: c, video, gaming, sdl

Having now firmly abandoned my attempts at writing a video game with Mercury because I realised I was more into writing a game instead of of proving to the world that I have the brains to use and understand Mercury. I do, but. That’s a life post for another day.

TL;DR

Be very careful with using assignments in if statements as using ‘==’ instead ‘=’ can have drastic effects!

Why plain C ?

Because, after trying to write this game in in…

  • Haskell
  • Mercury
  • Prolog
  • Lisp

I realised a couple of things:

  • I don’t need to prove to myself anymore that I am not dumb just because I didn’t go to university. That has affected me my entire life.
  • I don’t want to write another SDL2 wrapper in another language ever again
  • I want to complete the game more than anything!

I looked at C++ a year or so back and walked away for a variety of reasons. C++ has become something I’d rather not do business with anymore.

So that left either Go (which I know enough to code) or C. C was the first language I really mastered many many moons ago, I lost my trusty copy of the “K&R” book too. :(

Having spent over a decade within the FP (functional programming) mindset and knowing too well the pitfalls of sloppy C coding (segfaults, dangling pointers etc.) I was convinced that I could apply FP techniques along with good TDD practice to produce a simple space shooter as practice and as an apprentice piece into the world of indie gaming.

Goals of my Game.

Having grown up in the era of Bugbyte, Manic Miner, Jet Set Willy and all my fave games on my Atari 400, I do not know exactly how the game will end up but it will:

  • be loud, very loud and noisy
  • be manic, fast and addictive
  • have different levels
  • not segfault, ever
  • be something Jeff Minter would like to play

Pre-coding fears…

I have always shied away from selling anything I have ever written because I always think that everything I do sucks. It probably doesn’t but that’s the way it is. Anyway, starting out on this game I have been very wary of NOT dynamically allocating memory and instead using fixed allocation through compiler generated array sizes and then being fastidious with assert() calls and range checking etc. So far so good.

I also found cmocka , which has proven to be lightweight and very very easy to use. If you are wondering about C and TDD frameworks I highly recommend it even after only a few hours of use, I blogged about it already in a previous rambling you might like to read.

Enough already, tell us about the bug!

The Bug

Well, here’s the offending code, it’s from the asset manage that loads in the audio effects and music during the startup phase of the game:

STATICFN void audio_load_sfx(APPSTATE* s) {
  assert(s);
  char filename[1024+1];
  Mix_Chunk *chunk = NULL;

  for(int i = sfxFirst; i < sfxLast; i++)
  {
    if (assetsSFX[i])
      {
        sprintf(filename, "%s%s", SFX_BASE_FOLDER, assetsSFX[i]);

        if (chunk == _load_chunk(filename))
          {
            eInfo("SFX loaded[%i]: %s %p\u2713", i, filename, chunk);
            s->audio.sfx[i] = chunk;
          }
        else
          {
            s->err = Mix_GetError();
            eCrit("SFX failed: %s: %s", filename, s->err);
          }
      }
    else
    {
      s->err = "Code error: not enough SFX asset names";
      eCrit("audio_load_sfx: %s", s->err);
    }
  }
}

As you can see it looks innocuous enough, and here is the SDL logger output that got me scratching my head:

INFO: SDL_Init OK, flags: ffffbfce
INFO: SDL_GetBasePath => /home/sean/Documents/c/ddgame/
INFO: SDL_GetPrefPath => /home/sean/.local/share/uk.co.madx/ddgame/
INFO: Mix_Init OK
INFO: IMG_Init OK, flags: 0003
INFO: SFX loaded[0]: ./assets/SOUNDS/./Weapons/Lasers/sfx_wpn_laser 10.wav (nil)✓
INFO: MUS loaded[0]: ./assets/MUSIC/4-track_from_heaven.mod (nil)✓
ERROR: audio_musicON: fade: music parameter was NULL
CRITICAL: Startup failed, check log output

Lines 6 and 7 have (nil) but the code path implies that the resource loaded OK. WTAF? Looking up we can see that the initial setting for for this `` variable is NULL, that’s line 4 of the previous code block. Duh!

So when it gets to line 12, the == (the bug!) means that it looked like it worked and of course it didn’t.

The Fix

The fix was simple: change == to = and good to go.

if (chunk == _load_chunk(filename))
  {
  }

Avoiding it again

The lesson learned then is that really, one shouldn’t place assignment statements into if tests. I know that sometimes it’s nice to save vertical space but I use GNU style and that doesn’t not care at all about verticals!

So…

  • Don’t put assignments in ‘if’ conditionals
  • Always put the constant first.
  • Use more
  • Use a C linter as well…that’s on my TODO for sure!
  • Use -Wall and -Werror

Happy coding!

:)

Comments