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

_______________________________________________
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev