Dark Corners of C - The Comma Operator in Embedded

www.spiroprojects.com


if (!dry_run && ((stdout_closed = true), close_stream (stdout) != 0))
The comma operator is a fundamental operator in C that doesn't get nearly the exposure that other, more often used operators such as '=' or '++' get. Its purpose is to join together expressions into one line in a very specific way: the expression to the left of the comma is first evaluated and the result is ignored, then the expression to the right is evaluated and the result is returned. 

At first glance it doesn't seem to be that useful of an operator - who needs to put two expressions on the same line and ignore the outcome of one of them? Judging from the lack of common knowledge of the comma operator it seems most people get along fine without it, so what is a good use case for it?
The Wikipedia article has a few rather good use cases for it that you should read. The upshot is that it can be helpful in grouping expressions together in code to improve the readability of the code. In my opinion this is a slight advantage that introduces its own disadvantages. I'll illustrate with an example.
While I don't remember when I first saw the comma operator for the first time, it came back into the forefront of my mind when I was working on writing a circular buffer library for an upcoming article. Circular buffers involve reading and writing lots of memory which means lots of loops. A typical memory copy (for a linear buffer) looks something like this:

void memcpy(void * dest, void * src, size_t len)
{
    int i;
    for(i=0;i<len;i++)
        dest[i] = src[i];
}


This function relies on the fact that the source buffer is linear. The next memory address is always right after the current one : all you need to do to get there is increment. A circular buffer doesn't work the same way because it's topology is (obviously) a circle: the source pointer can start off anywhere within the buffer and will wrap back to the start of the buffer when it reaches the end. This makes it more complicated to write a memory copy out of a circular buffer - it might look something like this:

void cbRead(cbRef buffer, void * dest, size_t len)
{
    int i;
    for(i=0;i<len;i++)
    {
        dest[i] = buffer.data[buffer.srcPtr++]
        if(buffer.srcPtr == buffer.length-1)
            buffer.srcPtr = 0;
    }
}

