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.nyst...@gmail.com <mailto:love.nyst...@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) <gigah...@gmail.com <mailto:gigah...@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.nyst...@gmail.com <mailto:love.nyst...@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@reactos.org <mailto:Ros-dev@reactos.org> http://www.reactos.org/mailman/listinfo/ros-dev
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
