Re: [ros-dev] [ros-diffs] [jgardou] 52339: [NTOSKRNL] - don't rely on a boolean being 1 or 0, it's a bad idea (tm)

[Alex Ionescu]

???

This is retarded.

You're doing

0 ? 0 : 1

Best regards,
Alex Ionescu

On Sat, Jun 18, 2011 at 3:45 PM,  [e-mail] wrote:
> Author: jgardou
> Date: Sat Jun 18 14:45:08 2011
> New Revision: 52339
>
> URL: http://svn.reactos.org/svn/reactos?rev=52339&view=rev
> Log:
> [NTOSKRNL]
>  - don't rely on a boolean being 1 or 0, it's a bad idea (tm)
>
> Modified:
>    trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
>
> Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c...
> ==============================================================================
> --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original)
> +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Jun 18 14:45:08 2011
> @@ -1510,7 +1510,7 @@
>
> /* Set thread fields */
>     Thread->TrapFrame = TrapFrame;
> Thread->PreviousMode = KiUserTrap(TrapFrame);
> Thread->PreviousMode = KiUserTrap(TrapFrame) ? UserMode : KernelMode;
>
> /* Enable interrupts */
>     _enable();

[Alex Ionescu]

Or rather

1 ? 1 : 0

(just as retarded).

Of course a BOOLEAN is 0 or 1!!!

Best regards,
Alex Ionescu

On Sat, Jun 18, 2011 at 4:22 PM, Alex Ionescu [e-mail] wrote:
> ???
>
> This is retarded.
>
> You're doing
>
> 0 ? 0 : 1
>
> Best regards,
> Alex Ionescu
>
> On Sat, Jun 18, 2011 at 3:45 PM,  [e-mail] wrote:
> > Author: jgardou
> > Date: Sat Jun 18 14:45:08 2011
> > New Revision: 52339
> >
> > URL: http://svn.reactos.org/svn/reactos?rev=52339&view=rev
> > Log:
> > [NTOSKRNL]
> >  - don't rely on a boolean being 1 or 0, it's a bad idea (tm)
> >
> > Modified:
> >    trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> >
> > Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c...
> > ==============================================================================
> > --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original)
> > +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Jun 18 14:45:08 2011
> > @@ -1510,7 +1510,7 @@
> >
> > /* Set thread fields */
> >     Thread->TrapFrame = TrapFrame;
> > Thread->PreviousMode = KiUserTrap(TrapFrame);
> > Thread->PreviousMode = KiUserTrap(TrapFrame) ? UserMode : KernelMode;
> >
> > /* Enable interrupts */
> >     _enable();

[Jérôme Gardou]

Well, just try this snippet of code in VS 2010, and tell me what you see 
as result :-)
BOOLEAN is what it is : a typedef to BYTE.
PS : no fancy optimization option or whatever

// test_boolean.cpp : définit le point d'entrée pour l'application console.
//

#include <iostream>
#include <Windows.h>
#include <tchar.h>

using namespace std;

BOOLEAN test0x10(int i)
{
     return i & 0x10;
}

int _tmain(int argc, _TCHAR* argv[])
{
     if(test0x10(0x10) == 1)
         cout << "Alex Ionescu is right" << endl;
     else if(test0x10(0x10))
         cout << "Sometimes Alex Ionescu is wrong" << endl;
     else
         cout << "Unexpected behaviour!!!" << endl;

return 0;
}

