Re: [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(a)gmail.com 
>  <mailto:[e-mail]>> 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)
> >      &lt;gigaherz(a)gmail.com <mailto:[e-mail]>> 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
> >          &lt;love.nystrom(a)gmail.com <mailto:[e-mail]>> 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(a)reactos.org <mailto:[e-mail]>
>      http://www.reactos.org/mailman/listinfo/ros-dev
>
>