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

[Rafał Harabień]

W dniu 2014-11-16 o 11:19, Eric Kohl pisze:
>  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 
This is not true for logical operators:
C11(ISO/IEC 9899:201x) §6.5.8 /Relational operators/
    Each of the operators < (less than), > (greater than), <= (less than
    or equal to), and >= (greater than or equal to) shall yield 1 if the
    specified relation is true and 0 if it is false.107) The result has
    type int.
The problem exists if we assign value obtained from bit operators e.g.
"BOOLEAN HasFlag = a & SOME_FLAG"
Regards,
Rafał
>  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