Trigger warning: This post will show some examples that “just work” and “just work fine” but may not be correct. I am posting this because I’d like to hear how you do this. Show us all a much better way.

---

I was today years old when I “learned” (possibly incorrectly) something (possibly) wrong that I have seen “always” done in the C debug print macros I’ve encountered.

Debug Macros: Basic

To enabled or disable debugging printf output, I often see macros like this:

#if defined(DEBUG_ENABLED)
    #define DEBUG_PRINTF(...) printf(__VA_ARGS__)
#else
    #define DEBUG_PRINTF(...)
#endif

This allows debug printfs to appear in code when debug is enabled or not exist at all when debugging is not enabled:

DEBUG_PRINTF ("Starting up system...\n");

Debug Macros: Intermediate

There is a fancier version of this macro. Instead of just using a #define DEBUG_ENABLED (present means enabled), the code uses a #define set to a number. That number is used to tell when to include the debug print:

#if (DEBUG_LEVEL > 1)
    DEBUG_PRINTF ("tlv_parse_ptr (0x%x, %u, 0x%x)\r\n",
                  (unsigned int)((uintptr_t)p_buf),
                  buf_size, (unsigned int)(uintptr_t)p_tlv_table);
#endif

If you “#define DEBUG_LEVEL 1”, the above code will not be part of the program. If the DEBUG_LEVEL is defined to be greater than 1, it will.

That also “just works” but makes the source code messier with all those extra “#if (DEBUG_LEVEL > x)” and #endif” lines.

Debug Macros: Advanced

This leads to the even fancier version which builds the level check into the macro itself. Those get used like:

DEBUG_PRINTF (2, "This will only print at level 2 and above.\n");

The downside of that approach is that a #define macro cannot contain preprocessor #if to check another #define macro inside of it. You can NOT do this:

#define DEBUG_PRINTF(level, ...) \
    #if (level < DEBUG_LEVEL)     \
        printf (__VA_ARGS__);    \
    #endif

Instead, the macro must have actual C code with an “if” and such, which embeds that extra code into your program:

#define DEBUG_PRINTF(level, ...) \
    if (level < DEBUG_LEVEL)     \
    {                            \
        printf(__VA_ARGS__);     \
    }

The problem with this is that even with debugging disabled (where you may not want the bulk of printf included in your program) it will still include all that macro C code in your program. With DEBUG_LEVEL enabled:

#define DEBUG_LEVEL 1
DEBUG_PRINTF(0, "Starting system up...\n")

…you get:

if (0 < 1)
{
    printf ("Starting system up...\n");
}

But if you do not want debugging, and set the debug level to 0, you’d get this:

#define DEBUG_LEVEL 0
DEBUG_PRINTF(0, "Starting system up...\n")

if (0 < 0)
{
    printf ("Starting system up...\n");
}

That is more like using some syslog() library that is always present, even if you are logging things below the level that go to the system log. But the point of using macros is so you can have the code NOT included at all when you don’t want to use it.

A bit more work is needed, which leads to something like this:

#define DEBUG_LEVEL 1

#if (DEBUG_LEVEL > 0)
    #define DEBUG_PRINTF(level, ...) \
        if (level < DEBUG_LEVEL)     \
        {                            \
            printf(__VA_ARGS__);     \
        }
#else
    #define DEBUG_PRINTF(level, ...)
#endif

Now we get what we want when we want it, and get nothing when we don’t.

But that’s really not important to this story…

The above macros are things that “just work” which may explain why I see things like that often.

But today I learned they are actually not fine. Our good robot friend Copilot “taught” me that the macro above should really look like this, with the code wrapped in something like a do/while:

#define DEBUG_LEVEL 1

#if (DEBUG_LEVEL > 0)
    #define DEBUG_PRINTF(level, ...) \
        do {                         \
            if (level < DEBUG_LEVEL) \
            {                        \
                printf(__VA_ARGS__); \
            }                        \
        } while (0)
