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

[Mario J. Rugiero]

Although they don't include the exact case of argument, these slides 
show a very similar example of how it's mostly useless to try to 
outsmart the compiler in such simple optimizations. The most similar 
case, IMO, is the bounds check. Check them out.


http://www.linux-kongress.org/2009/slides/compiler_survey_felix_von_leitner.pdf

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Love Nystrom]

On 2014-11-22 18.47, Timo Kreuzer wrote:

Am 22.11.2014 11:39, schrieb Love Nystrom:


On 2014-11-21 04.00, Timo Kreuzer wrote:

Am 20.11.2014 14:18, schrieb Love Nystrom:


Well... Actually not exactly the same.. ;)
"if (f != FALSE)" requires an explicit comparison with a second 
operand,
No, it does not. It requries the compiler to generate code that 
executes the following statement, when f is not 0. 


I suspect we look at it from two different viewpoints here..
Yours is "C centric" and mine is "object code centric".
You talk about what the compiler is required to do,
and I talk about what comes out at the end of compilation.
You claimed '"if (f != FALSE)" requires an explicit comparison with a 
second operand,' 


The "if (f != FALSE)" instruction has *two operands*, variable f and the 
immediate 0.

The "if ( f )" instruction has *one operand*, variable f.
However, according to you two plus one is four.

I'm outta here..
Perhaps I'll swing by in a few years and see if you've matured a bit, Mr 
Grand Master.


Tada

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Daniel Reimer]

Heheh, who could it be... ^^
It was always a event to watch you two arguing. ^^


Am 23.11.2014 21:57 schrieb Alex Ionescu :
>
> I'm just going to chime in here and confirm that Timo does indeed own a 
> master's diploma on surviving pissing contests, taught by the greatest master 
> there ever was.
>
> Best regards,
> Alex Ionescu
>
> On Sat, Nov 22, 2014 at 11:47 AM, Timo Kreuzer  wrote:
>>
>> Am 22.11.2014 11:39, schrieb Love Nystrom:
>>>
>>>
>>> On 2014-11-21 04.00, Timo Kreuzer wrote:

 Am 20.11.2014 14:18, schrieb Love Nystrom:
>
>
> Well... Actually not exactly the same.. ;)
> "if (f != FALSE)" requires an explicit comparison with a second operand,

 No, it does not. It requries the compiler to generate code that executes 
 the following statement, when f is not 0.
>>>
>>>
>>> I suspect we look at it from two different viewpoints here..
>>> Yours is "C centric" and mine is "object code centric".
>>> You talk about what the compiler is required to do, 
>>> and I talk about what comes out at the end of compilation.
>>
>> And what comes out at the end of the compilation is what the compiler 
>> creates. And the compiler is following the rules of the C standard and the 
>> rules of logic.
>> You claimed '"if (f != FALSE)" requires an explicit comparison with a second 
>> operand,' and that is factually wrong. No matter whether you are looking at 
>> it from the compiler perspective or from the perspective of an expressionist 
>> painter living in a yellow tree house on the bottom of Lake Tanganyika.
>>
>>>
>>> And.. dear friend.. don't turn this into a pissing contest.
>>
>> Don't even get me started. I battled the grand master and I survived.
>>
>>
>>> Let's check the egos in with the coat check girl at the entrance.
>>> May I ask how old you are?
>>
>> Are we talking about age or maturity?
>>
>> We better end this discussion, it's not leading anywhere. And you don't want 
>> me to turn into the Grinch and steal your Christmas.
>>
>> Thanks,
>> Timo
>>
>>
>> ___
>> 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

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

[Alex Ionescu]

I'm just going to chime in here and confirm that Timo does indeed own a
master's diploma on surviving pissing contests, taught by the greatest
master there ever was.

Best regards,
Alex Ionescu

On Sat, Nov 22, 2014 at 11:47 AM, Timo Kreuzer  wrote:

