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

[Love Nystrom]

On 2014-11-14 00.41, Alex Ionescu wrote:
> I would much rather see if (f != FALSE) instead of if ( f ).

Well, it's certainly a valid option, and C++ compilers seem to generate
the same "CMP  boolRm, 0" in both cases (though the latter could actually
generate the potentially faster and better "OR  boolRm, boolRm" instead).

I expect some people will use the former.
Personally I much prefer the latter.

Best Regards
// Love


>
> On Wed, Nov 12, 2014 at 10:03 PM, Love Nystrom <love.nystrom at gmail.com 
> <mailto:love.nystrom at gmail.com>> wrote:
>
>     Thanks..
>     Well, someone had to do it, and I had a little time to spare. ;-)
>
>     I've attached 15 differentiated patches to this post, to ease review.
>     Please use these smaller patches *only for review*.
>
>     I have not added them to the bug tracker, because I strongly urge
>     that the "big" patch be applied system wide in this case.
>
>     Best Regards
>     // Love
>
>
>     On 2014-11-12 19.42, David Quintana (gigaherz) wrote:
>>     Nice work though.
>>
>>     On 12 November 2014 13:41, David Quintana (gigaherz)
>>     <gigaherz at gmail.com <mailto:gigaherz at gmail.com>> wrote:
>>
>>         It's a common practice to include giant code dumps as
>>         attachments instead of inlining them in the text of the mail.
>>         It may also be a good idea to provide multiple patches based
>>         on component, so that different people can take a look at the
>>         patches relating to their areas of expertise.
>>         If providing one patch per folder is too much work, then at
>>         least based on the top-level one. applications.patch,
>>         dll.patch, ntoskrnl.patch, etc. would be much easier to review.
>>
>>         On 12 November 2014 10:48, Love Nystrom
>>         <love.nystrom at gmail.com <mailto:love.nystrom at gmail.com>> wrote:
>>
>>             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
>>
>     [abbrev]
>
>
>     _______________________________________________
>     Ros-dev mailing list
>     Ros-dev at reactos.org <mailto:Ros-dev at reactos.org>
>     http://www.reactos.org/mailman/listinfo/ros-dev
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20141114/ee7ecd34/attachment.html>