#else
    #define DEBUG_PRINTF(level, ...) \
        do { } while (0)
#endif

I found it odd that it suggested an empty “do / while(0)” and had to research this a bit. You may already know this, but since I had never seen it, I was unaware of the issue(s). Here is what Copilot says about this:

1. It forces the macro to behave like ONE statement
2. It makes the trailing semicolon safe
3. It prevents `if/else` breakage
4. It prevents partial execution
5. It compiles away to nothing when disabled

I could argue with the robot about #5 since I have certainly used compiler that where happy to leave in unused variables and functions without even a warning about them. ;-)

Of that list, #3 is really the only oneI think that applies to this specific debug macro. As Copilot points out, this example will not work with the non-do/while version of the macro:

if (flag)
    DEBUG_PRINTF(1, "msg");
else
    do_other_thing();

That would translate into this:

if (flag)
    if (level < DEBUG_LEVEL) \
    {                        \
        printf(__VA_ARGS__); \
    }
    ;
else
    do_other_thing();

And that won’t compile.

However, the coding standards where I have worked all forbid if/else usage like that. Without having curly braces around the statements, someone might later introduce a bug like this:

if (flag)
    DEBUG_PRINTF(1, "msg");
else
    do_other_thing();
    this_will_always_run();

…because without the braces, the compiler is basically seeing the code like this:

if (flag) DEBUG_PRINTF(1, "msg"); else do_other_thing();
this_will_always_run();

The curly braces ensure the code runs the statements as intended.

If you use that original “bad” macro with the same code but add braces:

if (flag)
{
    DEBUG_PRINTF(1, "msg");
}
else
{
    do_other_thing();
}

It looks like this, and works fine:

if (flag)
{        
    if (level < DEBUG_LEVEL) \
    {                        \
        printf(__VA_ARGS__); \
    }
    ;
}
else
{
    do_other_thing();
}

But, the addition of the “useless” do/while loop (or similar logic) around the macro would make it work with the non-curly brace version:

if (flag)
    do {                         \
        if (level < DEBUG_LEVEL) \
        {                        \
            printf(__VA_ARGS__); \
        }                        \
    } while (0)
    ;
else
    do_other_thing();

Try it yourself.

Here is a sample you can test in a web browser:

https://onlinegdb.com/KpTEGVsg7

Or, here is the code:

#include <stdbool.h>
#include <stdio.h>

#define DEBUG_LEVEL 1

#if (DEBUG_LEVEL > 0)
    #define DEBUG_PRINTF(level, ...) \
        do {                         \
            if (level < DEBUG_LEVEL) \
            {                        \
                printf(__VA_ARGS__); \
            }                        \
        } while (0)
#else
    #define DEBUG_PRINTF(level, ...) \
        do { } while (0)
#endif

void do_other_thing () { return; }

int main()
{
    bool flag = true;

    if (flag)
        DEBUG_PRINTF (0, "Hello\n");
    else
        do_other_thing();

    return 0;
}

Here’s where you come in…

What have you seen? I know modern PC/Linux/Mac environments don’t care about a few extra K of code space, so far fancier debug logging is likely common. But for embedded space, have you ran into the macros like I have, or do you always see the “do / while(0)” ones or something else?

Comment away…

Until next time…

I have worked with a variety of C compilers at my day jobs over the years. Although they try to be “real” compilers, they often have limitations that would drive “real” C programmers crazy.

Like, one tool I used did not allow constant strings longer than 80 characters. The error it gave was unhelpful, but eventually support was able to figure out what was going on.

Imagine my surprise to find that this would simply not build:

char *msg = "123456789012345678901234567890123456789012345678901234567890123456789012345678901";

…but removing one character allowed it to build just fine.

Another compiler did not allow constants to be passed as arguments to a function. You could not do this:

strcpy (buffer, "Hello"); // This wouldn't work.

…and had to do this instead:

char *temp = "Hello";
strcpy (buffer, temp); // This is the way.

