LinuxLists.cc - [patch 1/1] consolidate TRUE and FALSE

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

Hi,

On Thu, 16 Mar 2006, [email protected] wrote:
> From: Andrew Morton <[email protected]>
>
> We have no less than 65 implementations of TRUE and FALSE in the tree, so the
> inevitable happened:
[snip
>
> The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> the private versions.

Great! That has really been long overdue.

> Various places are doing things like
>
> typedef {
> FALSE,
> TRUE
> } my_fave_name_for_a_bool;
>
> These are converted to
>
> typedef int my_fave_name_for_a_bool;

Given that the kernel now requires gcc 3.2 or later, that already includes
a native boolean type (_Bool)? Why not use that instead of "int"?

Also <stdbool.h> contains:

#define bool _Bool
#define true 1
#define false 0

So we could take the bool rather than _Bool, too given _Bool looks
rather ugly...

We could even go as far as removing all those typedefs and replacing all
their uses with the native boolean type (or the "bool" define or
whatever). Seems like the perfect janitorial task to me. (-;

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

2006-03-16 10:36:58

by Arjan van de Ven

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 2006-03-16 at 02:01 -0800, [email protected] wrote:
> From: Andrew Morton <[email protected]>
>
> We have no less than 65 implementations of TRUE and FALSE in the tree, so the
> inevitable happened:

makes me wonder how many of these users should actually just use 0 and 1
directly (but that's a long during janitor task I guess)

2006-03-16 10:45:18

by Andrew Morton

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

Anton Altaparmakov <[email protected]> wrote:
>
> > Various places are doing things like
> >
> > typedef {
> > FALSE,
> > TRUE
> > } my_fave_name_for_a_bool;
> >
> > These are converted to
> >
> > typedef int my_fave_name_for_a_bool;
>
> Given that the kernel now requires gcc 3.2 or later, that already includes
> a native boolean type (_Bool)?

It does?

Is it any good?

bix:/home/akpm> cat t.c
void foo()
{
_Bool b = 1;

b += 2;
}
bix:/home/akpm> gcc -O -Wall -c t.c
bix:/home/akpm>

Sigh.

> Why not use that instead of "int"?

That'd be a separate patch ;)

> Also <stdbool.h> contains:
>
> #define bool _Bool
> #define true 1
> #define false 0
>
> So we could take the bool rather than _Bool, too given _Bool looks
> rather ugly...

We have a couple of private bools and a couple of private 'true's and
`false's so I guess it'd be a simple patch. I wonder if it would have any
surprising side-effects.

(I think using `bool' is a good thing - it makes the code more readable.
It's a shame the compiler's handling of it is so useless).

2006-03-16 16:01:33

by Christoph Hellwig

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

> The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> the private versions.
>
> The patch also kills off a few private implementations of NULL.

NACK. Just kill them all and use 0/1

2006-03-16 16:27:55

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006 16:01:30 +0000 Christoph Hellwig wrote:

> > The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> > the private versions.
> >
> > The patch also kills off a few private implementations of NULL.
>
> NACK. Just kill them all and use 0/1

nah, the only place that using symbolic names for true and false
is a problem is when someone #defines or enums them bassackwards.

---
~Randy

2006-03-16 16:30:08

by Christoph Hellwig

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 08:29:51AM -0800, Randy.Dunlap wrote:
> On Thu, 16 Mar 2006 16:01:30 +0000 Christoph Hellwig wrote:
>
> > > The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> > > the private versions.
> > >
> > > The patch also kills off a few private implementations of NULL.
> >
> > NACK. Just kill them all and use 0/1
>
> nah, the only place that using symbolic names for true and false
> is a problem is when someone #defines or enums them bassackwards.

it makes the code longer and harder to read. there's a reason the core
code doesn't use it, and the periphal code should do the same.

2006-03-16 16:34:56

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006 16:30:01 +0000 Christoph Hellwig wrote:

> On Thu, Mar 16, 2006 at 08:29:51AM -0800, Randy.Dunlap wrote:
> > On Thu, 16 Mar 2006 16:01:30 +0000 Christoph Hellwig wrote:
> >
> > > > The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> > > > the private versions.
> > > >
> > > > The patch also kills off a few private implementations of NULL.
> > >
> > > NACK. Just kill them all and use 0/1
> >
> > nah, the only place that using symbolic names for true and false
> > is a problem is when someone #defines or enums them bassackwards.
>
> it makes the code longer and harder to read. there's a reason the core
> code doesn't use it, and the periphal code should do the same.

in your opinion.

---
~Randy

2006-03-16 16:36:26

by Christoph Hellwig

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

> > it makes the code longer and harder to read. there's a reason the core
> > code doesn't use it, and the periphal code should do the same.
>
> in your opinion.

of course. but that it's not used in core code implies this opinion is
widely shared.

2006-03-16 16:40:37

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006 16:36:21 +0000 Christoph Hellwig wrote:

> > > it makes the code longer and harder to read. there's a reason the core
> > > code doesn't use it, and the periphal code should do the same.
> >
> > in your opinion.
>
> of course. but that it's not used in core code implies this opinion is
> widely shared.

well I'll be happy to leave it up to Andrew to work out. :)

---
~Randy

2006-03-16 16:40:51

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006, Christoph Hellwig wrote:
> On Thu, Mar 16, 2006 at 08:29:51AM -0800, Randy.Dunlap wrote:
> > On Thu, 16 Mar 2006 16:01:30 +0000 Christoph Hellwig wrote:
> >
> > > > The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> > > > the private versions.
> > > >
> > > > The patch also kills off a few private implementations of NULL.
> > >
> > > NACK. Just kill them all and use 0/1

NACK to your NACK. (-;

> > nah, the only place that using symbolic names for true and false
> > is a problem is when someone #defines or enums them bassackwards.
>
> it makes the code longer and harder to read.

I could not disagree more. It does _exactly_ the opposite. It makes the
code much easier to read but obviously you perceive it differently... I
guess it is very much a matter of taste rather than anything else.

For me, black and white is much simpler than shades of grey and whilst you
can emulate black and white with shades of grey it is still more effort to
parse than true black and white.

Certainly I refuse to have my booleans (in the kernel or anywhere else)
converted to 0 and 1 but I am all for unifying to a single boolean type
across the kernel, so this patch gets a big ACK from me.

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

2006-03-16 16:42:48

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006, Christoph Hellwig wrote:
> > > it makes the code longer and harder to read. there's a reason the core
> > > code doesn't use it, and the periphal code should do the same.
> >
> > in your opinion.
>
> of course. but that it's not used in core code implies this opinion is
> widely shared.

Again, in your opinion. To me it is a simple consequence of there not
being a boolean type in the kernel so you cannot use it in the core code.
Once there is such a type I would imagine users will appear in the core
code over time.

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

2006-03-16 16:49:36

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 02:01:28AM -0800, [email protected] wrote:
>
> From: Andrew Morton <[email protected]>
>
> We have no less than 65 implementations of TRUE and FALSE in the tree, so the
> inevitable happened:
>
> In file included from drivers/pci/hotplug/ibmphp_core.c:40:
> drivers/pci/hotplug/ibmphp.h:409:1: warning: "FALSE" redefined
> In file included from include/acpi/acpi.h:55,
> from drivers/pci/hotplug/pci_hotplug.h:187,
> from drivers/pci/hotplug/ibmphp.h:33,
> from drivers/pci/hotplug/ibmphp_core.c:40:
> include/acpi/actypes.h:336:1: warning: this is the location of the previous definition
>
> The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> the private versions.

NAK. Simply remove those and be done with that. It's bad style and we
certainly don't need to propagate that lossage.

..oO[and no, #define BEGIN { is also not kernel.h fodder]

2006-03-16 16:50:49

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 04:42:29PM +0000, Anton Altaparmakov wrote:
> Again, in your opinion. To me it is a simple consequence of there not
> being a boolean type in the kernel so you cannot use it in the core code.
> Once there is such a type I would imagine users will appear in the core
> code over time.

And that's supposed to be an argument in favour of that crap?

2006-03-16 17:07:41

by Greg KH

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 02:01:28AM -0800, [email protected] wrote:
>
> From: Andrew Morton <[email protected]>
>
> We have no less than 65 implementations of TRUE and FALSE in the tree, so the
> inevitable happened:
>
> In file included from drivers/pci/hotplug/ibmphp_core.c:40:
> drivers/pci/hotplug/ibmphp.h:409:1: warning: "FALSE" redefined
> In file included from include/acpi/acpi.h:55,
> from drivers/pci/hotplug/pci_hotplug.h:187,
> from drivers/pci/hotplug/ibmphp.h:33,
> from drivers/pci/hotplug/ibmphp_core.c:40:
> include/acpi/actypes.h:336:1: warning: this is the location of the previous definition

I have a patch in my queue that fixes this, in the ibmphp driver.

The acpi stuff should still be fixed though...

thanks,

greg k-h

2006-03-16 17:09:52

by Xavier Bestel

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 2006-03-16 at 17:01, Christoph Hellwig wrote:
> > The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> > the private versions.
> >
> > The patch also kills off a few private implementations of NULL.
>
> NACK. Just kill them all and use 0/1

I'd say use var == 0 and var != 0 (or var and !var).
While FALSE is obviously 0, TRUE isn't only 1.

Xav

2006-03-16 17:11:27

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 09:05:05AM -0800, Ray Lee wrote:
> > And that's supposed to be an argument in favour of that crap?
>
> So an honest, if stupid, question: How is the TRUE/FALSE stuff any
> different than the case of using a NULL when assigning a zero to a
> pointer is explicitly required to have the same effect? They both
> seem to further the goal of better self-documenting code.

NULL is idiomatic in C. TRUE and FALSE are definitely not. Again,
if you want Bournegol, you know where to find it. Grab the v7 sh
and enjoy "self-documenting" code.

2006-03-16 17:13:33

by Greg KH

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 09:07:35AM -0800, Greg KH wrote:
> On Thu, Mar 16, 2006 at 02:01:28AM -0800, [email protected] wrote:
> >
> > From: Andrew Morton <[email protected]>
> >
> > We have no less than 65 implementations of TRUE and FALSE in the tree, so the
> > inevitable happened:
> >
> > In file included from drivers/pci/hotplug/ibmphp_core.c:40:
> > drivers/pci/hotplug/ibmphp.h:409:1: warning: "FALSE" redefined
> > In file included from include/acpi/acpi.h:55,
> > from drivers/pci/hotplug/pci_hotplug.h:187,
> > from drivers/pci/hotplug/ibmphp.h:33,
> > from drivers/pci/hotplug/ibmphp_core.c:40:
> > include/acpi/actypes.h:336:1: warning: this is the location of the previous definition
>
> I have a patch in my queue that fixes this, in the ibmphp driver.

Pushed out into my tree now, if you want to update your copy.

thanks,

greg k-h

2006-03-16 17:41:33

by Sam Ravnborg

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 04:30:01PM +0000, Christoph Hellwig wrote:

> it makes the code longer and harder to read. there's a reason the core
> code doesn't use it, and the periphal code should do the same.
Let see:
0 is an interger zero. It is not a null pointer.
1 is an interger one
NULL is an null pointer

We agree so far?

And you say:
0 is also false
1 is also true

But others say:
0 is not false, only false is false.
1 is not true, only true is true.
Thats more readable. No magic = 1; assignments or return 1; things.

Then we will have the case where a 1 is transformed to a true, and
likewise a 0 to a false. But thats just normal type conversion.

I assume that when you are not used to see 'bool', 'true' and 'false'
then they hurt the eye, but when used to it it looks natural.

Sam

2006-03-16 18:00:53

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 06:41:12PM +0100, Sam Ravnborg wrote:
> I assume that when you are not used to see 'bool', 'true' and 'false'
> then they hurt the eye, but when used to it it looks natural.

Five words: kernel is written in C.

Not in Pascal. Not in C++. Not in Algol. "When used to (something
non-idiomatic in C) it becomes natural" is not a valid argument.

2006-03-16 18:10:22

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006 18:00:47 +0000 Al Viro wrote:

> On Thu, Mar 16, 2006 at 06:41:12PM +0100, Sam Ravnborg wrote:
> > I assume that when you are not used to see 'bool', 'true' and 'false'
> > then they hurt the eye, but when used to it it looks natural.
>
> Five words: kernel is written in C.
>
> Not in Pascal. Not in C++. Not in Algol. "When used to (something
> non-idiomatic in C) it becomes natural" is not a valid argument.

C (C99) now includes booleans. Are we stuck pre-C99?

---
~Randy

2006-03-16 18:51:30

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006, Al Viro wrote:
> On Thu, Mar 16, 2006 at 04:42:29PM +0000, Anton Altaparmakov wrote:
> > Again, in your opinion. To me it is a simple consequence of there not
> > being a boolean type in the kernel so you cannot use it in the core code.
> > Once there is such a type I would imagine users will appear in the core
> > code over time.
>
> And that's supposed to be an argument in favour of that crap?

No, it was an alternative interpretation to Christoph's statement that the
non-use of booleans was due to them being crap. So I said that you can't
use what you don't have and I imagine the majority of people do not want
these pointless flamewars so no-one has so far bothered to introduce a
generic bollean type in the kernel...

Andy why is it crap? For a start it is valid C (well, ok C99, but even
you are fond of a lot of C99 features like the initializers) and second
it makes code more readable and makes meaning of functions a lot more
obvious, e.g reading:

"extern bool foo_is_compressed(bar);"

in a header file makes it damn obvious that if foo_is_compressed() returns
"true" then "bar" is compressed and if it returns "false" then "bar" is
not compressed.

If you see:

"extern int foo_is_compressed(bar);"

You have to go find the .c file containing the function and read it to
figure out what the heck the return value means/is. And don't tell me you
can rely on it returning 0 for not-compressed and 1 for compressed. For a
start it could return < 0 for error and a myriad of other things.
Especially in the kernel core the functions like that seem to randomly
return 0 for true and 1 for false and worse. But even if they were
consistent you could never rule out the < 0 for error case unless you read
the function itself.

So I beg to differ with your opinion of boolean type being crap.

As you might have noticed already, I think booleans have their place and
make code much more readable when used appropriately. I have only been
programming for the last 24 years and I like booleans and no-one is going
to get me to think otherwise after such a long time...

I am happy to agree that we disagree. Just stay away with s/true/1/
and s/false/0/ conversions from fs/ntfs/ please and I will shut up... (-:

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

2006-03-16 18:50:12

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 10:12:20AM -0800, Randy.Dunlap wrote:
> On Thu, 16 Mar 2006 18:00:47 +0000 Al Viro wrote:
>
> > On Thu, Mar 16, 2006 at 06:41:12PM +0100, Sam Ravnborg wrote:
> > > I assume that when you are not used to see 'bool', 'true' and 'false'
> > > then they hurt the eye, but when used to it it looks natural.
> >
> > Five words: kernel is written in C.
> >
> > Not in Pascal. Not in C++. Not in Algol. "When used to (something
> > non-idiomatic in C) it becomes natural" is not a valid argument.
>
> C (C99) now includes booleans. Are we stuck pre-C99?

TRUE and FALSE are not those. Your point is...?

2006-03-16 18:52:43

by Sam Ravnborg

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, Mar 16, 2006 at 06:00:47PM +0000, Al Viro wrote:
> On Thu, Mar 16, 2006 at 06:41:12PM +0100, Sam Ravnborg wrote:
> > I assume that when you are not used to see 'bool', 'true' and 'false'
> > then they hurt the eye, but when used to it it looks natural.
>
> Five words: kernel is written in C.
Yep - and bool, true and false are part of 'C' - C99.
>
> Not in Pascal. Not in C++. Not in Algol. "When used to (something
> non-idiomatic in C) it becomes natural" is not a valid argument.
The translation of idiomatic in my dictionary makes bool, true, false
far more idiomatic in the C than NULL.
But history has thaugth us to accept NULL with screaming UPPERCASE
where a null would be idiomatic C.

Sam

2006-03-16 18:53:54

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006, Anton Altaparmakov wrote:

> On Thu, 16 Mar 2006, Al Viro wrote:
> > On Thu, Mar 16, 2006 at 04:42:29PM +0000, Anton Altaparmakov wrote:
> > > Again, in your opinion. To me it is a simple consequence of there not
> > > being a boolean type in the kernel so you cannot use it in the core code.
> > > Once there is such a type I would imagine users will appear in the core
> > > code over time.
> >
> > And that's supposed to be an argument in favour of that crap?
>
> No, it was an alternative interpretation to Christoph's statement that the
> non-use of booleans was due to them being crap. So I said that you can't
> use what you don't have and I imagine the majority of people do not want
> these pointless flamewars so no-one has so far bothered to introduce a
> generic bollean type in the kernel...
>
> Andy why is it crap? For a start it is valid C (well, ok C99, but even

s/Andy/And/...

> you are fond of a lot of C99 features like the initializers) and second
> it makes code more readable and makes meaning of functions a lot more
> obvious, e.g reading:
>
> "extern bool foo_is_compressed(bar);"
>
> in a header file makes it damn obvious that if foo_is_compressed() returns
> "true" then "bar" is compressed and if it returns "false" then "bar" is
> not compressed.
>
> If you see:
>
> "extern int foo_is_compressed(bar);"
>
> You have to go find the .c file containing the function and read it to
> figure out what the heck the return value means/is. And don't tell me you
> can rely on it returning 0 for not-compressed and 1 for compressed. For a
> start it could return < 0 for error and a myriad of other things.
> Especially in the kernel core the functions like that seem to randomly
> return 0 for true and 1 for false and worse. But even if they were
> consistent you could never rule out the < 0 for error case unless you read
> the function itself.
>
> So I beg to differ with your opinion of boolean type being crap.
>
> As you might have noticed already, I think booleans have their place and
> make code much more readable when used appropriately. I have only been
> programming for the last 24 years and I like booleans and no-one is going
> to get me to think otherwise after such a long time...
>
> I am happy to agree that we disagree. Just stay away with s/true/1/
> and s/false/0/ conversions from fs/ntfs/ please and I will shut up... (-:
>
> Best regards,
>
> Anton
>

Best regards,

Anton
--
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

2006-03-16 18:56:58

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006 18:49:46 +0000 Al Viro wrote:

> On Thu, Mar 16, 2006 at 10:12:20AM -0800, Randy.Dunlap wrote:
> > On Thu, 16 Mar 2006 18:00:47 +0000 Al Viro wrote:
> >
> > > On Thu, Mar 16, 2006 at 06:41:12PM +0100, Sam Ravnborg wrote:
> > > > I assume that when you are not used to see 'bool', 'true' and 'false'
> > > > then they hurt the eye, but when used to it it looks natural.
> > >
> > > Five words: kernel is written in C.
> > >
> > > Not in Pascal. Not in C++. Not in Algol. "When used to (something
> > > non-idiomatic in C) it becomes natural" is not a valid argument.
> >
> > C (C99) now includes booleans. Are we stuck pre-C99?
>
> TRUE and FALSE are not those. Your point is...?

Correct. So it would be better if they were spelled 'true' and 'false',
as in the C99 spec?

---
~Randy

2006-03-16 19:55:18

by Nicholas Miell

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 2006-03-16 at 02:42 -0800, Andrew Morton wrote:
> Anton Altaparmakov <[email protected]> wrote:
> >
> > > Various places are doing things like
> > >
> > > typedef {
> > > FALSE,
> > > TRUE
> > > } my_fave_name_for_a_bool;
> > >
> > > These are converted to
> > >
> > > typedef int my_fave_name_for_a_bool;
> >
> > Given that the kernel now requires gcc 3.2 or later, that already includes
> > a native boolean type (_Bool)?
>
> It does?
>
> Is it any good?
>
> bix:/home/akpm> cat t.c
> void foo()
> {
> _Bool b = 1;
>
> b += 2;
> }
> bix:/home/akpm> gcc -O -Wall -c t.c
> bix:/home/akpm>
>
> Sigh.
>

If you were to read the value of b after "b += 2", you'd find that b is
still 1.

Also note that _Bool is a byte, so anything that exposed its own custom
boolean type to userspace needs to be carefully updated.

--
Nicholas Miell <[email protected]>

2006-03-16 19:55:43

by Jan Engelhardt

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

>
>Is it any good?
>
Depends.

20:51 shanghai:/dev/shm > cat bool.c
#include <stdio.h>

int main(void) {
_Bool x = 0;
x += 2;
printf("%d\n", x);
}
20:51 shanghai:/dev/shm > gcc bool.c && ./a.out
1

It can't "overflow". If that's good or not I can't tell, and I can only
imagine an artifical scenario where it affects things:

void do_something_wrong(int *x) {
*x += 2;
}

int x = 0;
do_something_wrong(&x);
if(x & 1)
printf("x & 1");

>From this one, we would expect nothing to be printed. But if x was a _Bool
x (and _Bool *x, respectively), something would be printed.

Jan Engelhardt
--

2006-03-16 19:59:26

by Jan Engelhardt

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

>> of course. but that it's not used in core code implies this opinion is
>> widely shared.
>
>[...] To me it is a simple consequence of there not
>being a boolean type in the kernel so you cannot use it in the core code.

typedef bool int;

And then happily use if(EXPR) and if(!EXPR) instead of if(EXPR == TRUE) or
if(EXPR == FALSE). :-)

