[RFC][PATCH] A generic boolean

[RFC][PATCH] A generic boolean

[ricknu-0]

From: Richard Knutsson

A first step to a generic boolean-type. The patch just introduce the bool (in
arch. i386 only (for the moment)), false and true + fixing some duplications in
the code.
It is diffed against Linus' git-tree (060718)
Compiled with my own-, allyesconfig- and allmodconfig-config.h-file.

-Why would we want it?
-There is already some how are depending on a "boolean"-type (like NTFS). Also,
it will clearify functions who returns a boolean from one returning a value, ex:
bool it_is_ok();
char it_is_ok();
The first one is obvious what it is doing, the secound might return some sort of
status.

-Why false and not FALSE, why not "enum {...} bool"
-They are not #define(d) and shouldn't because it is a value, like 'a'. But
because it is just a value, then bool is just a variable and should be able to
handle 0 and 1 equally well.

Well, this is _my_ opinion, it may be totally wrong. If so, please tell me ;)

If this takes off, I guess I will spend quite some time at kernel-janitors
"cleaning" those who use a boolean-type.

Til' next time...
/Richard Knutsson

PS
Yes, I know about Andrew's try to unify TRUE and FALSE, did read the thread with
interest (that's from where I got to know about _Bool). But mostly (then still
on the subject) was some people did not want FALSE and TRUE instead of 0 and 1.
I look at it as: 'a' = 97, if someone like to write 97 instead of 'a', please do
if you find it easier to read. I, on the other hand, think it is easier with
'a', false/FALSE, NULL, etc.
DS

PPS
One thing about _Bool thue:
_Bool a = 12; results in a = 1

test( char * t ) { t = 12; }
main() {
_Bool a;
test( (char *) &a ); results in a = 12.
}

But I do not think of it as a problem since a "true" is just !false. Doing:
if (boolvar == true)
seems odd, after all...

... and sorry for the longwinded letter :)
DDS

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/asm-i386/types.h          |    9 +++++++++
 include/linux/stddef.h            |    2 ++
 6 files changed, 13 insertions(+), 10 deletions(-)



diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
index 4b4b295..e35709a 100644
--- a/include/asm-i386/types.h
+++ b/include/asm-i386/types.h
@@ -10,6 +10,15 @@ typedef unsigned short umode_t;
  * header files exported to user space
  */
 
+#if defined(__GNUC__) && __GNUC__ >= 3
+typedef _Bool bool;
+#else
+#warning You compiler doesn't seem to support boolean types, will set 'bool' as
an 'unsigned char'
+typedef unsigned char bool;
+#endif
+
+typedef bool u2;
+
 typedef __signed__ char __s8;
 typedef unsigned char __u8;
 
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..5e5c611 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,8 @@ #else
 #define NULL ((void *)0)
 #endif
 
+enum { false = 0, true = 1 } __attribute__((packed));
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)

Re: [RFC][PATCH] A generic boolean

[Jeff Garzik]

ricknu-0@student.ltu.se wrote:
> A first step to a generic boolean-type. The patch just introduce the bool (in

Since gcc supports boolean types and can optimize for such, introducing 
bool is IMO a good thing.


> -Why would we want it?
> -There is already some how are depending on a "boolean"-type (like NTFS). Also,
> it will clearify functions who returns a boolean from one returning a value, ex:
> bool it_is_ok();
> char it_is_ok();
> The first one is obvious what it is doing, the secound might return some sort of
> status.

A better reason is that there is intrinsic compiler support for booleans.


> -Why false and not FALSE, why not "enum {...} bool"
> -They are not #define(d) and shouldn't because it is a value, like 'a'. But
> because it is just a value, then bool is just a variable and should be able to
> handle 0 and 1 equally well.
> 
> Well, this is _my_ opinion, it may be totally wrong. If so, please tell me ;)

> Yes, I know about Andrew's try to unify TRUE and FALSE, did read the thread with
> interest (that's from where I got to know about _Bool). But mostly (then still
> on the subject) was some people did not want FALSE and TRUE instead of 0 and 1.
> I look at it as: 'a' = 97, if someone like to write 97 instead of 'a', please do
> if you find it easier to read. I, on the other hand, think it is easier with
> 'a', false/FALSE, NULL, etc.

We should follow what C99 directs.


> diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> index 4b4b295..e35709a 100644
> --- a/include/asm-i386/types.h
> +++ b/include/asm-i386/types.h
> @@ -10,6 +10,15 @@ typedef unsigned short umode_t;
>   * header files exported to user space
>   */
>  
> +#if defined(__GNUC__) && __GNUC__ >= 3
> +typedef _Bool bool;
> +#else
> +#warning You compiler doesn't seem to support boolean types, will set 'bool' as
> an 'unsigned char'
> +typedef unsigned char bool;
> +#endif
> +
> +typedef bool u2;

NAK.  gcc >= 3 is required by now, AFAIK.

Also, you don't want to force 'unsigned char' on code, because often 
code prefers a machine integer to something smaller than a machine integer.


> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index b3a2cad..5e5c611 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -10,6 +10,8 @@ #else
>  #define NULL ((void *)0)
>  #endif
>  
> +enum { false = 0, true = 1 } __attribute__((packed));

How is 'packed' attribute useful here?

	Jeff

Re: [RFC][PATCH] A generic boolean

[Alexey Dobriyan]

On Wed, Jul 19, 2006 at 10:38:20PM +0200, ricknu-0@student.ltu.se wrote:
> A first step to a generic boolean-type. The patch just introduce the bool (in
> arch. i386 only (for the moment)),

What's do special about i386?

> false and true + fixing some duplications in
> the code.

> -Why would we want it?
> -There is already some how are depending on a "boolean"-type (like NTFS). Also,
> it will clearify functions who returns a boolean from one returning a value, ex:
> bool it_is_ok();
> char it_is_ok();
> The first one is obvious what it is doing, the secound might return some sort of
> status.

It should be obvious from name whether function returns int which is a
boolean or int which is a number.

> -Why false and not FALSE, why not "enum {...} bool"
> -They are not #define(d) and shouldn't because it is a value, like 'a'. But
> because it is just a value, then bool is just a variable and should be able to
> handle 0 and 1 equally well.

  -Why we wouldn't want it
  -C++ and Java fans will treat bool as a green light to the following

	if (!(flags == true))
  and
	if (!(flags == false))

> If this takes off, I guess I will spend quite some time at kernel-janitors
> "cleaning" those who use a boolean-type.

> Yes, I know about Andrew's try to unify TRUE and FALSE, did read the thread with
> interest (that's from where I got to know about _Bool). But mostly (then still
> on the subject) was some people did not want FALSE and TRUE instead of 0 and 1.
> I look at it as: 'a' = 97, if someone like to write 97 instead of 'a', please do
> if you find it easier to read. I, on the other hand, think it is easier with
> 'a', false/FALSE, NULL, etc.
> DS
> 
> PPS
> One thing about _Bool thue:
> _Bool a = 12; results in a = 1
> 
> test( char * t ) { t = 12; }
		    ^^^
> main() {
> _Bool a;
> test( (char *) &a ); results in a = 12.
> }
> 
> But I do not think of it as a problem since a "true" is just !false. Doing:
> if (boolvar == true)
> seems odd, after all...
> 
> ... and sorry for the longwinded letter :)
>

Please, show compiler flag[s] to enable warning[s] from gcc about

	_Bool foo = 42;

Until you do that the whole activity is moot.

> --- a/drivers/net/dgrs.c
> +++ b/drivers/net/dgrs.c
> @@ -110,7 +110,6 @@ static char version[] __initdata =
>   *	DGRS include files
>   */
>  typedef unsigned char uchar;
> -typedef unsigned int bool;
>  #define vol volatile
>  
>  #include "dgrs.h"

The only chunk that looks OK to me.

> --- a/include/asm-i386/types.h
> +++ b/include/asm-i386/types.h
> @@ -10,6 +10,15 @@ typedef unsigned short umode_t;
>   * header files exported to user space
>   */
>
> +#if defined(__GNUC__) && __GNUC__ >= 3
> +typedef _Bool bool;
> +#else
> +#warning You compiler doesn't seem to support boolean types, will set 'bool' as
> an 'unsigned char'
> +typedef unsigned char bool;

Why unsigned char? Why not unsigned int? What would this do wrt
bitfields?

> +#endif
> +
> +typedef bool u2;

What is it?

Re: [RFC][PATCH] A generic boolean

[ricknu-0]

Citerar Alexey Dobriyan <adobriyan@gmail.com>:

> On Wed, Jul 19, 2006 at 10:38:20PM +0200, ricknu-0@student.ltu.se wrote:
> > A first step to a generic boolean-type. The patch just introduce the bool
> (in
> > arch. i386 only (for the moment)),
> 
> What's do special about i386?
Oh, nothing. Meant the patch is only for i386. Because I do not have any other
setup there were little reason to change for any more arches

> > -Why would we want it?
> > -There is already some how are depending on a "boolean"-type (like NTFS).
> Also,
> > it will clearify functions who returns a boolean from one returning a
> value, ex:
> > bool it_is_ok();
> > char it_is_ok();
> > The first one is obvious what it is doing, the secound might return some
> sort of
> > status.
> 
> It should be obvious from name whether function returns int which is a
> boolean or int which is a number.
Yes idealy, but sometimes a "obvious" name for someone is a uncertain for others
+ if it is suppose to be an boolean, why not decleare it as one. Have seen quite
a few: int a; /* boolean */

> > -Why false and not FALSE, why not "enum {...} bool"
> > -They are not #define(d) and shouldn't because it is a value, like 'a'.
> But
> > because it is just a value, then bool is just a variable and should be able
> to
> > handle 0 and 1 equally well.
> 
>   -Why we wouldn't want it
>   -C++ and Java fans will treat bool as a green light to the following
> 
> 	if (!(flags == true))
>   and
> 	if (!(flags == false))
Thank god (or someone) for all the C fans who codereview ;)

Have you actually seen code like that (please point me to the place in that case :)


> Please, show compiler flag[s] to enable warning[s] from gcc about
> 
> 	_Bool foo = 42;
> 
> Until you do that the whole activity is moot.
On it...


> > --- a/include/asm-i386/types.h
> > +++ b/include/asm-i386/types.h
> > @@ -10,6 +10,15 @@ typedef unsigned short umode_t;
> >   * header files exported to user space
> >   */
> >
> > +#if defined(__GNUC__) && __GNUC__ >= 3
> > +typedef _Bool bool;
> > +#else
> > +#warning You compiler doesn't seem to support boolean types, will set
> 'bool' as
> > an 'unsigned char'
> > +typedef unsigned char bool;
> 
> Why unsigned char? Why not unsigned int? What would this do wrt
> bitfields?
I just took the smallest alternetive to a bit. Many uses an unsigned char as
boolean, others who uses integer should not suffer either (none of whom I have
checked).


> > +#endif
> > +
> > +typedef bool u2;
> 
> What is it?

Oww, it should be u1 (unsigned 1-bit). Thanks!

Re: [RFC][PATCH] A generic boolean

[ricknu-0]

Citerar Jeff Garzik <jeff@garzik.org>:

> ricknu-0@student.ltu.se wrote:
> > A first step to a generic boolean-type. The patch just introduce the bool
> (in
> 
> Since gcc supports boolean types and can optimize for such, introducing 
> bool is IMO a good thing.
Good to hear :)

> > -Why would we want it?
> > -There is already some how are depending on a "boolean"-type (like NTFS).
> 
> A better reason is that there is intrinsic compiler support for booleans.
Yeah, true.


