I've come across a subtle bug that could be in many places in the
sources. Basically, any comparrisons of booleans might be unsafe.
I first ran across it in sp_document_set_undo_sensitive in
document-undo.c. In it, the param 'sensitive' is of type gboolean. At
one point it is compared with doc->priv->sensitive. Ok, things seem fine
so far... however...
The specific types in question come to contribute to the problem. Since
the code is C and not C++, it uses gboolean instead of bool (of course,
since bool doesn't really exist in plain C). gboolean, in turn, lives in
glib's includes (a good place to get things from) and is typedef'd as gint.
That's where the problems come in. Since it's an int, not a true bool,
any non-zero value is taken as true, and any zero value is taken as
false. So far that seems OK, but let's take a quick look at some
0 == false
1 == true
-1 == true
$ffffffff == true
~0 == true
4 == true
Now, given all that, this next line of code clearly has some issues:
if (sensitive == doc->priv->sensitive) return;
Let's say that 'sensitive' is set to '~0' (all bits set. a nice safe
non-false value and often used for TRUE), and that doc->priv->sensitive
is set to 1. Now both integer values represent 'true', however they are
not actually equal to each other, and that check will fail even when
both variables are not false.
Therefore, anywhere two boolean-ish variables are compared for equality
or inequality they will not always evaluate properly. Additionally,
since these are just int values of one type or another, they could be
all over the codebase yet not noticed. (There's not something simple and
definitive that I can grep all of the code for, otherwise I would have
tried to find all of them myself).
So the solution would seem to be to have all eyes watching for potential
problem cases and then fixing them as they are encountered.
However, there is some good news. Doing a check of a single gboolean
variable is safe, since all non-zero values will be treated as true,
code will work as expected. It's only the case where two values are
compared that can have problems.
if ( sensitive ) // works
if ( !sensitive ) // works
if ( sensitive & foo ) // works
if ( sensitive == TRUE ) // broken
if ( sensitive != TRUE ) // broken
if ( sensitive == otherSens ) // broken
if ( sensitive != otherSens ) // broken
And even better news. There's a simple trick that's been pointed out to
me that can be used to make code safe. Since a logical inversion of the
value works, and will end up with a common value, just invert the
variables before checking.
if ( (!sensitive) == (!TRUE) ) // works
if ( (!sensitive) != (!TRUE) ) // works
if ( (!sensitive) == (!otherSens) ) // works
if ( (!sensitive) != (!otherSens) ) // works
(Oh, and since often it appears that there are some signed-unsigned
mismatches, and often values are compared to bitfields, accidental
mis-comparrisons have a higher probability of existing in the current