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,  <jgardou(a)svn.reactos.org> 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.…
>  ==============================================================================
>  --- 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 <ionucu(a)videotron.ca> wrote:
>   ???
>  This is retarded.
>  You're doing
>  0 ? 0 : 1
>  Best regards,
>  Alex Ionescu
>  On Sat, Jun 18, 2011 at 3:45 PM,  <jgardou(a)svn.reactos.org> 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.…
> >  ==============================================================================
> >  --- 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 Ionescu&lt;ionucu(a)videotron.ca&gt;  wrote:
> >   ???
> >  This is retarded.
> >  You're doing
> >  0 ? 0 : 1
> >  Best regards,
> >  Alex Ionescu
> >  On Sat, Jun 18, 2011 at 3:45 PM,&lt;jgardou(a)svn.reactos.org&gt;  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.…
> > > ==============================================================================
> > > --- 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
>  Ros-dev(a)reactos.org
>  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 <jerome.gardou(a)laposte.net>et>:
>   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 Ionescu&lt;ionucu(a)videotron.ca&gt;  wrote:
> > >  ???
> > >  This is retarded.
> > >  You're doing
> > >  0 ? 0 : 1
> > >  Best regards,
> > >  Alex Ionescu
> > >  On Sat, Jun 18, 2011 at 3:45 PM,&lt;jgardou(a)svn.reactos.org&gt;  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.…
> > > >
> > > > ==============================================================================
> > > > --- 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
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev 
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  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 Gardou<jerome.gardou(a)laposte.net>et>:
> >   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 Ionescu&lt;ionucu(a)videotron.ca&gt;    wrote:
> > > >   ???
> > > >  This is retarded.
> > > >  You're doing
> > > >  0 ? 0 : 1
> > > >  Best regards,
> > > >  Alex Ionescu
> > > >  On Sat, Jun 18, 2011 at 3:45 PM,&lt;jgardou(a)svn.reactos.org&gt;    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.…
> > > > >
> > > > > ==============================================================================
> > > > > --- 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
> > >  Ros-dev(a)reactos.org
> > >  http://www.reactos.org/mailman/listinfo/ros-dev 
> >  _______________________________________________
> >  Ros-dev mailing list
> >  Ros-dev(a)reactos.org
> >  http://www.reactos.org/mailman/listinfo/ros-dev
> >  
>  _______________________________________________
>  Ros-dev mailing list
>  Ros-dev(a)reactos.org
>  http://www.reactos.org/mailman/listinfo/ros-dev