> > -Why false and not FALSE, why not "enum {...} bool"
> > -They are not #define(d) and shouldn't because it is a value, like 'a'.
> But
> > because it is just a value, then bool is just a variable and should be able
> to
> > handle 0 and 1 equally well.
> > 
> > Well, this is _my_ opinion, it may be totally wrong. If so, please tell me
> ;)
> 
> > Yes, I know about Andrew's try to unify TRUE and FALSE, did read the thread
> with
> > interest (that's from where I got to know about _Bool). But mostly (then
> still
> > on the subject) was some people did not want FALSE and TRUE instead of 0
> and 1.
> > I look at it as: 'a' = 97, if someone like to write 97 instead of 'a',
> please do
> > if you find it easier to read. I, on the other hand, think it is easier
> with
> > 'a', false/FALSE, NULL, etc.
> 
> We should follow what C99 directs.
Yes. But I can not say I know what you are refering to. The enum vs #define,
false vs FALSE or both. May you please point me to appropriate text.


> > diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> > index 4b4b295..e35709a 100644
> > --- a/include/asm-i386/types.h
> > +++ b/include/asm-i386/types.h
> > @@ -10,6 +10,15 @@ typedef unsigned short umode_t;
> >   * header files exported to user space
> >   */
> >  
> > +#if defined(__GNUC__) && __GNUC__ >= 3
> > +typedef _Bool bool;
> > +#else
> > +#warning You compiler doesn't seem to support boolean types, will set
> 'bool' as
> > an 'unsigned char'
> > +typedef unsigned char bool;
> > +#endif
> > +
> > +typedef bool u2;
> 
> NAK.  gcc >= 3 is required by now, AFAIK.
Thanks, I forgot to remove it


> Also, you don't want to force 'unsigned char' on code, because often 
> code prefers a machine integer to something smaller than a machine integer.
But isn't a bit smaller than a byte? Sorry, do not understand what you mean.


> > diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> > index b3a2cad..5e5c611 100644
> > --- a/include/linux/stddef.h
> > +++ b/include/linux/stddef.h
> > @@ -10,6 +10,8 @@ #else
> >  #define NULL ((void *)0)
> >  #endif
> >  
> > +enum { false = 0, true = 1 } __attribute__((packed));
> 
> How is 'packed' attribute useful here?
Oh, nothing really. Added without thinking, nice catch.

Re: [RFC][PATCH] A generic boolean

[Peter Williams]

ricknu-0@student.ltu.se wrote:
> Citerar Alexey Dobriyan <adobriyan@gmail.com>:
>> Please, show compiler flag[s] to enable warning[s] from gcc about
>>
>> 	_Bool foo = 42;
>>
>> Until you do that the whole activity is moot.
> On it...

Would not the compiler treat 42 as a Boolean expression (as opposed to 
an integer expression) that evaluates to true and set foo accordingly. 
I.e. there's only a problem here if foo ends up with the value 42 
instead of the value true.

Peter
-- 
Peter Williams                                   pwil3058@bigpond.net.au

"Learning, n. The kind of ignorance distinguishing the studious."
  -- Ambrose Bierce

Re: [RFC][PATCH] A generic boolean

[ricknu-0]

Citerar Peter Williams <pwil3058@bigpond.net.au>:

> ricknu-0@student.ltu.se wrote:
> > Citerar Alexey Dobriyan <adobriyan@gmail.com>:
> >> Please, show compiler flag[s] to enable warning[s] from gcc about
> >>
> >> 	_Bool foo = 42;
> >>
> >> Until you do that the whole activity is moot.
> > On it...
> 
> Would not the compiler treat 42 as a Boolean expression (as opposed to 
> an integer expression) that evaluates to true and set foo accordingly. 
> I.e. there's only a problem here if foo ends up with the value 42 
> instead of the value true.
Yeah, that is true. As I said, the only way (I seen) is to cast the pointer from
the boolean variable to something else and change the value on that address. But
it would be neat if the compiler said something when inserting somthing else
then (or equal to) 0 and 1.
Right now it is happy with:
_Bool a = "Hello world";


Well, better go to bed while I still remember where it is.

Good night

Re: [RFC][PATCH] A generic boolean

[Jeff Garzik]

ricknu-0@student.ltu.se wrote:
> Citerar Jeff Garzik <jeff@garzik.org>:
>> Also, you don't want to force 'unsigned char' on code, because often 
>> code prefers a machine integer to something smaller than a machine integer.

> But isn't a bit smaller than a byte? Sorry, do not understand what you mean.

For all processors, it is generally preferred to have integer operations 
performed on a "machine integer."  A machine integer is the natural data 
type of the processor.  If it's a 32-bit processor, the natural data 
type for the ALU is a 32-bit int.  If it's a 64-bit processor, the 
natural data type for the ALU is a 64-bit int.  [though, for some 64-bit 
processors, a 32-bit int may be best for the situation anyway]

As such, the compiler and/or CPU must do more work, if an operation such 
as a bit test is performed on a data type other than a machine int.

Consider for example ARM or Alpha architectures, which may not have 
instructions 8-bit unsigned char integers.  The integers have to be 
_converted_ to a machine integer, before the operation is performed.

It is for this reason that you often see boolean implemented as 'int' 
rather than 'unsigned char'.  'int' is much more "natural", when you 
consider all the architectures Linux must support.

The best solution is to typedef 'bool' to the compiler's internal 
boolean data type, and then update code to use 'bool'.  Then all these 
issues magically go away, because you never have to care what the 
compiler's underlying boolean data type is.

	Jeff

Re: [RFC][PATCH] A generic boolean

[Vadim Lobanov]

On Wed, 19 Jul 2006, Jeff Garzik wrote:

> ricknu-0@student.ltu.se wrote:
> > Citerar Jeff Garzik <jeff@garzik.org>:
> >> Also, you don't want to force 'unsigned char' on code, because often
> >> code prefers a machine integer to something smaller than a machine integer.
>
> > But isn't a bit smaller than a byte? Sorry, do not understand what you mean.
>
> For all processors, it is generally preferred to have integer operations
> performed on a "machine integer."  A machine integer is the natural data
> type of the processor.  If it's a 32-bit processor, the natural data
> type for the ALU is a 32-bit int.  If it's a 64-bit processor, the
> natural data type for the ALU is a 64-bit int.

If this is the case, then wouldn't "long" be preferable to "int"?

> 	Jeff

-- Vadim Lobanov

RE: [RFC][PATCH] A generic boolean

[Shorty Porty]

Someone showed code like

  _Bool foo = 42;