But typedeffing it to int (or unsigned char, if someone likes that
for space optimization) does not "catch overflows" (see another lkml
mail from me) as _Bool would.

Jan Engelhardt
--

2006-03-16 21:17:41

by Valdis Klētnieks

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

On Thu, 16 Mar 2006 08:29:51 PST, "Randy.Dunlap" said:

> nah, the only place that using symbolic names for true and false
> is a problem is when someone #defines or enums them bassackwards.

Or goes off the deep end and does something like:

enum bool = {false, true, unknown};

http://thedailywtf.com/forums/47844/ShowPost.aspx
http://thedailywtf.com/forums/61792/ShowPost.aspx

Gaak. Unfortunately, they might someday try to write kernel code. ;)

Attachments:

(No filename) (228.00 B)

2006-03-16 21:29:05

by Joshua Hudson

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

>
> Or goes off the deep end and does something like:
>
> enum bool = {false, true, unknown};

Sounds like some of my AI code.

2006-03-16 21:29:33

by Andrew Morton

[permalink] [raw]

Subject: Re: [patch 1/1] consolidate TRUE and FALSE

Al Viro <[email protected]> wrote:
>
> On Thu, Mar 16, 2006 at 02:01:28AM -0800, [email protected] wrote:
> >
> > From: Andrew Morton <[email protected]>
> >
> > We have no less than 65 implementations of TRUE and FALSE in the tree, so the
> > inevitable happened:
> >
> > In file included from drivers/pci/hotplug/ibmphp_core.c:40:
> > drivers/pci/hotplug/ibmphp.h:409:1: warning: "FALSE" redefined
> > In file included from include/acpi/acpi.h:55,
> > from drivers/pci/hotplug/pci_hotplug.h:187,
> > from drivers/pci/hotplug/ibmphp.h:33,
> > from drivers/pci/hotplug/ibmphp_core.c:40:
> > include/acpi/actypes.h:336:1: warning: this is the location of the previous definition
> >
> > The patch implements TRUE and FALSE in include/linux/kernel.h and removes all
> > the private versions.
>
> NAK. Simply remove those and be done with that. It's bad style and we
> certainly don't need to propagate that lossage.

No, it is not bad style.

It is bad that the C design failed to distinguish between booleans and
scalars. These are quite different concepts and it is a gruesome kludge to
go and use a scalar when a boolean is what was intended.

There are readability (and hence maintainability) advantages in using
boolean types for boolean variables.

Yes, the use of scalars in this manner is an (ancient, old-fashioned) C
idiom. But it is one which was forced on us by shortcomings in the
language. Times change. Just because something is idiomatic doesn't mean
that it is either good or right. If C had had proper boolean support back
in the old days then using scalars as booleans would be frowned upon.

C99 does have boolean support, so the proper thing to do is to start
using it - implement stdbool.h, fix up fallout, start fixing subsystems.
Given that, and as Greg has fixed up this particular build error I'll drop
the patch.

(It's a shame that the compiler doesn't preperly enforce boolean
typechecking, but sparse could do that).

2006-03-16 22:35:42