Le 18/06/2011 17:22, Alex Ionescu a écrit :
> Or rather
>
> 1 ? 1 : 0
>
> (just as retarded).
>
> Of course a BOOLEAN is 0 or 1!!!
>
> Best regards,
> Alex Ionescu
>
> On Sat, Jun 18, 2011 at 4:22 PM, Alex [e-mail]  wrote:
> > ???
> >
> > This is retarded.
> >
> > You're doing
> >
> > 0 ? 0 : 1
> >
> > Best regards,
> > Alex Ionescu
> >
> > On Sat, Jun 18, 2011 at 3:45 PM,[e-mail]  wrote:
> > > Author: jgardou
> > > Date: Sat Jun 18 14:45:08 2011
> > > New Revision: 52339
> > >
> > > URL: http://svn.reactos.org/svn/reactos?rev=52339&view=rev
> > > Log:
> > > [NTOSKRNL]
> > > don't rely on a boolean being 1 or 0, it's a bad idea (tm)
> > >
> > > Modified:
> > >     trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > >
> > > Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > > URL: http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c...
> > > ==============================================================================
> > > --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original)
> > > +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Jun 18 14:45:08 2011
> > > @@ -1510,7 +1510,7 @@
> > >  /* Set thread fields */
> > >  Thread->TrapFrame = TrapFrame;
> > > Thread->PreviousMode = KiUserTrap(TrapFrame);
> > >
> > > Thread->PreviousMode = KiUserTrap(TrapFrame) ? UserMode : KernelMode;
> > >
> > > /* Enable interrupts */
> > > _enable();
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Alex Ionescu]

This is a completely different issue. You're confusing bitmask math
with BOOLEAN.

Best regards,
Alex Ionescu

2011/6/19 Jérôme Gardou [e-mail]:
> Well, just try this snippet of code in VS 2010, and tell me what you see as
> result :-)
> BOOLEAN is what it is : a typedef to BYTE.
> PS : no fancy optimization option or whatever
>
> // test_boolean.cpp : définit le point d'entrée pour l'application console.
> //
>
> #include <iostream>
> #include <Windows.h>
> #include <tchar.h>
>
> using namespace std;
>
> BOOLEAN test0x10(int i)
> {
>    return i & 0x10;
> }
>
> int _tmain(int argc, _TCHAR* argv[])
> {
>    if(test0x10(0x10) == 1)
>        cout << "Alex Ionescu is right" << endl;
>    else if(test0x10(0x10))
>        cout << "Sometimes Alex Ionescu is wrong" << endl;
>    else
>        cout << "Unexpected behaviour!!!" << endl;
>
> return 0;
> }
>
> Le 18/06/2011 17:22, Alex Ionescu a écrit :
> > Or rather
> >
> > 1 ? 1 : 0
> >
> > (just as retarded).
> >
> > Of course a BOOLEAN is 0 or 1!!!
> >
> > Best regards,
> > Alex Ionescu
> >
> > On Sat, Jun 18, 2011 at 4:22 PM, Alex [e-mail]  wrote:
> > > ???
> > >
> > > This is retarded.
> > >
> > > You're doing
> > >
> > > 0 ? 0 : 1
> > >
> > > Best regards,
> > > Alex Ionescu
> > >
> > > On Sat, Jun 18, 2011 at 3:45 PM,[e-mail]  wrote:
> > > > Author: jgardou
> > > > Date: Sat Jun 18 14:45:08 2011
> > > > New Revision: 52339
> > > >
> > > > URL: http://svn.reactos.org/svn/reactos?rev=52339&view=rev
> > > > Log:
> > > > [NTOSKRNL]
> > > >  - don't rely on a boolean being 1 or 0, it's a bad idea (tm)
> > > >
> > > > Modified:
> > > >    trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > > >
> > > > Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > > > URL:
> > > > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c...
> > > >
> > > > ==============================================================================
> > > > --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original)
> > > > +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Jun 18
> > > > 14:45:08 2011
> > > > @@ -1510,7 +1510,7 @@
> > > >
> > > > /* Set thread fields */
> > > >     Thread->TrapFrame = TrapFrame;
> > > > Thread->PreviousMode = KiUserTrap(TrapFrame);
> > > > Thread->PreviousMode = KiUserTrap(TrapFrame) ? UserMode :
> > > >
> > > > KernelMode;
> > > >
> > > > /* Enable interrupts */
> > > >     _enable();
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev

[Jérôme Gardou]