and if we were to make the compiler warn about it that would mean we are
basically trying to change/manipulate the standard (I'm guessing). It's
probably not in the standard because it's such a moot point. However if we
were to use

  if(foo)
  {
    ...
  }

we'd see it was true. That's because 
  FALSE == 0
and
  TRUE == !FALSE (i.e. any value that isn't 0)

from the compiler's standpoint. Function that return 'true' for an integer
type (as opposed to a C++ standard-type bool) should be tested like

  if(SomeFunction())

or
  if(!SomeFunction())

instead of testing for equality

  if(SomeFunction() == TRUE)
of
  if(SomeFunction() == FALSE)

as the former (IMO) is as readable, if not more readable as the latter, and
it's likely to get optimised better. That and someone might give true AND
return a status by returning neither 0 or 1, in which case 

  if(... == TRUE)

would fail, as TRUE == 1.

And just as a note, you really should read the documentation (at least once)
for any function you use, and therefore know if it returns {FALSE, TRUE, ...
, TRUE} or {OK, ERR1, ERR2, ..., ERRn}

> > If this is the case, then wouldn't "long" be preferable to "int"?

Meh, it's all the same. I don't think 3 wasted CPU cycles is going to worry
anyone too much. Hell, sometimes int IS long, though I might be wrong there.


P.S. First post! Hello everybody.

Re: [RFC][PATCH] A generic boolean

[Dmitry Torokhov]

On Wednesday 19 July 2006 23:53, Shorty Porty wrote:
> > > If this is the case, then wouldn't "long" be preferable to "int"?
> 
> Meh, it's all the same. I don't think 3 wasted CPU cycles is going to worry
> anyone too much. Hell, sometimes int IS long, though I might be wrong there.
> 

It is the _kernel_. In hot codepaths even 1 cycle matters.

-- 
Dmitry

RE: [RFC][PATCH] A generic boolean

[Jan Engelhardt]

>[...]
>from the compiler's standpoint. Function that return 'true' for an integer
>type (as opposed to a C++ standard-type bool) should be tested like
>
>  if(SomeFunction())
>
>or
>  if(!SomeFunction())
>
>instead of testing for equality
>
>  if(SomeFunction() == TRUE)
>of
>  if(SomeFunction() == FALSE)
>
>as the former (IMO) is as readable, if not more readable as the latter

Full ACK. What currently bugs me is that there are code places which 
"violate" your suggestion in a different way, i.e.

 void *foo = null_or_not_null_that_is_the_question();
 ...
 if(foo)
      do_bar();

vs

 if(foo == NULL)

>, and it's likely to get optimised better.

Unlikely that it will be better, as the compiler knows that bar() returns 
bool, testing it against TRUE or FALSE does not make a difference to
using "bar()" or "!bar()" respectively.

But I agree. The "!" operator was invented *for a reason*, so we do not 
need ==false.

>That and someone might give true AND return a status by returning neither 
>0 or 1, in which case 

In that case you cannot work with bools at all, because they are not 
guaranteed to be big enough for status codes. Here, 'int' comes into play.
And then func_return_int()==TRUE/FALSE seems strange anyhow. That's like

>> > If this is the case, then wouldn't "long" be preferable to "int"?
>
>Meh, it's all the same.

On x86, it is not. But x86_64 has int=4 bytes and long=8 bytes.
What's wasted is probably the cacheline due to a longer immediate 
integer in the instruction. SPARC64 for example is even more sensitive:
also has int=4 and long=8, but generates a lot more instructions for 'long' 
data shuffling than for 'int', because it has a small instruction size.
And also start to think of 'long long' (resp. int64_t) used somewhere [in 
code] on x86 -- also needs extra ops because the regs are not wide enough.


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean

[Jan Engelhardt]

>
>  -Why we wouldn't want it
>  -C++ and Java fans will treat bool as a green light to the following
>
>	if (!(flags == true))
>  and
>	if (!(flags == false))
>
You can already do that with C. (given a #define true and #define false).
Just add something to CodingStyle like [warning: generalized] "we don't 
accept stupid bool logic code".


Jan Engelhardt
-- 

[RFC][PATCH] A generic boolean (version 2)

[ricknu-0]

Here is the second "version". My special thanks to Jeff Garzik and Alexey Dobriyan.

The changes are:
* u2 has been corrected to u1 (and also added it as __u1)
* removed the check if the gcc-compiler is at least of version 3, since it is
required of the kernel
* changed "false" and "true" from enum to #define, since an enum is the size of
an int. No #undef since an other definition of "false" and/or "true" should be
tracked down and not hidden)
    (this is the same as in gcc's stdbool.h-file. Is it not C99 compliant then?)
* bool is of "unsigned int" (under i386, may differ on other arches), instead of
"unsigned char"


Happy hacking
/Richard Knutsson

PS
I added everyone I found in the thread so if you to be removed, please tell.
DS


Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/asm-i386/types.h          |   11 +++++++++++
 include/linux/stddef.h            |    3 +++
 6 files changed, 16 insertions(+), 10 deletions(-)


diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
index 4b4b295..841792b 100644
--- a/include/asm-i386/types.h
+++ b/include/asm-i386/types.h
@@ -1,6 +1,13 @@
 #ifndef _I386_TYPES_H
 #define _I386_TYPES_H
 
+#if defined(__GNUC__)
+typedef _Bool bool;
+#else
+#warning You compiler doesn't seem to support boolean types, will set 'bool' as
an 'unsigned int'
+typedef unsigned int bool;
+#endif
+
 #ifndef __ASSEMBLY__
 
 typedef unsigned short umode_t;
@@ -10,6 +17,8 @@ typedef unsigned short umode_t;
  * header files exported to user space
  */
 
+typedef bool __u1;
+
 typedef __signed__ char __s8;
 typedef unsigned char __u8;
 
@@ -36,6 +45,8 @@ #define BITS_PER_LONG 32
 #ifndef __ASSEMBLY__
 
 
+typedef bool u1;
+
 typedef signed char s8;
 typedef unsigned char u8;
 
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..498813b 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,9 @@ #else
 #define NULL ((void *)0)
 #endif
 
+#define false	((0))
+#define true	((1))
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)

Re: [RFC][PATCH] A generic boolean (version 2)

[Jeff Garzik]

ricknu-0@student.ltu.se wrote:
> diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> index 4b4b295..841792b 100644
> --- a/include/asm-i386/types.h
> +++ b/include/asm-i386/types.h
> @@ -1,6 +1,13 @@
>  #ifndef _I386_TYPES_H
>  #define _I386_TYPES_H
>  
> +#if defined(__GNUC__)
> +typedef _Bool bool;
> +#else
> +#warning You compiler doesn't seem to support boolean types, will set 'bool' as
> an 'unsigned int'
> +typedef unsigned int bool;
> +#endif
> +
>  #ifndef __ASSEMBLY__
>  
>  typedef unsigned short umode_t;

Just delete the #ifdef and assume its either gcc, or a compatible 
compiler.  That's what we assume with other data types.


> @@ -10,6 +17,8 @@ typedef unsigned short umode_t;
>   * header files exported to user space
>   */
>  
> +typedef bool __u1;
> +
>  typedef __signed__ char __s8;
>  typedef unsigned char __u8;
>  
> @@ -36,6 +45,8 @@ #define BITS_PER_LONG 32
>  #ifndef __ASSEMBLY__
>  
>  
> +typedef bool u1;
> +
>  typedef signed char s8;
>  typedef unsigned char u8;
>  

I wouldn't bother with these types.  Nobody uses creates in their own 
hand-crafted bool uses, so I don't think people would suddenly start.


> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index b3a2cad..498813b 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -10,6 +10,9 @@ #else
>  #define NULL ((void *)0)
>  #endif
>  
> +#define false	((0))
> +#define true	((1))

I would say:

#undef true
#undef false
enum {
	false	= 0,
	true	= 1
};

#define false false
#define true true

Re: [RFC][PATCH] A generic boolean (version 2)

[Pekka Enberg]

On 7/21/06, Jeff Garzik <jeff@garzik.org> wrote:
> I would say:
>
> #undef true
> #undef false
> enum {
>         false   = 0,
>         true    = 1
> };
>
> #define false false
> #define true true

Just curious, why the #defines?

Re: [RFC][PATCH] A generic boolean (version 2)

[Jan Engelhardt]

>The changes are:
>* u2 has been corrected to u1 (and also added it as __u1)

Do we really need this? Is not 'bool' enough?


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean (version 2)

[Michael Buesch]

On Friday 21 July 2006 16:23, Jan Engelhardt wrote:
> >The changes are:
> >* u2 has been corrected to u1 (and also added it as __u1)
> 
> Do we really need this? Is not 'bool' enough?

I would say we don't even _want_ this.
A u1 variable will basically never be one bit wide.
It will be at least 8bit, or let's say 32bit. Maybe
even 64bit on some archs. It all depends on the compiler
plus the arch.

We _don't_ want u1, because we don't get what we see.
If we say u8 or u32, we get an 8bit wide data type and
a 32bit wide type. But we _don't_ get a 1bit wide
type for u1. We get something undefined.

Consider:

struct device_control_buffer {
	u1 device_is_fooing;
	u32 foodata;
} __attribute__((packed));

This would not lead to the expected results.
It's horribly broken, obfuscating and misleading.

-- 
Greetings Michael.

Re: [RFC][PATCH] A generic boolean (version 2)

[Jeff Garzik]

Michael Buesch wrote:
> On Friday 21 July 2006 16:23, Jan Engelhardt wrote:
>>> The changes are:
>>> * u2 has been corrected to u1 (and also added it as __u1)
>> Do we really need this? Is not 'bool' enough?
> 
> I would say we don't even _want_ this.
> A u1 variable will basically never be one bit wide.
> It will be at least 8bit, or let's say 32bit. Maybe
> even 64bit on some archs. It all depends on the compiler
> plus the arch.
> 
> We _don't_ want u1, because we don't get what we see.

For this and 1000 other reasons, we don't want u1.

	Jeff

Re: [RFC][PATCH] A generic boolean (version 2)

[Jeff Garzik]

Pekka Enberg wrote:
> On 7/21/06, Jeff Garzik <jeff@garzik.org> wrote:
>> I would say:
>>
>> #undef true
>> #undef false
>> enum {
>>         false   = 0,
>>         true    = 1
>> };
>>
>> #define false false
>> #define true true
> 
> Just curious, why the #defines?

So they are visible to cpp as well as C.

	Jeff

Re: [RFC][PATCH] A generic boolean (version 2)

[ricknu-0]

Citerar Jeff Garzik <jeff@garzik.org>:

> Michael Buesch wrote:
> > On Friday 21 July 2006 16:23, Jan Engelhardt wrote:
> >>> The changes are:
> >>> * u2 has been corrected to u1 (and also added it as __u1)
> >> Do we really need this? Is not 'bool' enough?
> > 
> > I would say we don't even _want_ this.
> > A u1 variable will basically never be one bit wide.
> > It will be at least 8bit, or let's say 32bit. Maybe
> > even 64bit on some archs. It all depends on the compiler
> > plus the arch.
> > 
> > We _don't_ want u1, because we don't get what we see.
> 
> For this and 1000 other reasons, we don't want u1.

This is a classic "do what others have done (with some modifications) and not
give a thought about it"... it's gone!

Re: [RFC][PATCH] A generic boolean (version 2)

[ricknu-0]

Citerar Jeff Garzik <jeff@garzik.org>:

> ricknu-0@student.ltu.se wrote:
> > diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> > index 4b4b295..841792b 100644
> > --- a/include/asm-i386/types.h
> > +++ b/include/asm-i386/types.h
> > @@ -1,6 +1,13 @@
> >  #ifndef _I386_TYPES_H
> >  #define _I386_TYPES_H
> >  
> > +#if defined(__GNUC__)
> > +typedef _Bool bool;
> > +#else
> > +#warning You compiler doesn't seem to support boolean types, will set
> 'bool' as
> > an 'unsigned int'
> > +typedef unsigned int bool;
> > +#endif
> > +
> >  #ifndef __ASSEMBLY__
> >  
> >  typedef unsigned short umode_t;
> 
> Just delete the #ifdef and assume its either gcc, or a compatible 
> compiler.  That's what we assume with other data types.

You are right. Will remove it.
Just remembered one of reasons I had version-check before, how about linux 2.4?
What I understand, they have the same drivers as 2.6 but they have not commited
to use gcc >= 3. Can anyone confirm or deny this? Otherwise the discussion about
alternetiv bool-type is off no relevence anymore.

> 
> 
> > @@ -10,6 +17,8 @@ typedef unsigned short umode_t;
> >   * header files exported to user space
> >   */
> >  
> > +typedef bool __u1;
> > +
> >  typedef __signed__ char __s8;
> >  typedef unsigned char __u8;
> >  
> > @@ -36,6 +45,8 @@ #define BITS_PER_LONG 32
> >  #ifndef __ASSEMBLY__
> >  
> >  
> > +typedef bool u1;
> > +
> >  typedef signed char s8;
> >  typedef unsigned char u8;
> >  
> 
> I wouldn't bother with these types.  Nobody uses creates in their own 
> hand-crafted bool uses, so I don't think people would suddenly start.

Removed

/Richard

Re: [RFC][PATCH] A generic boolean (version 3)

[ricknu-0]

Here 

Citerar ricknu-0@student.ltu.se:

> From: Richard Knutsson
> 
> A first step to a generic boolean-type. The patch just introduce the bool
> (in
> arch. i386 only (for the moment)), false and true + fixing some duplications
> in
> the code.
> It is diffed against Linus' git-tree (060718)
> Compiled with my own-, allyesconfig- and allmodconfig-config.h-file.
> 
> -Why would we want it?
> -There is already some how are depending on a "boolean"-type (like NTFS).
> Also,
> it will clearify functions who returns a boolean from one returning a value,
> ex:
> bool it_is_ok();
> char it_is_ok();
> The first one is obvious what it is doing, the secound might return some sort
> of
> status.
> 
> -Why false and not FALSE, why not "enum {...} bool"
> -They are not #define(d) and shouldn't because it is a value, like 'a'. But
> because it is just a value, then bool is just a variable and should be able
> to
> handle 0 and 1 equally well.
> 
> Well, this is _my_ opinion, it may be totally wrong. If so, please tell me
> ;)
> 
> If this takes off, I guess I will spend quite some time at kernel-janitors
> "cleaning" those who use a boolean-type.
> 
> Til' next time...
> /Richard Knutsson
> 
> PS
> Yes, I know about Andrew's try to unify TRUE and FALSE, did read the thread
> with
> interest (that's from where I got to know about _Bool). But mostly (then
> still
> on the subject) was some people did not want FALSE and TRUE instead of 0 and
> 1.
> I look at it as: 'a' = 97, if someone like to write 97 instead of 'a', please
> do
> if you find it easier to read. I, on the other hand, think it is easier with
> 'a', false/FALSE, NULL, etc.
> DS
> 
> PPS
> One thing about _Bool thue:
> _Bool a = 12; results in a = 1
> 
> test( char * t ) { t = 12; }
> main() {
> _Bool a;
> test( (char *) &a ); results in a = 12.
> }
> 
> But I do not think of it as a problem since a "true" is just !false. Doing:
> if (boolvar == true)
> seems odd, after all...
> 
> ... and sorry for the longwinded letter :)
> DDS
> 
> Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>
> 
> ---
> 
>  drivers/block/DAC960.h            |    2 +-
>  drivers/media/video/cpia2/cpia2.h |    4 ----
>  drivers/net/dgrs.c                |    1 -
>  drivers/scsi/BusLogic.h           |    5 +----
>  include/asm-i386/types.h          |    9 +++++++++
>  include/linux/stddef.h            |    2 ++
>  6 files changed, 13 insertions(+), 10 deletions(-)
> 
> 
> 
> diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
> index a82f37f..f9217c3 100644
> --- a/drivers/block/DAC960.h
> +++ b/drivers/block/DAC960.h
> @@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
>    Define a Boolean data type.
>  */
>  
> -typedef enum { false, true } __attribute__ ((packed)) boolean;
> +typedef bool boolean;
>  
>  
>  /*
> diff --git a/drivers/media/video/cpia2/cpia2.h
> b/drivers/media/video/cpia2/cpia2.h
> index c5ecb2b..8d2dfc1 100644
> --- a/drivers/media/video/cpia2/cpia2.h
> +++ b/drivers/media/video/cpia2/cpia2.h
> @@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
>  /***
>   * Image defines
>   ***/
> -#ifndef true
> -#define true 1
> -#define false 0
> -#endif
>  
>  /*  Misc constants */
>  #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
> diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
> index fa4f094..4dbc23d 100644
> --- a/drivers/net/dgrs.c
> +++ b/drivers/net/dgrs.c
> @@ -110,7 +110,6 @@ static char version[] __initdata =
>   *	DGRS include files
>   */
>  typedef unsigned char uchar;
> -typedef unsigned int bool;
>  #define vol volatile
>  
>  #include "dgrs.h"
> diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
> index 9792e5a..d6d1d56 100644
> --- a/drivers/scsi/BusLogic.h
> +++ b/drivers/scsi/BusLogic.h
> @@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
>    Define a Boolean data type.
>  */
>  
> -typedef enum {
> -	false,
> -	true
> -} PACKED boolean;
> +typedef bool boolean;
>  
>  /*
>    Define a 10^18 Statistics Byte Counter data type.
> diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> index 4b4b295..e35709a 100644

> --- a/include/asm-i386/types.h
> +++ b/include/asm-i386/types.h
> @@ -10,6 +10,15 @@ typedef unsigned short umode_t;
>   * header files exported to user space
>   */
>  
> +#if defined(__GNUC__) && __GNUC__ >= 3
> +typedef _Bool bool;
> +#else
> +#warning You compiler doesn't seem to support boolean types, will set 'bool'
> as
> an 'unsigned char'
> +typedef unsigned char bool;
> +#endif
> +
> +typedef bool u2;
> +
>  typedef __signed__ char __s8;
>  typedef unsigned char __u8;
>  
> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index b3a2cad..5e5c611 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -10,6 +10,8 @@ #else
>  #define NULL ((void *)0)
>  #endif
>  
> +enum { false = 0, true = 1 } __attribute__((packed));
> +
>  #undef offsetof
>  #ifdef __compiler_offsetof
>  #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
> 
> -
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/
> 