>  Am 22.11.2014 11:39, schrieb Love Nystrom:
>
>
> On 2014-11-21 04.00, Timo Kreuzer wrote:
>
> Am 20.11.2014 14:18, schrieb Love Nystrom:
>
>
> Well... Actually not exactly the same.. ;)
> "if (f != FALSE)" requires an explicit comparison with a second operand,
>
> No, it does not. It requries the compiler to generate code that executes
> the following statement, when f is not 0.
>
>
> I suspect we look at it from two different viewpoints here..
> Yours is "C centric" and mine is "object code centric".
> You talk about what the compiler is required to do,
> and I talk about what comes out at the end of compilation.
>
> And what comes out at the end of the compilation is what the compiler
> creates. And the compiler is following the rules of the C standard and the
> rules of logic.
> You claimed '"if (f != FALSE)" requires an explicit comparison with a
> second operand,' and that is factually wrong. No matter whether you are
> looking at it from the compiler perspective or from the perspective of an
> expressionist painter living in a yellow tree house on the bottom of Lake
> Tanganyika.
>
>
> And.. dear friend.. don't turn this into a pissing contest.
>
> Don't even get me started. I battled the grand master and I survived.
>
>
>  Let's check the egos in with the coat check girl at the entrance.
> May I ask how old you are?
>
> Are we talking about age or maturity?
>
> We better end this discussion, it's not leading anywhere. And you don't
> want me to turn into the Grinch and steal your Christmas.
>
> Thanks,
> Timo
>
>
> ___
> 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

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

[Timo Kreuzer]

Am 22.11.2014 11:39, schrieb Love Nystrom:


On 2014-11-21 04.00, Timo Kreuzer wrote:

Am 20.11.2014 14:18, schrieb Love Nystrom:


Well... Actually not exactly the same.. ;)
"if (f != FALSE)" requires an explicit comparison with a second operand,
No, it does not. It requries the compiler to generate code that 
executes the following statement, when f is not 0. 


I suspect we look at it from two different viewpoints here..
Yours is "C centric" and mine is "object code centric".
You talk about what the compiler is required to do,
and I talk about what comes out at the end of compilation.
And what comes out at the end of the compilation is what the compiler 
creates. And the compiler is following the rules of the C standard and 
the rules of logic.
You claimed '"if (f != FALSE)" requires an explicit comparison with a 
second operand,' and that is factually wrong. No matter whether you are 
looking at it from the compiler perspective or from the perspective of 
an expressionist painter living in a yellow tree house on the bottom of 
Lake Tanganyika.




And.. dear friend.. don't turn this into a pissing contest.

Don't even get me started. I battled the grand master and I survived.



Let's check the egos in with the coat check girl at the entrance.
May I ask how old you are?

Are we talking about age or maturity?

We better end this discussion, it's not leading anywhere. And you don't 
want me to turn into the Grinch and steal your Christmas.


Thanks,
Timo

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Love Nystrom]

On 2014-11-21 04.00, Timo Kreuzer wrote:

Am 20.11.2014 14:18, schrieb Love Nystrom:


Well... Actually not exactly the same.. ;)
"if (f != FALSE)" requires an explicit comparison with a second operand,
No, it does not. It requries the compiler to generate code that 
executes the following statement, when f is not 0. 


I suspect we look at it from two different viewpoints here..
Yours is "C centric" and mine is "object code centric".
You talk about what the compiler is required to do,
and I talk about what comes out at the end of compilation.

And.. dear friend.. don't turn this into a pissing contest.
Let's check the egos in with the coat check girl at the entrance.
May I ask how old you are?

Best Regards
// Love

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Timo Kreuzer]

Am 20.11.2014 14:18, schrieb Love Nystrom:


Well... Actually not exactly the same.. ;)
"if (f != FALSE)" requires an explicit comparison with a second operand,
No, it does not. It requries the compiler to generate code that executes 
the following statement, when f is not 0. It could even use crazy 
contructs of cdq and sbb and other things.
And a sane compiler does not distinguish between 2 things that are 
semantically 100% identical, unless it's not capable of recognizing that 
it's identical. You can be sure that every compiler knows that if (a) 
and if (a != 0) are the same. The proof that these terms are identical 
is left as an exercise for the reader.
Most compilers are not completely retarded. For example when you write 
"if (x >= 23 && x <= 42) return 1; else return 0;" the compiler will 
usually optimize this into a single unsigned compare and a subtraction, 
like sub ecx, 23; cmp ecx, 9; setbe al;



___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Mario J. Rugiero]

Most modern compilers are wise enough to realize you are comparing with 
zero and just do what you said.


___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Love Nystrom]

On 2014-11-16 05.22, Timo Kreuzer wrote:

