16 Nov
2014
16 Nov
'14
10:19 a.m.
Hello Love,
I think you are trying to fix a bug at the wrong end. The boolean types
BOOL and BOOLEAN only have, by definition, two valid values: TRUE (aka
1) and FALSE (aka 0). Your 'other' trues (aka 2 to 2^32-1) are illegal
values which must NEVER be assigned to a boolean variable. Otherwise you
accept to break the checks for TRUE.
Developers MUST be VERY STRICT when assigning values to a boolean
variable. They HAVE to make sure that the assigned values are either
TRUE or FALSE. IMO other programmers have the right to rely on the fact
that a function, which returns a boolean value, only returns the values
TRUE or FALSE and NEVER returns any other value. Code like "if
(ReadFile(...) == TRUE)" is absolutely OK and MUST work as the
programmer expects!
Unfortunately, several month ago, some patches were applied to the Wine
code that replaced expressions like "(a < 7) ? TRUE : FALSE" by "(a
<
7)". These patches could be the origin of some bugs and should be
reverted from the Wine codebase.
Regards,
Eric
Am 12.11.2014 10:48, schrieb Love Nystrom:
Grep'ing for [ \t]*==[ \t]*TRUE and [ \t]*!=[
\t]*TRUE revealed some 400
matches..
That's *400 potential malfunctions begging to happen*, as previously
concluded.
If you *must*, for some obscure reason, code an explicit truth-value
comparison,
for God's sake make it (boolVal != FALSE) or (boolVal == FALSE), which
is safe,
because a BOOL has 2^32-2 TRUE values !!!
However, the more efficient "if ( boolVal )" and "if ( !boolVal )"
ought
to be *mandatory*.
I do hope nobody will challenge that "if ( boolVal )" equals "if (
boolVal != FALSE )",
and does *not* equal "if ( boolVal == TRUE )", when boolVal is BOOL or
BOOLEAN...
I've patched all those potential errors against the current trunk.
In most cases a simple removal of "== TRUE" was sufficient, however in
asserts I replaced it with "!= FALSE", since that may be clearer when
triggered.
The only places I let it pass was in pure debug strings and comments.
As this is a *fairly extensive patch*, I would very much appreciate if a
*prioritized regression test* could be run by you guys who do such things,
since this may actually fix some "mysterious" malfunctions, or introduce
bugs that did not trigger in my alpha test.
My own alpha test was limited to building and installing it on VMware
Player 6,
and concluding that "it appears to run without obvious malfunctions".
*Actually, when compared to a pre-patch build, one "mysterious" crash
disappeared!*
The patch has been submitted as bug CORE-8799, and is also included
inline in this post.
Best Regards
// Love