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

[Alex Ionescu]

I would much rather see if (f != FALSE) instead of if (f).

Best regards,
Alex Ionescu

On Wed, Nov 12, 2014 at 10:03 PM, Love Nystrom <love.nystrom(a)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(a)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(a)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(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev
>
>