[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 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>
> 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> 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
> http://www.reactos.org/mailman/listinfo/ros-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.reactos.org/pipermail/ros-dev/attachments/20141113/e7b9f4e1/attachment.html>