Define variable before while loop or as static in loop

Question

I am programming a microcontroller (PIC18, 8 bit) in C99. I have this piece of code: a timeout, that returns early if triggered:

uint16_t timeout_x100us = 50000;

while (!is_interruptflag())                 // When this interrupt occurs, continue.
{
    if (!(--timeout_x100us))                // Decrement and check timer.
    {
        return;                             // Timeout ran out.
    }

    __delay_us(100);                        // Delay for 100µs.
}

I have two questions: (1) Is it good practice/does it make sense to define the variable timeout_x100us as static inside the loop to minimize its scope?

while (!is_interruptflag())                 // When this interrupt occurs, continue.
{
    static uint16_t timeout_x100us = 50000;

    if (!(--timeout_x100us))                // Decrement and check timer.
    {
        return;                             // Timeout ran out.
    }

    __delay_us(100);                        // Delay for 100µs.
}

(2) I am also trying to minimize program memory usage. The first code snippet uses 36 bytes of memory, whereas the second one (static) uses 54 bytes, which is significantly more (no compiler optimizations). Why does the static declaration use more program memory? Thank you.

Answer 1

(1) Is it good practice/does it make sense to define the variable timeout_x100us as static inside the loop to minimize its scope?

Generally it is good practice to reduce scope of variables. Regardless of that, you need to set it to a value in run-time before it is used.

However, for embedded systems specifically, it is bad practice to write code that relies on static storage initialization (of .data/.bss). Partially because a very long time could have passed between initialization and variable use, partially because it is common to use non-standard start-up code in the CRT that doesn't even initialize such variables.

What you should do is to keep timeout_x100us as a local variable with automatic storage, then initialize it to a value before executing the code, each time.

Also replace the magic number 50000 with a meaningful named #define.

Also keep in mind that the integer constant 50000 is of type 32 bit long on PIC, which is potentially a performance-killing bug. Replace it with 50000u to ensure 16 bit unsigned int instead.

You should be able to replace the code with something like this:

uint16_t i;
for(i=0; !is_interruptflag() && i<TIMEOUT; i++)
{
  __delay_us(100);
}

if(i==TIMEOUT)
{
  return ;
}

---

(2) I am also trying to minimize program memory usage. The first code snippet uses 36 bytes of memory, whereas the second one (static) uses 54 bytes

The first uses the stack instead, so it simply uses a different kind of memory. The extra code bloat could be related to some PIC Harvard architecture hiccup. Disassemble and investigate why.