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.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> > 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> 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 > http://www.reactos.org/mailman/listinfo/ros-dev > >
_______________________________________________ Ros-dev mailing list Ros-dev@reactos.org http://www.reactos.org/mailman/listinfo/ros-dev