Am 14.11.2014 15:32, schrieb 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
[abbrev]
"if (f != FALSE)" and "if (f)" are exactly 100% the same for the 
compiler. "if (f)" is nothing but a synonym for "if (f != 0)" and 
FALSE is 0. 


Well... Actually not exactly the same.. ;)
"if (f != FALSE)" requires an explicit comparison with a second operand,
whereas "if ( f )" can be performed by comparing the value *with itself* 
using an OR instruction.
You'll see that a lot in the C string library (I think you know this), 
where it's used to check quickly

for the terminating NUL without having to compare with a second operand.

Best Regards
// Love

Whether it creates a CMP, or an OR or a TEST is all up to the compiler 
and it will be the same in both cases. If it was not the same, there 
is something ... well not neccessarily "wrong" but at least very 
strange with the compiler.




I expect some people will use the former.
Personally I much prefer the latter.

I think both have their pros and cons. So I don't really care.




___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[Timo Kreuzer]

Am 16.11.2014 11:19, schrieb Eric Kohl:

Hello Love,

I think you are trying to fix a bug at the wrong end. The boolean types
BOOL and BOOLEAN only have, by definition, two valid values: TRUE (aka
1) and FALSE (aka 0). Your 'other' trues (aka 2 to 2^32-1) are illegal
values which must NEVER be assigned to a boolean variable. Otherwise you
accept to break the checks for TRUE.
The problem with this strict definition is, that we cannot use it, since 
it does not match with MS' definition.

MSDN says about BOOL and BOOLEAN:
"A Boolean variable (*should* be TRUE or FALSE)." Should is not must.
Most win32 APIs that return BOOL are described in MSDN with something like:
"If the function succeeds, the return value is *nonzero*."
And there are Windows APIs that return BOOL with values other than 0 and 1.
The defensive approach of avoiding direct comparison to TRUE should be 
the best.
Quoting Raymond Chen: "Checking for equality against TRUE is asking for 
trouble."



Developers MUST be VERY STRICT when assigning values to a boolean
variable. They HAVE to make sure that the assigned values are either
TRUE or FALSE. IMO other programmers have the right to rely on the fact
that a function, which returns a boolean value, only returns the values
TRUE or FALSE and NEVER returns any other value. Code like "if
(ReadFile(...) == TRUE)" is absolutely OK and MUST work as the
programmer expects!
That might be ok for ReadFile, where MSDN explicitly says it returns 
TRUE on success.

But it's not necessarily ok for other APIs.



___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[David Quintana (gigaherz)]

@Eric: Do a quick google search for "TROOLEAN", and you'll know why your
assumption that BOOL only has two possible values is wrong.

On 16 November 2014 11:19, Eric Kohl  wrote:

>
> Hello Love,
>
> I think you are trying to fix a bug at the wrong end. The boolean types
> BOOL and BOOLEAN only have, by definition, two valid values: TRUE (aka
> 1) and FALSE (aka 0). Your 'other' trues (aka 2 to 2^32-1) are illegal
> values which must NEVER be assigned to a boolean variable. Otherwise you
> accept to break the checks for TRUE.
>
> Developers MUST be VERY STRICT when assigning values to a boolean
> variable. They HAVE to make sure that the assigned values are either
> TRUE or FALSE. IMO other programmers have the right to rely on the fact
> that a function, which returns a boolean value, only returns the values
> TRUE or FALSE and NEVER returns any other value. Code like "if
> (ReadFile(...) == TRUE)" is absolutely OK and MUST work as the
> programmer expects!
>
> Unfortunately, several month ago, some patches were applied to the Wine
> code that replaced expressions like "(a < 7) ? TRUE : FALSE" by "(a <
> 7)". These patches could be the origin of some bugs and should be
> reverted from the Wine codebase.
>
>
> Regards,
> Eric
>
>
> Am 12.11.2014 10:48, schrieb Love Nystrom:
> > 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
>
> ___
> 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

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

[Rafał Harabień]

W dniu 2014-11-16 o 11:19, Eric Kohl pisze:
>
> Unfortunately, several month ago, some patches were applied to the Wine
> code that replaced expressions like "(a < 7) ? TRUE : FALSE" by "(a <
> 7)". These patches could be the origin of some bugs and should be
> reverted from the Wine codebase.
>
>
> Regards,
> Eric

This is not true for logical operators:

C11(ISO/IEC 9899:201x) §6.5.8 /Relational operators/

