while and if and braces … oh my.

Another day, another issue with a C compiler of questionable quality…

Consider this bit of C code, which lives in an infinite main loop and is designed to do something every now and then (based on the status of a variable being toggled by a timer interrupt):

while (timeToDoSomething == true)
{
    timeToDoSomething = false;

    // Do something.
}

The program in question was trying to Do Something every 25 ms. A timer interrupt was toggling a boolean to true. The main loop would check that flag and if it were true it would set it back to false then handle whatever it was it was supposed to handle.

While this would have worked with “while”, it would really be better as an “if” — especially if the code to handle whatever it was supposed to handle took longer than 25ms causing the loop to get stuck.

Thus, it was changed to an “if”, but a typo left the old while still in the code:

//
while (timeToDoSomething == true)
if (timeToDoSomething == true)
{
    timeToDoSomething = false;

    // Do something.
}

Since things were taking longer than 25ms, the new code was still getting stuck in that loop — and that’s when the while (which was supposed to be commented out) was noticed.

The while without braces or a semicolon after it generated no compiler warning. That seemed wrong, but even GCC with full error reporting won’t show a warning.

Because C is … C.

Curly braces! Foiled again.

In C, it is common to see code formatted using whitespace like this:

if (a == 1)
    printf("One!\n");

That is fine, since it is really just doing this:

if (a == 1) printf("One!\n");

…but is considered poor coding style these days because many programmers seem to be used to languages where indention actually means something — as opposed to C, where whitespace is whitespace. Thus, you frequently find bugs where someone has added more code like this:

if (a == 1)
    printf("One!\n");
    DoSomething ();
    printf("Done.\n");

Above, it feels like it should execute three things any time a is 1, but to C, it really looks like this:

if (a == 1) printf("One!\n");
DoSomething ();
printf("Done.\n");

Thus, modern coding standards often say to always use curly braces even if there is just one thing after the if:

if (a == 1)
{
    printf("One!\n");
}

With the braces in place, adding more statements within the braces would work as expected:

if (a == 1)
{
    printf("One!\n");
    doSomething ();
    printf("Done.\n");
}

This is something that was drilled in to my brain at a position I had many years ago, and it makes great sense. And, the same thing should be said about using while. But while has it’s own quirks. Consider these two examples:

// This way:
while (1);

// That way:
while (1)
{
}

They do the same thing. One uses a semicolon to mark the end of the stuff to do, and other uses curly braces around the stuff to do. That’s the key to the code at the start of this post:

while (timeToDoSomething == true)
if (timeToDoSomething == true)
{
    timeToDoSomething = false;

    // Do something.
}

Just like you could do…

while (timeToDoSomething == true) printf("I am doing something");

…you could also write it as…

while (timeToDoSomething == true)
{
    printf("I am doing something");
}

So when the “if” got added after the “while”, it was legit code, as if the user was trying to do this:

while (timeToDoSomething == true)
{
    if (timeToDoSomething == true)
    {
        timeToDoSomething = false;

        // Do something.
    }
}

Since while can be followed by braces or a statement, it can also be followed by a statement using just braces.

The compiler can’t easily warn about needing a brace, since it is not required to have braces. But if it braces were required, that would catch the issues mentioned here with if and while blocks.

Code that looks like it should at least generate a warning is completely valid and legal C code, and that same code can be formatted in a way that makes it clear(er):

while (timeToDoSomething == true)
    if (timeToDoSomething == true)
    {
        timeToDoSomething = false;

        // Do something.
    }

Whitespace makes things look pretty, but lack of it can also make things look wrong. Or correct when they aren’t.

I suppose the soapbox message of today is just to use braces. That wouldn’t have caught this particular typo (forgetting to comment something out), but its probably still good practice…

Until next time…

Leave a Reply

This site uses Akismet to reduce spam. Learn how your comment data is processed.