Page 1 of 1

Dangerous Code (C/C++)

Posted: Thu Oct 02 2008
by Devteam
Some coding practices can cause problems in release and/or debug builds and pass unnoticed by the compiler. The code itself is NOT problematic but missing a semi-colon can cause problems while compiling correctly.

For example:

Case 1:

Code: Select all

if (A)
    statement_1;
else
    AGL_ASSERT(B);
statement_2;
Case 2:

Code: Select all

If (C)
    AGL_ASSERT(D);
Else
    statement_3;
Looking at the definition of AGL_ASSERT

Code: Select all

#ifdef _DEBUG_GLIB_TRACE
    #define AGL_ASSERT(x) if (!(x)) AGL_CrtDbgBreak();
#else
    #define AGL_ASSERT(x)
If we miss a semi-colon after AGL_ASSERT, the problems arise and what we see is NOT what we get at runtime.

In case 1, if _DEBUG_GLIB_TRACE is not defined (in release mode), the statement_2 after the AGL_ASSERT gets executed when A is false.

In case 2, if _DEBUG_GLIB_TRACE IS defined, statement_3 is executed if C is true AND D is true:

Code: Select all

If (C)
    if (!(D))
        AGL_CrtDbgBreak();
    else
        statement_3;
So my recommendation is to never #define anything as an empty statement or define it as a series of statements without enclosing it between curly brackets, we can easily get a troublesome code.
I would use the following definition for AGL_ASSERT

Code: Select all

#ifdef _DEBUG_GLIB_TRACE
    #define AGL_ASSERT(x) { if (!(x)) AGL_CrtDbgBreak(); }
#else
    #define AGL_ASSERT(x) do {;} while(0)
NB. As for the do {;} while(0), the compiler would optimize the code and remove it safely from the binary.