Oliver Smith stumbled across a small but surprising bug when running some automated tests against a mostly clean code-base. Specifically, they were trying to break things by playing around with different compiler flags and settings. And they did, though in a surprising case.

bool long_name_that_maybe_distracted_someone()
{
  return (execute() ? CONDITION_SUCCESS : CONDITION_FAILURE);
}

Note the return type of the method is boolean. Note that execute must also return boolean. So once again, we’ve got a ternary which exists to turn a boolean value into a boolean value, which we’ve seen so often it’s barely worth mentioning. But there’s an extra, hidden assumption in this code- specifically that CONDITION_SUCCESS and CONDITION_FAILURE are actually defined to be true or false.

CONDITION_SUCCESS was in fact #defined to be 1. CONDITION_FAILURE, on the other hand, was #defined to be 2.

Worse, while long_name_that_maybe_distracted_someone is itself a wrapper around execute, it was in turn called by another wrapper, which also mapped true/false to CONDITION_SUCCESS/CONDITION_FAILURE. Oddly, however, that method itself always returned CONDITION_SUCCESS, even if there was some sort of failure.

This code had been sitting in the system for years, unnoticed, until Oliver and company had started trying to find the problem areas in their codebase.

[Advertisement] Utilize BuildMaster to release your software with confidence, at the pace your business demands. Download today!