Re: [RFC][PATCH] A generic boolean (version 3)

[ricknu-0]

(Sorry about the dummy-mail I just sent. Pressed send by accident.)

Here is the next "version".

Changes since version 2:
* removed __u1 and u1, not really usable and error-prone.
* removed #ifdef __GNUC__ when defining bool.
* changed back the version-checking when defining bool, this since I'm quite
sure Linux 2.4 and 2.6 share (if not all, then large part of) the drivers. And
2.4 is still compiled with the 2.95 Gcc.
* changed to Jeff's version when defining "false" and "true".

Thanks for all the comments and suggestion.
/Richard Knutsson

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/asm-i386/types.h          |    7 +++++++
 include/linux/stddef.h            |   11 +++++++++++
 6 files changed, 20 insertions(+), 10 deletions(-)


diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
index 4b4b295..f035a15 100644
--- a/include/asm-i386/types.h
+++ b/include/asm-i386/types.h
@@ -1,6 +1,13 @@
 #ifndef _I386_TYPES_H
 #define _I386_TYPES_H
 
+#if __GNUC__ >= 3
+typedef _Bool bool;
+#else
+#warning You compiler doesn't seem to support boolean types, will set 'bool' as
an 'unsigned int'
+typedef unsigned int bool;
+#endif
+
 #ifndef __ASSEMBLY__
 
 typedef unsigned short umode_t;
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..f137815 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,17 @@ #else
 #define NULL ((void *)0)
 #endif
 
+#undef false
+#undef true
+
+enum {
+	false	= 0,
+	true	= 1
+};
+
+#define false false
+#define true true 
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)

Re: [RFC][PATCH] A generic boolean (version 3)

[Stefan Richter]

ricknu-0@student.ltu.se wrote:
> * changed back the version-checking when defining bool, this since I'm quite
> sure Linux 2.4 and 2.6 share (if not all, then large part of) the drivers. And
> 2.4 is still compiled with the 2.95 Gcc.

Drivers in 2.4 and 2.6 differ. We don't put 2.4-compatibility code into 
2.6. And the bool type won't get into 2.4.
-- 
Stefan Richter
-=====-=-==- -=== =-==-
http://arcgraph.de/sr/

Re: [RFC][PATCH] A generic boolean (version 2)

[Jan Engelhardt]

>> >* u2 has been corrected to u1 (and also added it as __u1)
>> 
>> Do we really need this? Is not 'bool' enough?
>
>I would say we don't even _want_ this.
>A u1 variable will basically never be one bit wide.

Not without a compiler hack at least.

>Consider:
>
>struct device_control_buffer {
>	u1 device_is_fooing;
>	u32 foodata;
>} __attribute__((packed));
>
>This would not lead to the expected results.
>It's horribly broken, obfuscating and misleading.

And in fact, bitfields work different:

struct device {
	int device_is_fooing:1;
	u32 foodata;
};

but the result is likely the same.


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean (version 3)

[Jan Engelhardt]

>+++ b/drivers/block/DAC960.h
>@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
>   Define a Boolean data type.
> */
> 
>-typedef enum { false, true } __attribute__ ((packed)) boolean;
>+typedef bool boolean;
> 
Probably I missed some mail, but why can't we just have typedef _Bool bool?
Like below?

>+++ b/include/asm-i386/types.h
>@@ -1,6 +1,13 @@
> #ifndef _I386_TYPES_H
> #define _I386_TYPES_H
> 
>+#if __GNUC__ >= 3
>+typedef _Bool bool;
>+#else
>+#warning You compiler doesn't seem to support boolean types, will set 'bool' as
>an 'unsigned int'
>+typedef unsigned int bool;
>+#endif
>+
> #ifndef __ASSEMBLY__
> 
> typedef unsigned short umode_t;
>--- a/include/linux/stddef.h
>+++ b/include/linux/stddef.h
>@@ -10,6 +10,17 @@ #else
> #define NULL ((void *)0)
> #endif
> 
>+#undef false
>+#undef true

Wasnot this supposed to go away?

>+
>+enum {
>+	false	= 0,
>+	true	= 1
>+};
>+
>+#define false false
>+#define true true 
>+
> #undef offsetof
> #ifdef __compiler_offsetof
> #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
>

Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean (version 3)

[Lars Gullik Bjønnes]

ricknu-0@student.ltu.se writes:

| --- a/include/asm-i386/types.h
| +++ b/include/asm-i386/types.h
| @@ -1,6 +1,13 @@
|  #ifndef _I386_TYPES_H
|  #define _I386_TYPES_H
|  
| +#if __GNUC__ >= 3
| +typedef _Bool bool;
| +#else
| +#warning You compiler doesn't seem to support boolean types, will set 'bool' as
| an 'unsigned int'
| +typedef unsigned int bool;
| +#endif
| +

What does C99 say about sizeof(_Bool)?

At least with gcc 4 it is 1. Can that pose a problem? gcc < 3 giving a
different size for bool?

-- 
	Lgb

Re: [RFC][PATCH] A generic boolean (version 3)

[ricknu-0]

Citerar Stefan Richter <stefanr@s5r6.in-berlin.de>:

> ricknu-0@student.ltu.se wrote:
> > * changed back the version-checking when defining bool, this since I'm
> quite
> > sure Linux 2.4 and 2.6 share (if not all, then large part of) the drivers.
> And
> > 2.4 is still compiled with the 2.95 Gcc.
> 
> Drivers in 2.4 and 2.6 differ. We don't put 2.4-compatibility code into 
> 2.6. And the bool type won't get into 2.4.

It doesn't?! Good, that simplify it to only a:
typedef _Bool bool;
line. Did googled on it but did not find anything that comfirmed or denied it.
Thanks

> -- 
> Stefan Richter

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 3)

[ricknu-0]

Citerar Jan Engelhardt <jengelh@linux01.gwdg.de>:

> >+++ b/drivers/block/DAC960.h
> >@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
> >   Define a Boolean data type.
> > */
> > 
> >-typedef enum { false, true } __attribute__ ((packed)) boolean;
> >+typedef bool boolean;
> > 
> Probably I missed some mail, but why can't we just have typedef _Bool bool?
> Like below?

Because it defines bool in include/asm/types.h, then "all" files will have the
bool-type. But because DAC960.h calles it boolean I just typedefed the new
bool-type to "boolean". Then if this gets into -mm or linus-tree, I/we can start
to clean up all the code using booleans. After all, there is also BOOL and
BOOLEAN but they are not effected by this change.

> 
> >+++ b/include/asm-i386/types.h
> >@@ -1,6 +1,13 @@
> > #ifndef _I386_TYPES_H
> > #define _I386_TYPES_H
> > 
> >+#if __GNUC__ >= 3
> >+typedef _Bool bool;
> >+#else
> >+#warning You compiler doesn't seem to support boolean types, will set
> 'bool' as
> >an 'unsigned int'
> >+typedef unsigned int bool;
> >+#endif
> >+
> > #ifndef __ASSEMBLY__
> > 
> > typedef unsigned short umode_t;
> >--- a/include/linux/stddef.h
> >+++ b/include/linux/stddef.h
> >@@ -10,6 +10,17 @@ #else
> > #define NULL ((void *)0)
> > #endif
> > 
> >+#undef false
> >+#undef true
> 
> Wasnot this supposed to go away?

#undef or enum?
There has not been any #undef's before, and enum I think is better. This since
Jeff showed it could be combined with the #define for the cpp's sake.

> 
> >+
> >+enum {
> >+	false	= 0,
> >+	true	= 1
> >+};
> >+
> >+#define false false
> >+#define true true 
> >+
> > #undef offsetof
> > #ifdef __compiler_offsetof
> > #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
> >
> 
> Jan Engelhardt

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 3)

[Stefan Richter]

ricknu-0@student.ltu.se wrote:
> Citerar Stefan Richter <stefanr@s5r6.in-berlin.de>:
...
>>Drivers in 2.4 and 2.6 differ. We don't put 2.4-compatibility code into 
>>2.6. And the bool type won't get into 2.4.
> 
> It doesn't?! Good, that simplify it to only a:
> typedef _Bool bool;
> line. Did googled on it but did not find anything that comfirmed or denied it.
...

Well, it's because the 2.4 mainline receives only bug fixes now. You can 
imagine that since 2.5 was started, new data types were added in 2.5/2.6 
but never backported to 2.4, especially if they were tied to 
infrastructural changes or new features.

Of course there may be downstream projects who maintain 2.4 drivers to 
further extent than the 2.4 mainline. But this doesn't imply a 
requirement to put compatibility code into 2.6.
-- 
Stefan Richter
-=====-=-==- -=== =-==-
http://arcgraph.de/sr/

Re: [RFC][PATCH] A generic boolean (version 3)

[ricknu-0]

Citerar Lars Gullik Bjønnes <larsbj@gullik.net>:

> ricknu-0@student.ltu.se writes:
> 
> | --- a/include/asm-i386/types.h
> | +++ b/include/asm-i386/types.h
> | @@ -1,6 +1,13 @@
> |  #ifndef _I386_TYPES_H
> |  #define _I386_TYPES_H
> |  
> | +#if __GNUC__ >= 3
> | +typedef _Bool bool;
> | +#else
> | +#warning You compiler doesn't seem to support boolean types, will set
> 'bool' as
> | an 'unsigned int'
> | +typedef unsigned int bool;
> | +#endif
> | +
> 
> What does C99 say about sizeof(_Bool)?
> 
> At least with gcc 4 it is 1. Can that pose a problem? gcc < 3 giving a
> different size for bool?

Well, it might. I don't see it other then when someone uses a variable of
another type and the "boolean" variable (now an int) is of a larger value then
that variable. But then that is a bug and should be fixed.

But rest easy, bool will only be _Bool from now on. :)

> -- 
> 	Lgb

Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 4)

[ricknu-0]

And here comes lucky number four.

Since there seems to be no case where _Bool is not defined (we uses only gcc >=
3), it got simplefied to just a:
typedef _Bool bool;

Hopefully it is now ready for a "real" patch, whom adds bool to all arches. If
there is no comments on this one, it will be sent about tomorrow night (GMT).

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/asm-i386/types.h          |    2 ++
 include/linux/stddef.h            |   11 +++++++++++
 6 files changed, 15 insertions(+), 10 deletions(-)


diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
index 4b4b295..3cb84ac 100644
--- a/include/asm-i386/types.h
+++ b/include/asm-i386/types.h
@@ -1,6 +1,8 @@
 #ifndef _I386_TYPES_H
 #define _I386_TYPES_H
 
+typedef _Bool bool;
+
 #ifndef __ASSEMBLY__
 
 typedef unsigned short umode_t;
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..f137815 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,17 @@ #else
 #define NULL ((void *)0)
 #endif
 
+#undef false
+#undef true
+
+enum {
+	false	= 0,
+	true	= 1
+};
+
+#define false false
+#define true true 
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)

Re: [RFC][PATCH] A generic boolean (version 4)

[Jan Engelhardt]

>Hopefully it is now ready for a "real" patch, whom adds bool to all
>arches. If there is no comments on this one, it will be sent about
>tomorrow night (GMT).

--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*

Looks good. (I know found out what this is good for. Eventually, all booleans
in the source of DAC960 et al. should be changed to just 'bool' but that's
another patch's job.)

Looks good, except for the "all arches" thing. You only seem to add it
to i386:

>--- a/include/asm-i386/types.h
>+++ b/include/asm-i386/types.h
>@@ -1,6 +1,8 @@
> #ifndef _I386_TYPES_H
> #define _I386_TYPES_H
> 
>+typedef _Bool bool;
>+
> #ifndef __ASSEMBLY__
> 
> typedef unsigned short umode_t;


>+#undef false
>+#undef true
>+
>+enum {
>+	false	= 0,
>+	true	= 1
>+};
>+
>+#define false false
>+#define true true 

Can someone please tell me what advantage 'define true true' is going to
bring, besides than being able to '#ifdef true'?



Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean (version 4)

[Michael Buesch]

On Sunday 23 July 2006 17:49, ricknu-0@student.ltu.se wrote:
> And here comes lucky number four.

> diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> index 4b4b295..3cb84ac 100644
> --- a/include/asm-i386/types.h
> +++ b/include/asm-i386/types.h
> @@ -1,6 +1,8 @@
>  #ifndef _I386_TYPES_H
>  #define _I386_TYPES_H
>  
> +typedef _Bool bool;
> +
>  #ifndef __ASSEMBLY__

I'd say that typedef must go into the !__ASSEMBLY__ section here,
like the other typedefs in that header.

-- 
Greetings Michael.

Re: [RFC][PATCH] A generic boolean (version 4)

[ricknu-0]

Citerar Jan Engelhardt <jengelh@linux01.gwdg.de>:

> 
> >Hopefully it is now ready for a "real" patch, whom adds bool to all
> >arches. If there is no comments on this one, it will be sent about
> >tomorrow night (GMT).
> 
> --- a/drivers/block/DAC960.h
> +++ b/drivers/block/DAC960.h
> @@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
>    Define a Boolean data type.
>  */
>  
> -typedef enum { false, true } __attribute__ ((packed)) boolean;
> +typedef bool boolean;
>  
>  
>  /*
> 
> Looks good. (I know found out what this is good for. Eventually, all
> booleans
> in the source of DAC960 et al. should be changed to just 'bool' but that's
> another patch's job.)

Yepp :)

> Looks good, except for the "all arches" thing. You only seem to add it
> to i386:

I said: 'ready for a "real" patch', meaning it will be in another patch.
Will try to be more clear in the future.

> >+#undef false
> >+#undef true
> >+
> >+enum {
> >+	false	= 0,
> >+	true	= 1
> >+};
> >+
> >+#define false false
> >+#define true true 
> 
> Can someone please tell me what advantage 'define true true' is going to
> bring, besides than being able to '#ifdef true'?

Assembly-code can not use enum but #define. That is the reason I find but there
might be more.


> Jan Engelhardt

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 4)

[ricknu-0]

Citerar Michael Buesch <mb@bu3sch.de>:

> On Sunday 23 July 2006 17:49, ricknu-0@student.ltu.se wrote:
> > And here comes lucky number four.
> 
> > diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> > index 4b4b295..3cb84ac 100644
> > --- a/include/asm-i386/types.h
> > +++ b/include/asm-i386/types.h
> > @@ -1,6 +1,8 @@
> >  #ifndef _I386_TYPES_H
> >  #define _I386_TYPES_H
> >  
> > +typedef _Bool bool;
> > +
> >  #ifndef __ASSEMBLY__
> 
> I'd say that typedef must go into the !__ASSEMBLY__ section here,
> like the other typedefs in that header.

You are right. Misstook the #endif as #else (a downside of programming late).
It is corrected, thanks.

> Greetings Michael.

See ya
/Richard

Re: [RFC][PATCH] A generic boolean (version 2)

[ricknu-0]

Citerar Jeff Garzik <jeff@garzik.org>:

> ricknu-0@student.ltu.se wrot> 

> > diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> > index b3a2cad..498813b 100644
> > --- a/include/linux/stddef.h
> > +++ b/include/linux/stddef.h
> > @@ -10,6 +10,9 @@ #else
> >  #define NULL ((void *)0)
> >  #endif
> >  
> > +#define false	((0))
> > +#define true	((1))
> 
> I would say:
> 
> #undef true
> #undef false

Sorry about the delay but why the undef's? Found no problem to remove those and
think a warning would be good if a #define of false/true would show up
(otherwise, why have them there in the first place?).

> enum {
> 	false	= 0,
> 	true	= 1
> };
> 
> #define false false
> #define true true

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 4)

[Jeff Garzik]

ricknu-0@student.ltu.se wrote:
> diff --git a/include/asm-i386/types.h b/include/asm-i386/types.h
> index 4b4b295..3cb84ac 100644
> --- a/include/asm-i386/types.h
> +++ b/include/asm-i386/types.h
> @@ -1,6 +1,8 @@
>  #ifndef _I386_TYPES_H
>  #define _I386_TYPES_H
>  
> +typedef _Bool bool;
> +
>  #ifndef __ASSEMBLY__
>  


This probably belongs in include/linux/types.h.

Other than that, looks good to me.

	Jeff

Re: [RFC][PATCH] A generic boolean (version 4)

[Jeff Garzik]

Jan Engelhardt wrote:
>> +#undef false
>> +#undef true
>> +
>> +enum {
>> +	false	= 0,
>> +	true	= 1
>> +};
>> +
>> +#define false false
>> +#define true true 
> 
> Can someone please tell me what advantage 'define true true' is going to
> bring, besides than being able to '#ifdef true'?

It

(a) makes type information available to the C compiler, where a plain 
#define does not.
(b) handles all '#ifndef true' statements properly

	Jeff

Re: [RFC][PATCH] A generic boolean (version 4)

[Jeff Garzik]

ricknu-0@student.ltu.se wrote:
>>> +#undef false
>>> +#undef true
>>> +
>>> +enum {
>>> +	false	= 0,
>>> +	true	= 1
>>> +};
>>> +
>>> +#define false false
>>> +#define true true 
>> Can someone please tell me what advantage 'define true true' is going to
>> bring, besides than being able to '#ifdef true'?
> 
> Assembly-code can not use enum but #define. That is the reason I find but there
> might be more.


That won't work -- the #define just makes the enum available, which 
doesn't work at the assembler level.

But assembly code doesn't use this stuff, so no worries.

	Jeff

Re: [RFC][PATCH] A generic boolean (version 4)

[Paul Jackson]

The defines of false and true to be themselves are going to cause a
thousand code readers to say "wtf?".  How about a comment, as in:

 enum {
 	false	= 0,
 	true	= 1
 };
+
+/* Let any other #if[n]def's on false/true presume they're defined */
 #define false false
 #define true true 
 

===

On second thought, I just grep'd the entire kernel with:
	grep -E '#ifn*def.*(false|true)'
and only found exactly one other such #ifdef construct:

  drivers/media/video/cpia2/cpia2.h:
    #ifndef true
    #define true 1
    #define false 0
    #endif

Perhaps we should just drop the cpia2.h defines, and drop
the defines of false/true to be themselves.  And drop my
comment ;).

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [RFC][PATCH] A generic boolean (version 4)

[Jan Engelhardt]

>> > +#define false false
>> > +#define true true 
>> 
>> Can someone please tell me what advantage 'define true true' is going to
>> bring, besides than being able to '#ifdef true'?
>
> It
>
> (a) makes type information available to the C compiler, where a plain #define
> does not.

Do you mean preprocessor? C already knows about true from the enum.

> (b) handles all '#ifndef true' statements properly

Holy *, is there _really_ code in linux/ that depends on true being 
[not] defined?


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean (version 4)

[Jeff Garzik]

Jan Engelhardt wrote:
>>>> +#define false false
>>>> +#define true true 
>>> Can someone please tell me what advantage 'define true true' is going to
>>> bring, besides than being able to '#ifdef true'?
>> It
>>
>> (a) makes type information available to the C compiler, where a plain #define
>> does not.
> 
> Do you mean preprocessor? C already knows about true from the enum.

I was describing the overall purpose of the enum + #define change, when 
you take my "(a)" and "(b)" in sum.


>> (b) handles all '#ifndef true' statements properly
> 
> Holy *, is there _really_ code in linux/ that depends on true being 
> [not] defined?

I suggest re-reading the boolean patches in this thread, to answer that 
question...

Programmer wanting to use boolean inevitably add an '#ifndef true' or 
'#ifndef TRUE' style statement to their code.

	Jeff

Re: [RFC][PATCH] A generic boolean (version 4)

[Paul Jackson]

Jan wrote:
> Can someone please tell me what advantage 'define true true' is going to
> bring, besides than being able to '#ifdef true'?

I think Jeff's replies to this are a tad confusing.  He seems
to be answering in part with the broader point of the value of
increased type checking using enums, and not focusing on the
specific reason these two defines of true and false are there.

As best as I can tell, these two odd looking defines are just to
suppress #define alternative's to our enum based false and true
constructs, in the likely case that the alternatives are guarded
with the usual #ifndef logic.

