Bugs of the day, Tuesday 2013-03-26

One bug, and one better solution to a previous bug.

Local static state is local, static

int Foo() {
  static int timeout = kTimeoutLimit;
  bool polled_condition = CheckExternalCondition();
  if( !polled_condition )
    --timeout;
  if( timeout == 0 ) { 
    printf("condition not set before timeout");
    return kTaskFailed;
  }
  if( polled_condition )
    return kTaskComplete;
  else
    return kTaskIncomplete;
}
void Bar() { 
  while( Foo() == kTaskIncomplete );
}

Sometimes a local static variable is a useful thing — conveniently quick to add to any piece of code, and localized to a particular scope. You know everything about what can happen to it during its entire lifetime — assuming you know about how the scope it is contained within operates. (and I’m disregarding the returning of references to local static variables from a function. That’s a whole different bag of fun…)

Quickly rewriting some code today, I added a local static counter — counting down each time a desired condition was not met, and triggering a printf() should the count ever reach zero. I was using this to diagnose that something unexpected had happened, and that some event had not taken place.

The above code should work adequately well — if Bar() is only ever run once (and only from one thread at a time — but that’s not the issue here). timeout is never reset, so unless CheckExternalCondition() returns true, first time, every time, the value in timeout will continue to decrease with each run of Bar(). If it drops below zero, there will no longer be a limit on the on how long Foo() can loop.

A fix here is to make sure timeout is reset each time Foo() “completes” i.e. when it returns kTaskComplete or kTaskFailed.

More generally, this does not seem like a good use of a static local variable — relevant context for the function is not readily apparent within the function, so it seems better to store the timeout value outside of Foo().

(The actual code is more convoluted than the example above, so it’s not quite as simple as adding a local timeout var in Bar(), but only just)

Redux: Changed virtual function prototype results in correct function not being called

This was a bug where I changed the prototype for a virtual function, but that classes with the old prototype would not be detected at compile time and would silently do nothing should the virtual function be called. Read the details via the link above.

I had added an assert to catch any classes that didn’t have the new version of the function, but that only worked at runtime, and only when the function was actually called. What I really wanted for this was a compile-time check that was more reliable than the inheriting programmer remembering to use override. I discovered that there is a way to do this with C++11: final.

class Base {
public:
  virtual void Foo(int some_param, WhatsitType some_other_param)  {  }
private:
  virtual void Foo(int) final { }
};

With this, anything that inherits from Base and attempts to define its own void Foo(int) method will result in a compile error. I’m much happier with this, and making this change revealed some extra instances of the old function that I’d missed in my previous sweep. Huzzah.

Leave a Reply

Your email address will not be published.