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

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

[Bodo Eggert]

Jes Sorensen <jes@sgi.com> wrote:
> Andreas Schwab wrote:

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

It's not a bug, since 64 bit integers require only 4-byte-alignment if
addressed by the CPU/FPU. You may benefit from cacheline effects if you
align to 64 bits, but not on i[34]86 (and those define the ABI).

If you want 64-bit-alignment, you can use -malign-double.
-- 
Ich danke GMX dafür, die Verwendung meiner Adressen mittels per SPF
verbreiteten Lügen zu sabotieren.

http://david.woodhou.se/why-not-spf.html

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