And that was just the start of it. In that world, the “const” keyword was basically off-limits to use as you might be used to using it. A function that is not supposed to mess with a buffer passed in might look like this:

void function (const char *buffer_ptr, size_t buffer_size)
{
    // stuff...
}

…but since you cannot pass const data into functions on that compiler, that code will not compile.

And if you are trying to use code between a modern system (like a PC) and this compiler, removing “const” then has the modern compiler — with strict warnings turned on — will issue warnings if you pass a “constant string” into that function because it removes that “const” protection since it is a “char *” instead of a “const char *”.

My career is fun. Let’s use say I use “#ifdef” more than most C programmers ever have to…

If you don’t have to deal with this type of stuff, I hope you appreciate that. Embedded programmers have to do so much more work with so much less resources to get something done ;-)

Until next time…

…and there have been many like this.

I try to wrote gooder code, I really do. I am happy to own up to one of my own bugs, but when someone elses’ bug wastes hours or even days of my time, I am … not so happy.

And when “someone else” is whoever wrote the compiler I am using, and it created a bug that defies logic… well, that’s when I blog about it.

Consider this:

unsigned int type = 1;

printf ("Is %u == %u? ", p_tlv_table[table_entry].type, type);

if (p_tlv_table[table_entry].type == type)
{
    printf ("Yes.\r\n");
}
else
{
    printf ("No.\r\n");
}

The table is a structure that contains sets of Type-Length-Value numbers, but that is not important. What is important is that the debug output prints the following:

Is 1 == 1? No.

What if I told you that 1 == 1 is not true?

At the time of the printf, the value of “type” in the table at this entry is 1. It prints 1. It is 1.

And, at the time of the printf, the value of “type” is 1. It prints 1. It is 1.

Yet … the simple “if” fails, reporting that 1 does not equal 1.

This, my friends, is an S.C.T. – Strange Compiler Thing. (I just made that up, but I have been using S.W.T – Strange Window Thing – for way too many years.)

The values in the table are:

typedef struct
{
   uint8_t  type;
   uint8_t  length;
   uint16_t offset; // or uint8_t if not struct is > 255 bytes.
} tlv_offset_entry_t;

So “type” from that table entry is a U8, while “type” in the local variable is an unsigned int. But that should not matter. Test code like this in the same program works as you would expect:

unsigned int val1 = 1;
uint8_t      val2 = 1;

printf ("%u == %u is %d\r\n", val1, val2, (val1 == val2));

That will print 1, indicating that “val1 == val2” was true. They are equal.

So that is not the issue. I mean, you couldn’t do much if you can’t compare variables of different sizes like that.

And if I compare the same structure outside of the function it was passed into, the comparison works as expected.

A compiler quirk strikes again.

And when you can’t check “if (1 == 1)”, this is a good one ;-)

Welcome to my world. (Within minutes of me finally getting code down to be easily reproducible, the company is already working with me to identify why this happens. They rock.)

At my day job (I just made a new category for these posts) we have been working on an official coding standard to use for our software projects. When I was hired five years ago, I was given a three-page document about “Software Best Practices” that served as a casual guide to how code should be formatted and how functions and variables should be named. From looking at the million+ lines of code I maintain, it is clear that some items were adhered to, while others were ignored completely (thankfully; I disliked the suggested variable naming).

BARR-C focuses on embedded C programming and, unlike the coding standards I have seen at other jobs, it focuses on bug reduction rather than making the code pretty. I purchased a physical copy of the book, but you can download a PDF of it for free:

https://barrgroup.com/sites/default/files/barr_c_coding_standard_2018.pdf

While I consider myself quite stubborn or stuck in my ways, I can flip on a dime if presented compelling new information. The BARR-C standard is making me rethink a few things. Here is one example, which I share with you to get your take on it.

switch and case

// The way I have been using
switch (color)
{
    case RED:
        stop();
        break;

    case YELLOW:
        slow();
        break;

    case GREEN:
        speed();
        break;

    default:
        break;
}

