Re: [ros-dev] Replacing unsafe TRUE comparisons (Following Re: bool => BOOL)

[David Quintana (gigaherz)]

@Eric: Do a quick google search for "TROOLEAN", and you'll know why your
assumption that BOOL only has two possible values is wrong.

On 16 November 2014 11:19, Eric Kohl <eric.kohl(a)t-online.de> wrote:

>  
>  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(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>