I'm not confonding anything, I just consider that calling a function 
which returns a BOOLEAN expecting it will return 0 or 1 is a bad idea, 
because someone might have written the exact same function I've shown 
you as an example.
As a general rule, consider something true when it's not false :-).

Le 19/06/2011 15:29, Alex Ionescu a écrit :
> This is a completely different issue. You're confusing bitmask math
> with BOOLEAN.
>
> Best regards,
> Alex Ionescu
>
> 2011/6/19 Jérôme [e-mail]:
> > Well, just try this snippet of code in VS 2010, and tell me what you see as
> > result :-)
> > BOOLEAN is what it is : a typedef to BYTE.
> > PS : no fancy optimization option or whatever
> >
> > // test_boolean.cpp : définit le point d'entrée pour l'application console.
> > //
> >
> > #include<iostream>
> > #include<Windows.h>
> > #include<tchar.h>
> >
> > using namespace std;
> >
> > BOOLEAN test0x10(int i)
> > {
> >     return i&  0x10;
> > }
> >
> > int _tmain(int argc, _TCHAR* argv[])
> > {
> >     if(test0x10(0x10) == 1)
> >         cout<<  "Alex Ionescu is right"<<  endl;
> >     else if(test0x10(0x10))
> >         cout<<  "Sometimes Alex Ionescu is wrong"<<  endl;
> >     else
> >         cout<<  "Unexpected behaviour!!!"<<  endl;
> > return 0;
> >
> > }
> >
> > Le 18/06/2011 17:22, Alex Ionescu a écrit :
> > > Or rather
> > >
> > > 1 ? 1 : 0
> > >
> > > (just as retarded).
> > >
> > > Of course a BOOLEAN is 0 or 1!!!
> > >
> > > Best regards,
> > > Alex Ionescu
> > >
> > > On Sat, Jun 18, 2011 at 4:22 PM, Alex [e-mail]    wrote:
> > > > ???
> > > >
> > > > This is retarded.
> > > >
> > > > You're doing
> > > >
> > > > 0 ? 0 : 1
> > > >
> > > > Best regards,
> > > > Alex Ionescu
> > > >
> > > > On Sat, Jun 18, 2011 at 3:45 PM,[e-mail]    wrote:
> > > > > Author: jgardou
> > > > > Date: Sat Jun 18 14:45:08 2011
> > > > > New Revision: 52339
> > > > >
> > > > > URL: http://svn.reactos.org/svn/reactos?rev=52339&view=rev
> > > > > Log:
> > > > > [NTOSKRNL]
> > > > > don't rely on a boolean being 1 or 0, it's a bad idea (tm)
> > > > >
> > > > > Modified:
> > > > >     trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > > > >
> > > > > Modified: trunk/reactos/ntoskrnl/ke/i386/traphdlr.c
> > > > > URL:
> > > > > http://svn.reactos.org/svn/reactos/trunk/reactos/ntoskrnl/ke/i386/traphdlr.c...
> > > > >
> > > > > ==============================================================================
> > > > > --- trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] (original)
> > > > > +++ trunk/reactos/ntoskrnl/ke/i386/traphdlr.c [iso-8859-1] Sat Jun 18
> > > > > 14:45:08 2011
> > > > > @@ -1510,7 +1510,7 @@
> > > > >  /* Set thread fields */
> > > > >  Thread->TrapFrame = TrapFrame;
> > > > > Thread->PreviousMode = KiUserTrap(TrapFrame);
> > > > > Thread->PreviousMode = KiUserTrap(TrapFrame) ? UserMode :
> > > > >
> > > > > KernelMode;
> > > > >  /* Enable interrupts */
> > > > >  _enable();
> > >
> > > Ros-dev mailing list
> > > [e-mail]
> > > http://www.reactos.org/mailman/listinfo/ros-dev
> >
> > Ros-dev mailing list
> > [e-mail]
> > http://www.reactos.org/mailman/listinfo/ros-dev
>
> Ros-dev mailing list
> [e-mail]
> http://www.reactos.org/mailman/listinfo/ros-dev