Each of the operators < (less than), > (greater than), <= (less than
or equal to), and >= (greater than or equal to) shall yield 1 if the
specified relation is true and 0 if it is false.107) The result has
type int.

The problem exists if we assign value obtained from bit operators e.g.
"BOOLEAN HasFlag = a & SOME_FLAG"

Regards,
Rafał
>
> Am 12.11.2014 10:48, schrieb Love Nystrom:
>> 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
> ___
> 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

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

[Eric Kohl]

Hello Love,

I think you are trying to fix a bug at the wrong end. The boolean types
BOOL and BOOLEAN only have, by definition, two valid values: TRUE (aka
1) and FALSE (aka 0). Your 'other' trues (aka 2 to 2^32-1) are illegal
values which must NEVER be assigned to a boolean variable. Otherwise you
accept to break the checks for TRUE.

Developers MUST be VERY STRICT when assigning values to a boolean
variable. They HAVE to make sure that the assigned values are either
TRUE or FALSE. IMO other programmers have the right to rely on the fact
that a function, which returns a boolean value, only returns the values
TRUE or FALSE and NEVER returns any other value. Code like "if
(ReadFile(...) == TRUE)" is absolutely OK and MUST work as the
programmer expects!

Unfortunately, several month ago, some patches were applied to the Wine
code that replaced expressions like "(a < 7) ? TRUE : FALSE" by "(a <
7)". These patches could be the origin of some bugs and should be
reverted from the Wine codebase.


Regards,
Eric


Am 12.11.2014 10:48, schrieb Love Nystrom:
> 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

___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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

[David Quintana (gigaherz)]

To me it's a matter of readability/semantics.

Even though a BOOL is still an integer, if a variable is clearly meant for
the purpose of a bool, such as "if (bEnableThis)", then using != FALSE is
redundant.

If the variable doesn't have an implicit "boolean-ness" in its name, then
it may be best to write != FALSE to clarify, although in such case it may
be a better choice to rename the variable itself rather than use the
verbose comparison.

For pointers, it's a similar matter: I understand "if (ptrSomething)" to
mean != NULL. But for numbers of any type, be it int, float, or otherwise,
I strongly prefer using != 0.

It may sound silly, but I prefer it that way because in my head "if
(myNumber)" reads as "if there is a number in this variable", and of course
there is a number, it just happens to be 0, which is still a number. So
although I understand the way it works, it just feels better for me to
compare with != 0 and make my intention clear.

On 15 November 2014 23:22, Timo Kreuzer  wrote:

>  Am 14.11.2014 15:32, schrieb 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).
>
> "if (f != FALSE)" and "if (f)" are exactly 100% the same for the compiler.
> "if (f)" is nothing but a synonym for "if (f != 0)" and FALSE is 0. Whether
> it creates a CMP, or an OR or a TEST is all up to the compiler and it will
> be the same in both cases. If it was not the same, there is something ...
> well not neccessarily "wrong" but at least very strange with the compiler.
>
>
> I expect some people will use the former.
> Personally I much prefer the latter.
>
> I think both have their pros and cons. So I don't really care.
>
>
>
> ___
> 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

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

[Timo Kreuzer]

Am 14.11.2014 15:32, schrieb 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).
"if (f != FALSE)" and "if (f)" are exactly 100% the same for the 
compiler. "if (f)" is nothing but a synonym for "if (f != 0)" and FALSE 
is 0. Whether it creates a CMP, or an OR or a TEST is all up to the 
compiler and it will be the same in both cases. If it was not the same, 
there is something ... well not neccessarily "wrong" but at least very 
strange with the compiler.




I expect some people will use the former.
Personally I much prefer the latter.

I think both have their pros and cons. So I don't really care.


___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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 > 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)
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
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 
http://www.reactos.org/mailman/listinfo/ros-dev




___
Ros-dev mailing list
Ros-dev@reactos.org
http://www.reactos.org/mailman/listinfo/ros-dev

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 
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) 
> 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  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

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

[Love Nystrom]

On 2014-11-12 19.41, David Quintana (gigaherz) 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.


Yes, I know..
But since each of these patches are exceedingly trivial, strongly 
inter-related,

exposes a common bad habit in programming, and this touches practically
every part of the system, I chose to make one big patch this time.

Had the patches been more involved, or fewer,
I would of course have posted them in chunks.

Best Regards
// Love



On 12 November 2014 10:48, Love Nystrom > 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