While loop with empty body checking volatile ints - what does this mean?

Question

I am looking at a C++ class which has the following lines:

while( x > y );
return x - y;

x and y are member variables of type volatile int. I do not understand this construct.

I found the code stub here: https://gist.github.com/r-lyeh/cc50bbed16759a99a226. I guess it is not guaranteed to be correct or even work.

Answer 1

Since x and y have been declared as volatile, the programmer expects that they will be changed from outside the program.

In that case, your code will remain in the loop

 while(x>y);

and will return the value x-y after the values are changed from outside such that x <= y. The exact reason why this is written can be guessed after you tell us more about your code and where you saw it. The while loop in this case is a waiting technique for some other event to occur.

Answer 2

It seems

while( x > y );

is a spinning loop. It won't stop until x <= y. As x and y are volatile, they may be changed outside of this routine. So, once x <= y becomes true, x - y will be returned. This technique is used to wait for some event.

Update

According to the gist you added, it seems the idea was to implement thread-safe lock-free circular buffer. Yes, the implementation is incorrect. For example, the original code-snippet is

unsigned count() const {
    while( tail > head );
    return head - tail;
}

Even if tail becomes less or equal to head, it is not guaranteed that head - tail returns positive number. The scheduler may switch execution to another thread immediately after the while loop, and that thread may change head value. Anyway, there are a lot of other issues related to how reading and writing to shared memory work (memory reordering etc.), so just ignore this code.

Answer 3

Other replies have already pointed out in detail what the instruction does, but just to recap that, since y (or head in the linked example) is declared as volatile changes made to that variable from a different thread will cause the while loop to finish once the condition has been met.

However, even though the linked code example is very short, it's a near perfect example of how NOT to write code.

First of all the line

while( tail > head );

will waste enormous amounts of CPU cycles, pretty much locking up one core until the condition has been met.

The code gets even better as we go along.

buffer[head++ % N] = item;

Thanks to JAB for pointing out i mistook post- with pre-increment here. Corrected the implications. Since there are no locks or mutexes we obviously will have to assume the worst. The thread will switch after assigning the value in item and before head++ executes. Murphy will then call the function containing this statement again, assigning the value of item at the same head position. After that head increments. Now we switch back to the first thread and increment head again. So instead of

buffer[original_value_of_head+1] = item_from_thread1; 
buffer[original_value_of_head+2] = item_from_thread2;

we end up with

buffer[original_value_of_head+1] = item_from_thread2; 
buffer[original_value_of_head+2] = whatever_was_there_previously;

You might get away with sloppy coding like this on the client side with few threads, but on the server side this could only be considered a ticking time bomb. Please use synchronisation constructs such as locks or mutexes instead.

And well, just for the sake of completeness, the line

while( tail > head );

in the method pop_back() should be

while( tail >= head );

unless you want to be able to pop one more element than you actually pushed in (or even pop one element before pushing anything in).

Sorry for writing what basically boils down to a long rant, but if this keeps just one person from copying and pasting that obscene code it was worth while.

Update: Thought i might as well give an example where a code like while(x>y); actually makes perfect sense. Actually you used to see code like that fairly often in the "good old" days. cough DOS. Was´nt used in the context of threading though. Mainly as a fallback in case registering an interrupt hook was not possible (you kids might translate that as "not possible to register an event handler").

startAsynchronousDeviceOperation(..);

That might be pretty much anything, e.g. tell the hardisk to read data via DMA, or tell the soundcard to record via DMA, possibly even invoke functions on a different processor (like the GPU). Typically initiated via something like outb(2).

while(byteswritten==0); // or while (status!=DONE);

If the only communication channel with a device is shared memory, then so be it. Wouldnt expect to see code like that nowadays outside of device drivers and microcontrollers though. Obviously assumes the specs state that memory location is the last one written to.

Answer 4

The volatile keyword is designed to prevent certain optimisations. In this case, without the keyword, the compiler could unroll your while loop into a concrete sequence of instructions which will obviously break in reality since the values could be modified externally.

Imagine the following:

int i = 2;
while (i-- > 0) printf("%d", i);

Most compilers will look at this and simply generate two calls to printf - adding the volatile keyword will result in it instead generating the CPU instructions that invoke a counter set to 2 and a check on the value after every iteration.

For example,

volatile int i = 2;
this_function_runs_on_another_process_and_modifies_the_value(&i);
while(i-- > 0) printf("%d", i);