In other words, their advantage is basically what you said, being
able to '#ifdef true' (or #ifndef).

As I recommend in another subthread, these two defines need a comment.

And since there is only one "#ifndef true" left in the entire kernel
source tree, in drivers/media/video/cpia2/cpia2.h, I would think it
would be better to just remove those lines from cpia2.h, and drop
these two odd looking defines.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [RFC][PATCH] A generic boolean (version 2)

[Roman Kononov]

On 07/21/2006 16:14, Jeff Garzik wrote:
> Pekka Enberg wrote:
>> On 7/21/06, Jeff Garzik <jeff@garzik.org> wrote:
>>> I would say:
>>>
>>> #undef true
>>> #undef false
>>> enum {
>>>         false   = 0,
>>>         true    = 1
>>> };
>>>
>>> #define false false
>>> #define true true
>>
>> Just curious, why the #defines?
> 
> So they are visible to cpp as well as C.

What can cpp do with them other than #if defined() or #if !defined()?

Roman

Re: [RFC][PATCH] A generic boolean (version 5)

[ricknu-0]

And here come another, the fifth "version".

Compiled with .config's allyesconfig and allmodconfig.
Diff'ed (with no errors) with recently pulled Linus git-tree.

Changes:
* moved the bool-defining to include/linux/types.h (thanks Jeff for reminding me)
	not sure if it is well-placed. Please comment if you find it to be more suited
on another line.

Regarding the #define false/true in include/linux/stddef.h: Jeff, I can only
find a reason for it is if a userspace-program is including both
<linux/stddef.h> and <stdbool.h>. But since it uses only #define's, why should
we make the cpp care about it?
I does not really care if it's in or not, just since cpia2.h is fixed, there
seems to be no use for it (other then userspace programs).

Have a nice night
/Richard Knutsson

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/linux/stddef.h            |   11 +++++++++++
 include/linux/types.h             |    2 ++
 6 files changed, 15 insertions(+), 10 deletions(-)


diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..e3ad881 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,17 @@ #else
 #define NULL ((void *)0)
 #endif
 
+#undef false
+#undef true
+
+enum {
+	false	= 0,
+	true	= 1
+};
+
+#define false false
+#define true true
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
diff --git a/include/linux/types.h b/include/linux/types.h
index 3f23566..85cf587 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -90,6 +90,8 @@ #define _CADDR_T
 typedef __kernel_caddr_t	caddr_t;
 #endif
 
+typedef _Bool			bool;
+
 /* bsd */
 typedef unsigned char		u_char;
 typedef unsigned short		u_short;

Re: [RFC][PATCH] A generic boolean (version 5)

[Paul Jackson]

Richard wrote:
> Regarding the #define false/true in include/linux/stddef.h: ... there
> seems to be no use for it (other then userspace programs).

We tend to be biased against doing things in kernel headers just for
userspace programs.  I'd suggest either (1) drop the #define false/true's,
or (2) if a compelling userspace need is presented that wins the day,
at least comment the dang defines.

>From the perspective of what's in the current kernel source, this
pair of defines has lost all sense that I can see.  So they need a
justifying comment if they stay.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [RFC][PATCH] A generic boolean (version 6)

[ricknu-0]

The sixth "version".

Changes (since fifth):
* removed the #undef false/true and #define false/true from include/linux/stddef.h.

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

Have not found any (real) reason letting the cpp know about false/true. As I
said in the last version, the only reason seem to be for the userspace. Well, as
there is no program of my knowlage that needs it, they were removed.

As there seems to be little respons nowadays (I hope its because there isn't to
much to comment on and not because of bordom to this), I will likely send this
in as a real patch (hoping for inclusion) tomorrow.

And once again; thanks to you all for all the comments and suggestions.

/Richard


 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/linux/stddef.h            |    5 +++++
 include/linux/types.h             |    2 ++
 6 files changed, 9 insertions(+), 10 deletions(-)


diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..0382065 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,11 @@ #else
 #define NULL ((void *)0)
 #endif
 
+enum {
+	false	= 0,
+	true	= 1
+};
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
diff --git a/include/linux/types.h b/include/linux/types.h
index 3f23566..85cf587 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -90,6 +90,8 @@ #define _CADDR_T
 typedef __kernel_caddr_t	caddr_t;
 #endif
 
+typedef _Bool			bool;
+
 /* bsd */
 typedef unsigned char		u_char;
 typedef unsigned short		u_short;

Re: [RFC][PATCH] A generic boolean (version 6)

[Paul Jackson]

Richard wrote:
> * removed the #undef false/true and #define false/true

Good - thanks.

+enum {
+	false	= 0,
+	true	= 1
+};

My inclination would have been to write this as the more terse:

+enum { false, true };

But I suspect yours is better, as some readers would not be
confident that the terse form made false == 0 and true == 1.

> a real patch (hoping for inclusion) tomorrow.

Good.

I'm delighted that this favors "true, false and bool",
over "TRUE, FALSE and various spellings of BOOLEAN".

Fun stuff to do in the future:
  Convert test_bit() and various other test_*() and
	atomic_*() operators to return bool.
  Convert many TRUE/FALSE to true/false, in a patch of
	similar size to Andrew's March 2006 patch entitled:
	"[patch 1/1] consolidate TRUE and FALSE".
  Convert a variety of spellings of BOOLEAN to "bool".
  Convert routines and variables using the old C
	convention of int/0/1 for boolean to the
	new bool/false/true.
  How do we detect breakage that results from converting
	an apparent boolean to these values, when the
	code actually worked by using more than just
	values 0 and 1 for the variable in question?
  How do we detect any breakage caused by possible changes
	in the sizeof variables whose type we changed?
  Various sparse and/or gcc checks that benefit from
	knowing the additional constraints on bool types.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [RFC][PATCH] A generic boolean (version 6)

[Josef Sipek]

On Wed, Jul 26, 2006 at 06:06:22PM -0700, Paul Jackson wrote:
> Richard wrote:
> > * removed the #undef false/true and #define false/true
> 
> Good - thanks.
> 
> +enum {
> +	false	= 0,
> +	true	= 1
> +};

You probably have said it before, but why do we need this?

Josef "Jeff" Sipek.

-- 
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like
that.
		- Linus Torvalds

Re: [RFC][PATCH] A generic boolean (version 6)

[Arnd Bergmann]

On Wednesday 26 July 2006 22:28, ricknu-0@student.ltu.se wrote:
> Have not found any (real) reason letting the cpp know about false/true. As I
> said in the last version, the only reason seem to be for the userspace. Well, as
> there is no program of my knowlage that needs it, they were removed.
> 
If we don't expect this to show up in the ABI (which I hope is true), then
the definition should probably be inside of #ifdef __KERNEL__. Right
now, it's inside of (!__KERNEL_STRICT_NAMES), which is not exactly the
same.

	Arnd <><

Re: [RFC][PATCH] A generic boolean (version 6)

[ricknu-0]

Citerar Arnd Bergmann <arnd.bergmann@de.ibm.com>:

> On Wednesday 26 July 2006 22:28, ricknu-0@student.ltu.se wrote:
> > Have not found any (real) reason letting the cpp know about false/true. As
> I
> > said in the last version, the only reason seem to be for the userspace.
> Well, as
> > there is no program of my knowlage that needs it, they were removed.
> > 
> If we don't expect this to show up in the ABI (which I hope is true), then
> the definition should probably be inside of #ifdef __KERNEL__. Right
> now, it's inside of (!__KERNEL_STRICT_NAMES), which is not exactly the
> same.

Thanks

What do you think about this?:

diff --git a/include/linux/types.h b/include/linux/types.h
index 3f23566..406d4ae 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -33,6 +33,8 @@ typedef __kernel_clockid_t	clockid_t;
 typedef __kernel_mqd_t		mqd_t;
 
 #ifdef __KERNEL__
+typedef _Bool			bool;
+
 typedef __kernel_uid32_t	uid_t;
 typedef __kernel_gid32_t	gid_t;
 typedef __kernel_uid16_t        uid16_t;


> 	Arnd <><

/Richard

Re: [RFC][PATCH] A generic boolean (version 6)

[ricknu-0]

Citerar Paul Jackson <pj@sgi.com>:

> Fun stuff to do in the future:
>   Convert test_bit() and various other test_*() and
> 	atomic_*() operators to return bool.
>   Convert many TRUE/FALSE to true/false, in a patch of
> 	similar size to Andrew's March 2006 patch entitled:
> 	"[patch 1/1] consolidate TRUE and FALSE".
>   Convert a variety of spellings of BOOLEAN to "bool".
>   Convert routines and variables using the old C
> 	convention of int/0/1 for boolean to the
> 	new bool/false/true.
>   How do we detect breakage that results from converting
> 	an apparent boolean to these values, when the
> 	code actually worked by using more than just
> 	values 0 and 1 for the variable in question?
>   How do we detect any breakage caused by possible changes
> 	in the sizeof variables whose type we changed?
>   Various sparse and/or gcc checks that benefit from
> 	knowing the additional constraints on bool types.

Well... that's some work to be done :)

Will save the list and try to mark it of along the road.

>                   Paul Jackson <pj@sgi.com> 1.925.600.0401

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 6)

[ricknu-0]

Citerar Josef Sipek <jsipek@fsl.cs.sunysb.edu>:

> On Wed, Jul 26, 2006 at 06:06:22PM -0700, Paul Jackson wrote:
> > Richard wrote:
> > > * removed the #undef false/true and #define false/true
> > 
> > Good - thanks.
> > 
> > +enum {
> > +	false	= 0,
> > +	true	= 1
> > +};
> 
> You probably have said it before, but why do we need this?

Well Jeff, there is no _need_ for the false and true, but it makes the code more
readable to many of us. (Why else is there all these definitions of false and true?)
There is other opinions on this and that is one of the reason* bool isn't
defined as:

enum { false, true } bool;

letting us choose our own way.
So when (hopefully) the converting to these starts, it will only affect the
files already using some sort of false and true. If the author is more
comfortable with 0 and 1's, that will be respectet (by me, at least).

Hope it make some sense.

> Josef "Jeff" Sipek.

/Richard Knutsson

PS
If I got you wrong and you meant why it can't be defined where it is used, I
refere you to Andrew's mail "[patch 1/1] consolidate TRUE and FALSE", where a
redefinition occured (of TRUE/FALSE).
DS

* another is that C99 defines the boolean type for us (as _Bool).

Re: [RFC][PATCH] A generic boolean (version 6)

[Paul Jackson]

Josef wrote:
> You probably have said it before, but why do we need this?

Well, Richard should answer this, more than me.  He's the one who
braved this latest charge against a topic of much prior debate.

And I'm unsure what part of the above you wonder the reason for.

Ignoring all that ... my motivation is thus.

We've got umpteen ways of spelling and defining boolean types.  Code
would be more consistent, clear and clean if we had one way of spelling
it uniformly, through the kernel.  For example, Andrew's failed assault
on this topic in March was motivated by a build failure, when a couple
of variant spellings collided.

Some of us, apparently a majority, though the minority includes some
respected and vocal citizens of long standing, find it clearer to
explicitly code boolean types using some variant of bool/false/true,
rather than implicitly in the traditional C style of int/0/1.

C99 has added a native _Bool, along with a stdbool.h that defines
bool/false/true, and C++ has always had a native bool/false/true type.
So, despite Andrew's attempt in March to standardize on spelling
these values as FALSE/TRUE, the C99 spelling of false/true seems to
be carrying the day.

With C99's _Bool and a typedef of 'bool' for _Bool, we have a gcc
supported (for some definition of support ;) 'bool'.

But we need either #defines or an enum to spell false and true.
Since the enum provides greater opportunities for type checking,
that appears to be winning the day.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [RFC][PATCH] A generic boolean (version 6)

[Josef Sipek]

On Thu, Jul 27, 2006 at 05:51:10AM +0200, ricknu-0@student.ltu.se wrote:
...
> Hope it make some sense.

Fair enough.

> PS
> If I got you wrong and you meant why it can't be defined where it is used, I
> refere you to Andrew's mail "[patch 1/1] consolidate TRUE and FALSE", where a
> redefinition occured (of TRUE/FALSE).

You first guess was right.

Josef "Jeff" Sipek.

-- 
NT is to UNIX what a doughnut is to a particle accelerator.

Re: [RFC][PATCH] A generic boolean (version 6)

[Nicholas Miell]

On Thu, 2006-07-27 at 04:48 +0200, Arnd Bergmann wrote:
> On Wednesday 26 July 2006 22:28, ricknu-0@student.ltu.se wrote:
> > Have not found any (real) reason letting the cpp know about false/true. As I
> > said in the last version, the only reason seem to be for the userspace. Well, as
> > there is no program of my knowlage that needs it, they were removed.
> > 
> If we don't expect this to show up in the ABI (which I hope is true), then
> the definition should probably be inside of #ifdef __KERNEL__. Right
> now, it's inside of (!__KERNEL_STRICT_NAMES), which is not exactly the
> same.
> 