(Note that this code isn't complete by any means - at the very least there's no check to ensure there's enough data in the buffer). 
In the circular buffer case, the buffer maintains a source pointer that indicates what element inside of its data array has the first byte of data. This pointer has to be wrapped back around to the beginning of the buffer by the if  statement to respect the circular nature of the buffer. 
This implementation looks clunkier than the linear buffer memory copy and with that conditional inside the loop, I'd guess it's slower as well. You can replace the conditional statement with something much quicker if you are willing to live by the following restrictions:
1.    Every buffer has to be the same, fixed size
2.    That size is a power of 2 minus one (example: 31 is 2^5 -1)
These restrictions allow you to use bit masking to replace the if statement:
void cbRead(cbRef buffer, void * dest, size_t len)
{
    int i;
    for(i=0;i<len;i++)
    {
        dest[i] = buffer.data[buffer.srcPtr++]
        buffer.srcPtr &=0x1F;    //buffer size of 31
    }
}

Replacing the if statement with a bit mask is much faster - especially on most microcontrollers, but it still looks less sleek than the original memory copy, doesn't it? This is where I thought the comma operator could "help" - by allowing me to write the same functionality like this:

void cbCopy(cbRef buffer, void * dest, size_t len)
{
    int i=0,endPtr;
    for(endPtr=(buffer.srcPtr+len)&0x1F;srcPtr!=endPtr;buffer.srcPtr=buffer.srcPtr++,buffer.srcPtr&0x1F)
    {
        dest[i++] = buffer.data[buffer.srcPtr]
    }
}

If you squint your eyes a bit and ignore most things that are happening in the for loop, this does indeed look sleek: there's only a single line inside the for loop and it looks like a memory copy. What's happening in the rest of it?
  • I changed the for loop from one that loops on from 0 to len to one that calculates an endPtr and then loops until the srcPtr is equal to it. This allows me to rewrite the for loop:
  • The initializer generates the endPtr.
  • The condition for terminating the loop is that the srcPtr is equal to the endPtr - i.e., we've copied all of the data we were asked to.
The increment is where the magic is. It (theoretically) uses the comma operator to join together two expressions: the first to increment the source pointer and the second to perform a bitmask on the pointer to wrap it back to 0 when it exceeds the length of the buffer. The comma operator should discard the result of the first increment and then return the value of the source pointer after it's been masked. 
I love being clever as much as the next guy and while I can admire clever, sleek pieces of code this just isn't one them. I can't recommend that you write code this way solely for the sake of using the comma operator for two three reasons:
1.    The comma operator is simply much less readable than the alternatives. Most people will be immediately confused by it and will have to fire up Google to see what's going on. This distraction makes it harder to read your code.
2.    It's rarely (if ever) necessary. There's always some combination of other, more readable statements that will give you the same effect.
3.    This code won't even work the way I expected it to: due to operator precedence, the 'b++' is evaluated first and the value returned, and the result of the mask is ignored.
My 'inventive' and 'clever' use of the comma operator changed a fairly straightforwad loop into a confusing mess that doesn't even fit into this article (on my browser anyhow) and wouldn't work if I ever coded it that way. Luckily, I took one look at that monstrosity and decided it wasn't worth it. Judging from a number of comments I've seen while researching this article, that seems to be the general consensus agreed to by most people (including the drafters of the MISRA C standard, which simply forbids the use of the operator). 
Still, I don't feel right having that be the final word. Yes, in this circumstance (and very likely, in most circumstances) using the comma operator produced code that was less readable than the alternative. That doesn't mean it's useless - it only means that we haven't found the appropriate use for it yet. In fact, you'll find a few practical, non-academic examples where the comma operator produces cleaner, more readable code. The answers to thisStackoverflow question have a few good examples:
So don't let the general opinion against the comma operator sway you. As with most things, programming skill improves when the programmer is challenged - regardless of whether the programmer passes the challenge or not! In this case, the challenge was to rewrite a memory copy loop using the comma operator (and maintain readability). I'd argue I failed this particular challenge but in the process improved my skills.  So I encourage you to challenge yourself: use the comma operator somewhere - anywhere! If it works out, great! If not, you'l have learned something - also great!  
UPDATE
Originally, I created an imaginary example using the comma operator:
a=b++,b>>2;

With the assumption that 'b++' would be executed and the result ignored and the outcome of 'b>>2' would be assigned to 'a' I thought the equivalent code to the above would be this:
b++;
a=b>>2;
This turned out not to be the case, but thankfully Old Wolf corrected me here and on reddit. Due to operator precedence rules in C he claimed, the equivalent code would actually be this:
a = b++;
b >> 2;
This would not have the intended effect at all. 
Now, I immediately believed Old Wolf because he said I was wrong - if there's anyone I don't trust it's me. However, sometimes I am in fact wrong about being wrong, so to determine which kind of wrong I was this time, I wrote this program (comma.c) to test the matter:
#include <stdio.h>
int main(void)
{
    int a=0;
    int b=7;
    a=b++,b>>2;
    printf("a is %d\r\n",a);
    printf("b is %d\r\n",b);
    return 0;
}
If I were correct I would expect a to be 2 and b to be 8. If Old Wolf were right, a will be 7 and b would be 8. So, which is it?

C:\Users\Stephen\Documents>gcc comma.c
C:\Users\Stephen\Documents>a.exe
a is 7
b is 8

I knew I should never have trusted me. My hat's off to Old Wolf. Apparently, an Old Wolf can teach a dog like me new tricks!
It's also worth noting that because of this error on my part, my circular buffer code wouldn't work correctly either. Luckily, that abomination is mainly meant to show that if you try to be too clever when writing code you'll end up making a mess of everything. Given that I tried to be clever and ended up making a mess out of everything, I think this article stands as sufficient proof by demonstration that you should stick with simplicity and clarity over clever tricks.

Previous
Next Post »