I am very used to seeing the statements indented past the “case”. But, one of the editors I work with constantly moves those statements back to where they line up with the switch:

// The way one of my editors wants me to type it:
switch (color)
{
case RED:
    stop();
    break;

default:
    break;
}

I certainly don’t like the idea of code being at the same level of the braces. This seems like an odd default to me. Have you seen this elsewhere? ‘prolly has some specific name for this convention.

And recently, I ran into something new that has been added by a later C standard than any I have used. It does not allow variables to be declared inside the switch! That seemed odd, since–at some point–the C standard moved away from “all variables must be declared at the top of the function” to “yeah, wherever you want, it’s fine.”

void function ()
{
    int counter; // We used to have to do this only here.

    ...some lines later...
    
    while (active)
    {
        int counter = 0; // But now we can do it here.

        // ...stuff...
    }
}

Today, I prefer “variables used just in this bit of code” to be declared around that code so it is much easier to see what that variable is used for. Of course, this wouldn’t matter if every function was small enough to completely fit on the screen at the time. Sadly, I always seem to work with legacy functions that are hundreds of lines long.

But I digress…

There is something new (to me) that now disallows declaring variables in the case. I have seen this done (and still do it myself) for many years. To make it work, you need an extra set of curly braces which causes switch/cases that look like this:

switch (color)
{
    case RED:
    {
        int x; // Now works because braces.
        // ...stuff...
        break;
    }

    default:
        break;
}

I suppose this has advantages. It is making a scoped (is that the term?) variable just inside those braces of the case. Each case could make its own “x” and use it, if it wanted to, I suppose.

Side Note: Of course I had to try this out. Indeed, by default, this works without a peep from the compiler, but if you enable the proper warnings you will at least get “warning: declaration of ‘x’ shadows a previous local [-Wshadow]”.

int main()
{
    int x = 42;
    
    switch (x)
    {
        case 1:
        {
            int x = 1;
            printf ("x = %d\n", x);
            break;
        }
        
        case 2:
        {
            int x = 2;
            printf ("x = %d\n", x);
            break;
        }
        
        default:
            printf ("x = %d\n", x);
            break;
    }

    return 0;  
}

But that’s not important to this story… The BARR-C is giving me a new formatting option, and a reason why I might want to do it. It lines up the “case” and “break” together:

switch (err)

{

    case ERR_A:

        ...

    break;



    case ERR_B:

        ...

    // Also perform the steps for ERR_C.

    case ERR_C:

        ...

    break;



    default:

        ...

    break;

}

I have never encountered the case/default and its break lined up like that before. It looks odd to me, and feels wrong. But the reason for this is given:

Reasoning: C’s switch statements are powerful constructs, but prone to errors such as omitted break statements and unhandled cases. By aligning the case labels with their break statements it is easier to spot a missing break.

– Embedded C Coding Standard, Michael Barr

Interesting. I have, on a number of occasions (including again recently), found a bug where a break was missing, or something happened where it got backspaced to the line above it where it was now at the end of a comment:

    case GIVE_UP:
        // Give up and return.break;

    default:
        // Never surrender!!!
        break;

This is a trivial example, but if there had actually been lines of code there, you’d have to look at the last line of each case to verify a break was there. But if you line up the case/break like this…

    case GIVE_UP:
        // Give up and return.break;

    default:
        // Never surrender!!!
    break;

…you can immediately notice a problem where the “patterns don’t match,” which our brains seem to notice easier.

Using curly braces would not make a missing break stand out — in fact, it might make you assume it is all good because you see the closing brace there.

So I kinda like it.

Even if I hate it.

What say you? Comments if you got ’em.

Side Note 2: Since I originally typed this in, I have now fully converted to this formatting look. And, it has already helped me spot an issue like the one I mentioned earlier — without me having to scrounge line by line through the code trying to figure out what is going on. Nice.

Until next time…