If _Bool does end up in the user-kernel ABI, be advised that validating
them will be tricky ("b == true || b == false" or "!!b" won't work), and
the compiler could in theory generate code which tests truthfulness by
comparing to 1 in one place and non-zero in another.

My brief IRC conversation with gcc people regarding validating untrusted
_Bool resulted in the instruction to never store a value in a _Bool
until after it has been validated. 

-- 
Nicholas Miell <nmiell@comcast.net>

Re: [RFC][PATCH] A generic boolean (version 6)

[Paul Jackson]

Nicholas wrote:
> If _Bool does end up in the user-kernel ABI

I'd be inclined to not put a _Bool in that ABI.
I'd stick with the scalar ints and such we have now.

The benefits of compile type checking are weaker
across that ABI, and the demands of compatibility
much higher than they are for kernel internals.

-- 
                  I won't rest till it's the best ...
                  Programmer, Linux Scalability
                  Paul Jackson <pj@sgi.com> 1.925.600.0401

Re: [RFC][PATCH] A generic boolean (version 7)

[ricknu-0]

... and it continues, here comes (lucky number) sleven.

Changes:
* put the bool-definition under __KERNEL__ (thanks Arnd).

Signed-off-by: Richard Knutsson <ricknu-0@student.ltu.se>

---

Like I said before, if this seems ok with you, I will send the "real" patch
tomorrow.

 drivers/block/DAC960.h            |    2 +-
 drivers/media/video/cpia2/cpia2.h |    4 ----
 drivers/net/dgrs.c                |    1 -
 drivers/scsi/BusLogic.h           |    5 +----
 include/linux/stddef.h            |    5 +++++
 include/linux/types.h             |    2 ++
 6 files changed, 9 insertions(+), 10 deletions(-)


diff --git a/drivers/block/DAC960.h b/drivers/block/DAC960.h
index a82f37f..f9217c3 100644
--- a/drivers/block/DAC960.h
+++ b/drivers/block/DAC960.h
@@ -71,7 +71,7 @@ #define DAC690_V2_PciDmaMask	0xfffffffff
   Define a Boolean data type.
 */
 
-typedef enum { false, true } __attribute__ ((packed)) boolean;
+typedef bool boolean;
 
 
 /*
diff --git a/drivers/media/video/cpia2/cpia2.h b/drivers/media/video/cpia2/cpia2.h
index c5ecb2b..8d2dfc1 100644
--- a/drivers/media/video/cpia2/cpia2.h
+++ b/drivers/media/video/cpia2/cpia2.h
@@ -50,10 +50,6 @@ #define CPIA2_PATCH_VER	0
 /***
  * Image defines
  ***/
-#ifndef true
-#define true 1
-#define false 0
-#endif
 
 /*  Misc constants */
 #define ALLOW_CORRUPT 0		/* Causes collater to discard checksum */
diff --git a/drivers/net/dgrs.c b/drivers/net/dgrs.c
index fa4f094..4dbc23d 100644
--- a/drivers/net/dgrs.c
+++ b/drivers/net/dgrs.c
@@ -110,7 +110,6 @@ static char version[] __initdata =
  *	DGRS include files
  */
 typedef unsigned char uchar;
-typedef unsigned int bool;
 #define vol volatile
 
 #include "dgrs.h"
diff --git a/drivers/scsi/BusLogic.h b/drivers/scsi/BusLogic.h
index 9792e5a..d6d1d56 100644
--- a/drivers/scsi/BusLogic.h
+++ b/drivers/scsi/BusLogic.h
@@ -237,10 +237,7 @@ enum BusLogic_BIOS_DiskGeometryTranslati
   Define a Boolean data type.
 */
 
-typedef enum {
-	false,
-	true
-} PACKED boolean;
+typedef bool boolean;
 
 /*
   Define a 10^18 Statistics Byte Counter data type.
diff --git a/include/linux/stddef.h b/include/linux/stddef.h
index b3a2cad..0382065 100644
--- a/include/linux/stddef.h
+++ b/include/linux/stddef.h
@@ -10,6 +10,11 @@ #else
 #define NULL ((void *)0)
 #endif
 
+enum {
+	false	= 0,
+	true	= 1
+};
+
 #undef offsetof
 #ifdef __compiler_offsetof
 #define offsetof(TYPE,MEMBER) __compiler_offsetof(TYPE,MEMBER)
diff --git a/include/linux/types.h b/include/linux/types.h
index 3f23566..406d4ae 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -33,6 +33,8 @@ typedef __kernel_clockid_t	clockid_t;
 typedef __kernel_mqd_t		mqd_t;
 
 #ifdef __KERNEL__
+typedef _Bool			bool;
+
 typedef __kernel_uid32_t	uid_t;
 typedef __kernel_gid32_t	gid_t;
 typedef __kernel_uid16_t        uid16_t;

Re: [RFC][PATCH] A generic boolean (version 6)

[ricknu-0]

Citerar Nicholas Miell <nmiell@comcast.net>:

> If _Bool does end up in the user-kernel ABI, be advised that validating
> them will be tricky ("b == true || b == false" or "!!b" won't work), and

Why would !!b not work?
I don't think it should end up in the ABI (at least, not yet). Just asking
because I'm curious. :)

> Nicholas Miell <nmiell@comcast.net>

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 6)

[Nicholas Miell]

On Thu, 2006-07-27 at 21:55 +0200, ricknu-0@student.ltu.se wrote:
> Citerar Nicholas Miell <nmiell@comcast.net>:
> 
> > If _Bool does end up in the user-kernel ABI, be advised that validating
> > them will be tricky ("b == true || b == false" or "!!b" won't work), and
> 
> Why would !!b not work?
> I don't think it should end up in the ABI (at least, not yet). Just asking
> because I'm curious. :)
> 

The compiler knows that "b = !!b;" is a no-op.

-- 
Nicholas Miell <nmiell@comcast.net>

Re: [RFC][PATCH] A generic boolean (version 6)

[ricknu-0]

Citerar Nicholas Miell <nmiell@comcast.net>:

> On Thu, 2006-07-27 at 21:55 +0200, ricknu-0@student.ltu.se wrote:
> > Citerar Nicholas Miell <nmiell@comcast.net>:
> > 
> > > If _Bool does end up in the user-kernel ABI, be advised that validating
> > > them will be tricky ("b == true || b == false" or "!!b" won't work), and
> > 
> > Why would !!b not work?
> > I don't think it should end up in the ABI (at least, not yet). Just asking
> > because I'm curious. :)
> > 
> 
> The compiler knows that "b = !!b;" is a no-op.

In what gcc version? Using 4.0.2 myself and got that if b equals 12 (using a
pointer to add the value to the boolean) then !!b equals 1.

/Richard Knutsson

Re: [RFC][PATCH] A generic boolean (version 6)

[Nicholas Miell]

On Fri, 2006-07-28 at 03:29 +0200, ricknu-0@student.ltu.se wrote:
> Citerar Nicholas Miell <nmiell@comcast.net>:
> 
> > On Thu, 2006-07-27 at 21:55 +0200, ricknu-0@student.ltu.se wrote:
> > > Citerar Nicholas Miell <nmiell@comcast.net>:
> > > 
> > > > If _Bool does end up in the user-kernel ABI, be advised that validating
> > > > them will be tricky ("b == true || b == false" or "!!b" won't work), and
> > > 
> > > Why would !!b not work?
> > > I don't think it should end up in the ABI (at least, not yet). Just asking
> > > because I'm curious. :)
> > > 
> > 
> > The compiler knows that "b = !!b;" is a no-op.
> 
> In what gcc version? Using 4.0.2 myself and got that if b equals 12 (using a
> pointer to add the value to the boolean) then !!b equals 1.

gcc version 4.1.1 20060525 (Red Hat 4.1.1-1) compiles:

#include <stdbool.h>
bool validBool(bool b) { return (b == true || b == false); }
bool normalizeBool(bool b) { return !!b; }

to:

validBool:
        movl    $1, %eax
        ret

normalizeBool:
        movzbl  %dil, %eax
        ret

-- 
Nicholas Miell <nmiell@comcast.net>

Re: [RFC][PATCH] A generic boolean (version 6)

[Alan Cox]

Ar Iau, 2006-07-27 am 13:13 -0700, ysgrifennodd Nicholas Miell:
> The compiler knows that "b = !!b;" is a no-op.

b = !!b isn't a no-op.

Try printf("%d", !!4);

Re: [RFC][PATCH] A generic boolean (version 6)

[Jan Engelhardt]

>I'm delighted that this favors "true, false and bool",
>over "TRUE, FALSE and various spellings of BOOLEAN".
>
>Fun stuff to do in the future:

Make people actually accept bool.
http://lkml.org/lkml/2006/5/1/123 was like a hit in the face wrt. bool.


PS: Can *anybody* tell me why lkml.org is so slow recently?


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean (version 6)

[Lars Noschinski]

* Alan Cox <alan@lxorguk.ukuu.org.uk> [2006-07-28 22:14]:
>Ar Iau, 2006-07-27 am 13:13 -0700, ysgrifennodd Nicholas Miell:
>> The compiler knows that "b = !!b;" is a no-op.
>
>b = !!b isn't a no-op.

For _Bool it should be:

>Try printf("%d", !!4);

printf("%d, %d", (_Bool)4, !!(_Bool)4);

prints "1, 1". From ISO/IEC 9899:1999:

When any scalar value is converted to _Bool, the result is 0 if the
value compares equal to 0; otherwise, the result is 1.


Greetings,
     Lars

Re: [RFC][PATCH] A generic boolean (version 6)

[Nicholas Miell]

On Fri, 2006-07-28 at 22:24 +0200, Lars Noschinski wrote:
> * Alan Cox <alan@lxorguk.ukuu.org.uk> [2006-07-28 22:14]:
> >Ar Iau, 2006-07-27 am 13:13 -0700, ysgrifennodd Nicholas Miell:
> >> The compiler knows that "b = !!b;" is a no-op.
> >
> >b = !!b isn't a no-op.
> 
> For _Bool it should be:
> 
> >Try printf("%d", !!4);
> 
> printf("%d, %d", (_Bool)4, !!(_Bool)4);
> 
> prints "1, 1". From ISO/IEC 9899:1999:
> 
> When any scalar value is converted to _Bool, the result is 0 if the
> value compares equal to 0; otherwise, the result is 1.
> 

We're not talking about scalar values converted to _Bool, we're talking
about the kernel getting a supposed _Bool from userspace which doesn't
actually contain 0 or 1. (i.e. as far as the kernel and/or gcc is
concerned, the scalar conversion has already taken place)

-- 
Nicholas Miell <nmiell@comcast.net>

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

>>>>> "Jeff" == Jeff Garzik <jeff@garzik.org> writes:

Jeff> ricknu-0@student.ltu.se wrote:
>> A first step to a generic boolean-type. The patch just introduce
>> the bool (in

Jeff> Since gcc supports boolean types and can optimize for such,
Jeff> introducing bool is IMO a good thing.

>> -Why would we want it?  -There is already some how are depending on
>> a "boolean"-type (like NTFS). Also, it will clearify functions who
>> returns a boolean from one returning a value, ex: bool it_is_ok();
>> char it_is_ok(); The first one is obvious what it is doing, the
>> secound might return some sort of status.

Jeff> A better reason is that there is intrinsic compiler support for
Jeff> booleans.

Well late to the dicussion, but I still want to point out that forcing
a boolean type of a different size upon existing kernel code is not
always a great idea and can have nasty side effects for struct
alignments. Not to mention that on some architectures, accessing a u1
is a lot slower than accessing an int. If a developer really wants to
use the smaller type he/she should do so explicitly being aware of the
impact.

The kernel is written in C, not C++ or Jave or some other broken
language and C doesn't have 'bool'. This patch falls under the
'typedefs considered evil' or typedef for the sake of typedef, if you
ask me.

Regards,
Jes

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Alan Cox wrote:
> Ar Gwe, 2006-08-04 am 10:03 -0400, ysgrifennodd Jes Sorensen:
>> alignments. Not to mention that on some architectures, accessing a u1
>> is a lot slower than accessing an int. If a developer really wants to
>> use the smaller type he/she should do so explicitly being aware of the
>> impact.
> 
> Which is just fine. Nobody at the moment is using the bool type because
> we don't have one. Nor is a C bool necessarily u1.

The proposed patch makes it u1 - if we end up with arch specific
defines, as the patch is proposing, developers won't know for sure what
the size is and will get alignment wrong. That is not fine.

If we really have to introduce a bool type, at least it has to be the
same size on all 32 bit archs and the same size on all 64 bit archs.

But again, the end result is we end up with yet another typedef for the
sake of introducing a typedef.

>> The kernel is written in C, not C++ or Jave or some other broken
>> language and C doesn't have 'bool'. 
> 
> Oh yes it does, as of C99 via stdbool.h. The only reason its not always
> "bool" is compatibility considerations. Welcome to the 21st century.

*Shiver*, I guess we'll need a machine that is PC2007 or whatever
compliant to run Linux next.

Jes

Re: [RFC][PATCH] A generic boolean

[Alan Cox]

Ar Gwe, 2006-08-04 am 10:03 -0400, ysgrifennodd Jes Sorensen:
> alignments. Not to mention that on some architectures, accessing a u1
> is a lot slower than accessing an int. If a developer really wants to
> use the smaller type he/she should do so explicitly being aware of the
> impact.

Which is just fine. Nobody at the moment is using the bool type because
we don't have one. Nor is a C bool necessarily u1.

> The kernel is written in C, not C++ or Jave or some other broken
> language and C doesn't have 'bool'. 

Oh yes it does, as of C99 via stdbool.h. The only reason its not always
"bool" is compatibility considerations. Welcome to the 21st century.

Alan

Re: [RFC][PATCH] A generic boolean

[Alan Cox]

Ar Gwe, 2006-08-04 am 16:35 +0200, ysgrifennodd Jes Sorensen:
> The proposed patch makes it u1 - if we end up with arch specific
> defines, as the patch is proposing, developers won't know for sure what
> the size is and will get alignment wrong. That is not fine.

The _Bool type is up to gcc implementation details.

> If we really have to introduce a bool type, at least it has to be the
> same size on all 32 bit archs and the same size on all 64 bit archs.

You don't use bool for talking to hardware, you use it for the most
efficient compiler behaviour when working with true/false values.

Alan

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Alan Cox wrote:
> Ar Gwe, 2006-08-04 am 16:35 +0200, ysgrifennodd Jes Sorensen:
>> The proposed patch makes it u1 - if we end up with arch specific
>> defines, as the patch is proposing, developers won't know for sure what
>> the size is and will get alignment wrong. That is not fine.
> 
> The _Bool type is up to gcc implementation details.

Which is even worse :(

>> If we really have to introduce a bool type, at least it has to be the
>> same size on all 32 bit archs and the same size on all 64 bit archs.
> 
> You don't use bool for talking to hardware, you use it for the most
> efficient compiler behaviour when working with true/false values.

Thats the problem, people will start putting them into structs, and
voila all alignment predictability has gone out the window.

Regards,
Jes

Re: [RFC][PATCH] A generic boolean

[Andreas Schwab]

Jes Sorensen <jes@sgi.com> writes:

> Alan Cox wrote:
>> Ar Gwe, 2006-08-04 am 16:35 +0200, ysgrifennodd Jes Sorensen:
>>> The proposed patch makes it u1 - if we end up with arch specific
>>> defines, as the patch is proposing, developers won't know for sure what
>>> the size is and will get alignment wrong. That is not fine.
>>
>> The _Bool type is up to gcc implementation details.
>
> Which is even worse :(

It's part of the ABI, just like any other C type.

>>> If we really have to introduce a bool type, at least it has to be the
>>> same size on all 32 bit archs and the same size on all 64 bit archs.
>>
>> You don't use bool for talking to hardware, you use it for the most
>> efficient compiler behaviour when working with true/false values.
>
> Thats the problem, people will start putting them into structs, and
> voila all alignment predictability has gone out the window.

Just like trying to predict the alignment of any other C type.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Andreas Schwab wrote:
> Jes Sorensen <jes@sgi.com> writes:
>> Thats the problem, people will start putting them into structs, and
>> voila all alignment predictability has gone out the window.
> 
> Just like trying to predict the alignment of any other C type.

Well in that case, could you list the size of it for all Linux archs
please? We know that today long is the only one that differs and that
m68k has horrible natural alignment rules for historical reasons, but
besides that it's pretty sane.

Just because something exists doesn't mean using it is a good thing.
Just like gcc supporting exceptions :)

Regards,
Jes

Re: [RFC][PATCH] A generic boolean

[Andreas Schwab]

Jes Sorensen <jes@sgi.com> writes:

> We know that today long is the only one that differs and that
> m68k has horrible natural alignment rules for historical reasons, but
> besides that it's pretty sane.

Try determining the alignment of u64 on i386.  You will be surprised.

#include <stdio.h>

typedef long long u64;
struct u64_s { u64 x; } x;

int main ()
{
  printf ("%d\n", __alignof__ (u64));
  printf ("%d\n", __alignof__ (x));
  return 0;
}

Btw, the iptables compat code was broken due to this.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Alan Cox wrote:
> Ar Gwe, 2006-08-04 am 17:58 +0200, ysgrifennodd Jes Sorensen:
>>> You don't use bool for talking to hardware, you use it for the most
>>> efficient compiler behaviour when working with true/false values.
>> Thats the problem, people will start putting them into structs, and
>> voila all alignment predictability has gone out the window.
> 
> Jes, try reading as well as writing. Given you even quoted "You don't
> use bool for talking to hardware" maybe you should read it.

Alan,

I did read, I never suggested putting it into structs describing
hardware registers.

> Structure alignment is generally a bad idea anyway because even array
> and word alignment are pretty variable between processors.

It's fairly common in drivers to layout things in effective ways in
structs relying on alignment. It can make a noticeable difference if
you get cacheline alignment wrong. For example in network drivers where
parts of the struct is used for receive and the other part is used for
transmit and the two parts can run in parallel on different CPUs.
Obviously one ends up using the aligned attribute, but the more one can
avoid adding those on in the struct the easier it is to maintain and
read.

Regards,
Jes

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Andreas Schwab wrote:
> Jes Sorensen <jes@sgi.com> writes:
> 
>> We know that today long is the only one that differs and that
>> m68k has horrible natural alignment rules for historical reasons, but
>> besides that it's pretty sane.
> 
> Try determining the alignment of u64 on i386.  You will be surprised.

If thats the case, then thats really scary :-( I'd claim it's a bug and
I am willing to be that iptables isn't the only place that is affected
or will be in the future.

Would a fix along the lines of this work?

typedef long long u64 __attribute__ ((aligned (8));

Cheers,
Jes

Re: [RFC][PATCH] A generic boolean

[Alan Cox]

Ar Gwe, 2006-08-04 am 17:58 +0200, ysgrifennodd Jes Sorensen:
> > You don't use bool for talking to hardware, you use it for the most
> > efficient compiler behaviour when working with true/false values.
> 
> Thats the problem, people will start putting them into structs, and
> voila all alignment predictability has gone out the window.

Jes, try reading as well as writing. Given you even quoted "You don't
use bool for talking to hardware" maybe you should read it.

Structure alignment is generally a bad idea anyway because even array
and word alignment are pretty variable between processors.

Re: [RFC][PATCH] A generic boolean

[Andreas Schwab]

Jes Sorensen <jes@sgi.com> writes:

> Andreas Schwab wrote:
>> Jes Sorensen <jes@sgi.com> writes:
>>
>>> We know that today long is the only one that differs and that
>>> m68k has horrible natural alignment rules for historical reasons, but
>>> besides that it's pretty sane.
>>
>> Try determining the alignment of u64 on i386.  You will be surprised.
>
> If thats the case, then thats really scary :-(

That's how the ABI is defined.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Andreas Schwab wrote:
> Jes Sorensen <jes@sgi.com> writes:
> 
>> Andreas Schwab wrote:
>>> Jes Sorensen <jes@sgi.com> writes:
>>>
>>>> We know that today long is the only one that differs and that
>>>> m68k has horrible natural alignment rules for historical reasons, but
>>>> besides that it's pretty sane.
>>> Try determining the alignment of u64 on i386.  You will be surprised.
>> If thats the case, then thats really scary :-(
> 
> That's how the ABI is defined.

That the ABI for long long or the ABI for uint64_t? Given that u64 is a
Linux thing, it ought to be ok to do the alignment the right way within
the kernel.

Cheers,
Jes

Re: [RFC][PATCH] A generic boolean

[H. Peter Anvin]

Jes Sorensen wrote:
>
>> That's how the ABI is defined.
> 
> That the ABI for long long or the ABI for uint64_t? Given that u64 is a
> Linux thing, it ought to be ok to do the alignment the right way within
> the kernel.
> 

And what will break if you make that switch?

	-hpa

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

H. Peter Anvin wrote:
> Jes Sorensen wrote:
>>
>>> That's how the ABI is defined.
>>
>> That the ABI for long long or the ABI for uint64_t? Given that u64 is a
>> Linux thing, it ought to be ok to do the alignment the right way within
>> the kernel.
>>
> 
> And what will break if you make that switch?

If we are lucky, some binary only modules? :-)

But you're right, it may just have to be documented as one of those
nasty issues to watch out for.

Cheers,
Jes

Re: [RFC][PATCH] A generic boolean

[H. Peter Anvin]

Jes Sorensen wrote:
>>
>> And what will break if you make that switch?
> 
> If we are lucky, some binary only modules? :-)
> 
> But you're right, it may just have to be documented as one of those
> nasty issues to watch out for.
> 

What is really poisonous is structures which get padded when all the 
members are naturally aligned.  Unfortunately gcc produces really crap 
code with __attribute__((packed)) on some architectures, so just using 
that isn't a good solution.  On the other hand, non-AEABI ARM sometimes 
needs it!

For the lack of a __attribute__((nopad)) that would throw a warning or 
error on excessive padding, I fear that our best option is an __abi 
annotation which would enforce certain rules using sparse, and 
presumably provide __attribute__((packed)) on ARM:

- All padding must be explicit.
- All members must be naturally aligned.
- No unportable constructs, like non-int-sized bitfields.

	-hpa

Re: [RFC][PATCH] A generic boolean

[Jan Engelhardt]

>typedef long long u64;

That's s64.

>int main ()

Not ANSI C conformant.

SCNR.


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean

[Jan Engelhardt]

> We know that today long is the only one that differs

For "modern architectures" maybe, but older compilers (like Turbo C 
compiler (1990)), int is a 16 bit quantity, and therefore does differ, from 
today's implementations at least.


Jan Engelhardt
-- 

Re: [RFC][PATCH] A generic boolean

[Andreas Schwab]

Jan Engelhardt <jengelh@linux01.gwdg.de> writes:

>>typedef long long u64;
>
> That's s64.

Right.

>>int main ()
>
> Not ANSI C conformant.

Wrong.

Andreas.

-- 
Andreas Schwab, SuSE Labs, schwab@suse.de
SuSE Linux Products GmbH, Maxfeldstraße 5, 90409 Nürnberg, Germany
PGP key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [RFC][PATCH] A generic boolean

[Jes Sorensen]

Jan Engelhardt wrote:
>> We know that today long is the only one that differs
> 
> For "modern architectures" maybe, but older compilers (like Turbo C 
> compiler (1990)), int is a 16 bit quantity, and therefore does differ, from 
> today's implementations at least.

Excuse me, but this conversation was about compiling the Linux kernel.
What non GCC compilers do is irrelevant to this.

Jes