embedded software boot camp

All variables are equal, but some are more equal than others

Tuesday, July 31st, 2012 by Nigel Jones

With all due apologies to George Orwell for the title, I thought I’d offer a little tidbit on the practice of the following construct:

uint8_t a,b,c,d;
a = b = c = d = 0;

This code declares four variables (a,b,c,d) and sets them all equal to 0. The question is, is this a good, bad or indifferent practice? Well, I think it is an excellent practice in one very limited case, but otherwise should be avoided. Consider these two examples:

#define MAX_SPD (42U)
void fna(void)
{
uint8_t spd1 = MAX_SPD;
uint8_t spd2 = MAX_SPD;
...
}
void fnb(void)
{
uint8_t spd1, spd2;
spd1 = spd2 = MAX_SPD;
...
}

What difference, if any, is there between these two functions? Well clearly they both declare two variables and assign them the value MAX_SPD. However, I would suggest that there is a very subtle difference. In fna() there are two variables that happen to have the same initialized value, whereas in fnb() there are two variables that are initialized to the same value, which happens to be MAX_SPD. So what you ask? Well consider someone maintaining this code. For fna() all they know is that the two variables happen to be initialized to the same value, and thus a change such as this is perhaps a reasonable thing to do:

void fna(void)
{
uint8_t spd1 = MAX_SPD;
uint8_t spd2 = MAX_SPD - 1;
...
}

Conversely, for fnb() there is a barrier to and a subtle hint against making spd1 different from spd2. Thus if the algorithm requires that spd1 and spd2 always be initialized to the same value then the construct in fnb() is better. Conversely, if it is essentially happenstance that spd1 and spd2 share the same initial value then fna() is better.

To put it another way, if you find yourself using this construct to save on typing or lines of code then the chances are you are doing the wrong thing. Conversely if you find yourself doing this to impart a subtle hint then the chances are you are doing the right thing.

A comment on comments. One can (and should) argue that in the case of fnb() one should have a comment to the effect that spd1 and spd2 must be initialized to the same value. While I agree wholeheartedly, I always try and use code constructs that minimize reliance on someone actually reading the comments.

As a final thought. I have seem coding standards that ban this practice. If your coding standard does ban it, perhaps its time to revisit it?

 

 

16 Responses to “All variables are equal, but some are more equal than others”

  1. Chris says:

    That’s not valid syntax for defining several variables, is it?

    gcc agrees with me:

    assign.c: In function ‘foo’:
    assign.c:6: error: ‘b’ undeclared (first use in this function)
    assign.c:6: error: (Each undeclared identifier is reported only once
    assign.c:6: error: for each function it appears in.)
    assign.c:6: error: ‘c’ undeclared (first use in this function)
    assign.c:6: error: ‘d’ undeclared (first use in this function)

  2. Daniele Romano says:

    I tried to compile (with no optimizations at all) the following code with Keil uVision (targeting an ARM Cortex M3-based MCU):

    void contextual_init(void)
    {
    int a1, a2;
    int b1, b2;
    a1 = 9;
    a2 = 9;
    b1 = b2 = 8;
    }

    The disassembly of the object code thus produced is the following:

    contextual_init
    PUSH {r4,lr}
    MOVS r0,#9
    MOVS r1,#9
    MOVS r4,#8
    MOV r3,r4
    MOV r2,r4
    POP {r4,pc}

    …thus using two MOVS instructions for the distinct, separate initializing code for a1 and a2
    variables, while using three MOVS instructions for the inline initializing construct, just to
    store the literal in a separate register and then store it back to the destination registers
    allocated for b1 and b2 variables. Maybe I’ve been a little bit pesky, but the produced code
    could worths a glance.

  3. The two ways of doing things are different not just because of the syntax. Firstly, fna() uses initialisation but fnb() uses (multiple) assignment, though I doubt that this would affect the compiled code. Secondly, the expression

    b1 = b2= 8;

    is not the same, semantically, as

    b1= 8;
    b2= 8;

    The former performs the assignment b2= 8 but assigns the result of this sub-expression ( (b2= 8), not simply the value 8 ) to b1. This explains the unoptimised compiled code. I would be mildly surprised if turning on optimisation didn’t “fix” this but I’m too lazy to try!

  4. I wrote ” simply the value 8″ but it combined with the following “)” to produce a smiley! Sorry about that, and I hope nothing similar happens to this post.

  5. Richard Hendricks says:

    I would never rely upon such a subtle hint as a requirement/allowance. You assume a coder with the same background as yourself. Don’t. Assume the next maintainer will be someone with less experience in C and/or embedded and/or a different culture. Hoping that a subtle hint that a=b=MAX_SPD will cross those boundaries is dooming that poor person to a roll of the bug dice (now there’s a thinkgeek product I could buy, dice that look like bugs, with different error codes on each side).

    Make a comment, link it to your problem tracking tool where you explain in full why you did what you did, who you talked to, and who agreed it was the right idea.

    spd1 = spd2 = MAX_SPD; //spd1 and spd2 must start at same speed, see BUG-123 for details

  6. david collier says:

    Hmmm. IMHO all discussion so far has missed the real point.

    To impart the subtle hint, what we need to do is:

    #define MAX_SPD (42U)

    void fnc(void)
    {
    uint8_t spd1, spd2;
    #define INIT_SPD MAX_SPD
    spd1 = spd2 = INIT_SPD;

    }

  7. david collier says:

    I am always wary of initialised variables.

    It took me a while to find out what was wrong with this

    for ( i=0; i < 3; i++)
    {
    int z = 99;

    printf( "%d\n" , z )

    z = 27;
    }

    What should be printed?

    GCC gives 99 99 99, but I swear I once saw a compiler do 99 27 27 – could have been codevisionAVR.
    And of course if someone comes along and finds a reasonj to add a 'static' then that's exactly what you get.
    So the =99 LOOKS like executable code but sometimes it isn't executed where it's written.
    For which reason I prefer to split the definition from the initialisation , so all is crystal clear.

    • Dave L says:

      [Sorry to revive an old thread, but I did find the article on the front page…]

      What you probably saw was
      for ( i=0; i < 3; i++)
      {
      static int z = 99; //<– Note change to static variable here
      printf( "%d\n" , z )
      z = 27;
      }

      This would print 99 27 27, because (in this context… ugh) "static" means the variable is only initialized once.

      It is almost _always_ a good idea to initialize variables to a known value; using an uninitialized variable is a major problem, and one that might lurk in waiting until a very inopportune time.

  8. Lundin says:

    Bad idea. Spot the bug in this code:

    uint8_t* ptr1, ptr2;
    ptr1 = ptr2 = malloc(1);

    Now spot the bug in this code:

    uint8_t* ptr1;
    uint8_t ptr2;
    ptr1 = malloc(1);
    ptr2 = malloc(1);

Leave a Reply

You must be logged in to post a comment.