All of lore.kernel.org
 help / color / mirror / Atom feed
* [U-Boot] [PATCH] Introduce a global bool type
@ 2013-01-07 22:10 York Sun
  2013-01-07 22:29 ` Scott Wood
                   ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: York Sun @ 2013-01-07 22:10 UTC (permalink / raw)
  To: u-boot

'bool' is defined in random places. This patch consolidates them into a
single typedef.

Signed-off-by: York Sun <yorksun@freescale.com>
---
 arch/blackfin/include/asm/posix_types.h |    3 ---
 board/Marvell/include/core.h            |    5 -----
 drivers/mtd/nand/mxc_nand.c             |    2 --
 drivers/usb/musb-new/linux-compat.h     |    2 --
 include/galileo/core.h                  |    5 -----
 include/linux/types.h                   |    2 ++
 include/xyzModem.h                      |    5 -----
 7 files changed, 2 insertions(+), 22 deletions(-)

diff --git a/arch/blackfin/include/asm/posix_types.h b/arch/blackfin/include/asm/posix_types.h
index 000ffe5..1f28b36 100644
--- a/arch/blackfin/include/asm/posix_types.h
+++ b/arch/blackfin/include/asm/posix_types.h
@@ -61,9 +61,6 @@ typedef unsigned int __kernel_gid32_t;
 typedef unsigned short __kernel_old_uid_t;
 typedef unsigned short __kernel_old_gid_t;
 
-#define BOOL_WAS_DEFINED
-typedef enum { false = 0, true = 1 } bool;
-
 #ifdef __GNUC__
 typedef long long __kernel_loff_t;
 #endif
diff --git a/board/Marvell/include/core.h b/board/Marvell/include/core.h
index c413439..3119d0a 100644
--- a/board/Marvell/include/core.h
+++ b/board/Marvell/include/core.h
@@ -91,11 +91,6 @@ extern unsigned int INTERNAL_REG_BASE_ADDR;
 #define _1G		0x40000000
 #define _2G		0x80000000
 
-#ifndef	BOOL_WAS_DEFINED
-#define BOOL_WAS_DEFINED
-typedef enum _bool{false,true} bool;
-#endif
-
 /* Little to Big endian conversion macros */
 
 #ifdef LE /* Little Endian */
diff --git a/drivers/mtd/nand/mxc_nand.c b/drivers/mtd/nand/mxc_nand.c
index d0ded48..04836c0 100644
--- a/drivers/mtd/nand/mxc_nand.c
+++ b/drivers/mtd/nand/mxc_nand.c
@@ -29,8 +29,6 @@
 
 #define DRIVER_NAME "mxc_nand"
 
-typedef enum {false, true} bool;
-
 struct mxc_nand_host {
 	struct mtd_info			mtd;
 	struct nand_chip		*nand;
diff --git a/drivers/usb/musb-new/linux-compat.h b/drivers/usb/musb-new/linux-compat.h
index 5c126ef..72c8c2b 100644
--- a/drivers/usb/musb-new/linux-compat.h
+++ b/drivers/usb/musb-new/linux-compat.h
@@ -12,8 +12,6 @@
 #define __iomem
 #define __deprecated
 
-typedef enum { false = 0, true = 1 } bool;
-
 struct unused {};
 typedef struct unused unused_t;
 
diff --git a/include/galileo/core.h b/include/galileo/core.h
index c277509..faf4962 100644
--- a/include/galileo/core.h
+++ b/include/galileo/core.h
@@ -110,11 +110,6 @@ extern unsigned int INTERNAL_REG_BASE_ADDR;
 #define _1G             0x40000000
 #define _2G             0x80000000
 
-#ifndef	BOOL_WAS_DEFINED
-#define BOOL_WAS_DEFINED
-typedef enum _bool{false,true} bool;
-#endif
-
 /* Little to Big endian conversion macros */
 
 #ifdef LE /* Little Endian */
diff --git a/include/linux/types.h b/include/linux/types.h
index 1b0b4a4..b359c33 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -113,6 +113,8 @@ typedef		__u64		u_int64_t;
 typedef		__s64		int64_t;
 #endif
 
+typedef _Bool bool;
+
 #endif /* __KERNEL_STRICT_NAMES */
 
 /*
diff --git a/include/xyzModem.h b/include/xyzModem.h
index f437bbd..9723e73 100644
--- a/include/xyzModem.h
+++ b/include/xyzModem.h
@@ -97,11 +97,6 @@ typedef struct {
 #endif
 } connection_info_t;
 
-#ifndef	BOOL_WAS_DEFINED
-#define BOOL_WAS_DEFINED
-typedef unsigned int bool;
-#endif
-
 #define false 0
 #define true 1
 
-- 
1.7.9.5

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-07 22:10 [U-Boot] [PATCH] Introduce a global bool type York Sun
@ 2013-01-07 22:29 ` Scott Wood
  2013-01-07 22:32 ` [U-Boot] [u-boot-release] " Timur Tabi
  2013-01-07 22:39 ` [U-Boot] " Wolfgang Denk
  2 siblings, 0 replies; 25+ messages in thread
From: Scott Wood @ 2013-01-07 22:29 UTC (permalink / raw)
  To: u-boot

On 01/07/2013 04:10:28 PM, York Sun wrote:
> diff --git a/include/linux/types.h b/include/linux/types.h
> index 1b0b4a4..b359c33 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -113,6 +113,8 @@ typedef		__u64		u_int64_t;
>  typedef		__s64		int64_t;
>  #endif
> 
> +typedef _Bool bool;
> +
>  #endif /* __KERNEL_STRICT_NAMES */
> 
>  /*
> diff --git a/include/xyzModem.h b/include/xyzModem.h
> index f437bbd..9723e73 100644
> --- a/include/xyzModem.h
> +++ b/include/xyzModem.h
> @@ -97,11 +97,6 @@ typedef struct {
>  #endif
>  } connection_info_t;
> 
> -#ifndef	BOOL_WAS_DEFINED
> -#define BOOL_WAS_DEFINED
> -typedef unsigned int bool;
> -#endif
> -
>  #define false 0
>  #define true 1

Please also move the definition of true/false into a common header.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [u-boot-release] [PATCH] Introduce a global bool type
  2013-01-07 22:10 [U-Boot] [PATCH] Introduce a global bool type York Sun
  2013-01-07 22:29 ` Scott Wood
@ 2013-01-07 22:32 ` Timur Tabi
  2013-01-07 22:39 ` [U-Boot] " Wolfgang Denk
  2 siblings, 0 replies; 25+ messages in thread
From: Timur Tabi @ 2013-01-07 22:32 UTC (permalink / raw)
  To: u-boot

York Sun wrote:
> 'bool' is defined in random places. This patch consolidates them into a
> single typedef.

... and defines 'bool' in a completely different way, so it doesn't just
"consolidate" the definitions.

I would add a comment that says that _Bool was introduced in C99, so it
should be safe to use this new definition instead of a hand-coded enum.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-07 22:10 [U-Boot] [PATCH] Introduce a global bool type York Sun
  2013-01-07 22:29 ` Scott Wood
  2013-01-07 22:32 ` [U-Boot] [u-boot-release] " Timur Tabi
@ 2013-01-07 22:39 ` Wolfgang Denk
  2013-01-07 22:50   ` Scott Wood
                     ` (2 more replies)
  2 siblings, 3 replies; 25+ messages in thread
From: Wolfgang Denk @ 2013-01-07 22:39 UTC (permalink / raw)
  To: u-boot

Dear York Sun,

In message <1357596628-27501-1-git-send-email-yorksun@freescale.com> you wrote:
> 'bool' is defined in random places. This patch consolidates them into a
> single typedef.

Has this been actually compile tested?

...
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -113,6 +113,8 @@ typedef		__u64		u_int64_t;
>  typedef		__s64		int64_t;
>  #endif
>  
> +typedef _Bool bool;

And what exactly would "_Bool" be?

...
> --- a/include/xyzModem.h
> +++ b/include/xyzModem.h
> @@ -97,11 +97,6 @@ typedef struct {
>  #endif
>  } connection_info_t;
>  
> -#ifndef	BOOL_WAS_DEFINED
> -#define BOOL_WAS_DEFINED
> -typedef unsigned int bool;
> -#endif
> -
>  #define false 0
>  #define true 1

And don't these remaining definitions of "false" and "true" cause
nasty build errors somewhere?


This seems broken to me.  Can we rather try8 and get rid of all this
"bool" stuff instead?  It's just obfuscating the code...

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Perl itself is  usually  pretty  good  about  telling  you  what  you
shouldn't do. :-)     - Larry Wall in <11091@jpl-devvax.JPL.NASA.GOV>

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-07 22:39 ` [U-Boot] " Wolfgang Denk
@ 2013-01-07 22:50   ` Scott Wood
  2013-01-07 22:54   ` Måns Rullgård
  2013-01-08 16:51   ` Tabi Timur-B04825
  2 siblings, 0 replies; 25+ messages in thread
From: Scott Wood @ 2013-01-07 22:50 UTC (permalink / raw)
  To: u-boot

On 01/07/2013 04:39:42 PM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <1357596628-27501-1-git-send-email-yorksun@freescale.com>  
> you wrote:
> > 'bool' is defined in random places. This patch consolidates them  
> into a
> > single typedef.
> 
> Has this been actually compile tested?
> 
> ...
> > --- a/include/linux/types.h
> > +++ b/include/linux/types.h
> > @@ -113,6 +113,8 @@ typedef		__u64		u_int64_t;
> >  typedef		__s64		int64_t;
> >  #endif
> >
> > +typedef _Bool bool;
> 
> And what exactly would "_Bool" be?

It's a standard C99 type (as is bool, but _Bool comes directly from the  
compiler rather than from headers).

> ...
> > --- a/include/xyzModem.h
> > +++ b/include/xyzModem.h
> > @@ -97,11 +97,6 @@ typedef struct {
> >  #endif
> >  } connection_info_t;
> >
> > -#ifndef	BOOL_WAS_DEFINED
> > -#define BOOL_WAS_DEFINED
> > -typedef unsigned int bool;
> > -#endif
> > -
> >  #define false 0
> >  #define true 1
> 
> And don't these remaining definitions of "false" and "true" cause
> nasty build errors somewhere?

Yes, the definition of true/false needs to move along with the  
definition of bool.

> This seems broken to me.  Can we rather try8 and get rid of all this
> "bool" stuff instead?  It's just obfuscating the code...

That's obviously a matter of opinion (I think "bool" is clearer than  
"int"), but I'd like to point out that Linux's use of bool has been  
growing, and U-Boot often borrows code from Linux...

Also, FWIW the compiler will generally allocate only one byte for a  
built-in boolean, so there is a minor run-time benefit in some  
situations.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-07 22:39 ` [U-Boot] " Wolfgang Denk
  2013-01-07 22:50   ` Scott Wood
@ 2013-01-07 22:54   ` Måns Rullgård
  2013-01-08  6:25     ` Wolfgang Denk
  2013-01-08 16:51   ` Tabi Timur-B04825
  2 siblings, 1 reply; 25+ messages in thread
From: Måns Rullgård @ 2013-01-07 22:54 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk <wd@denx.de> writes:

> Dear York Sun,
>
> In message <1357596628-27501-1-git-send-email-yorksun@freescale.com> you wrote:
>> 'bool' is defined in random places. This patch consolidates them into a
>> single typedef.
>
> Has this been actually compile tested?
>
> ...
>> --- a/include/linux/types.h
>> +++ b/include/linux/types.h
>> @@ -113,6 +113,8 @@ typedef		__u64		u_int64_t;
>>  typedef		__s64		int64_t;
>>  #endif
>>  
>> +typedef _Bool bool;
>
> And what exactly would "_Bool" be?

_Bool is a C99 type (though I fail to imagine why).  If using this, one
might as well use the C99 header stdbool.h providing macros for 'bool',
'true' and 'false' instead of this.

> Can we rather try and get rid of all this "bool" stuff instead?  It's
> just obfuscating the code...

Indeed.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-07 22:54   ` Måns Rullgård
@ 2013-01-08  6:25     ` Wolfgang Denk
  0 siblings, 0 replies; 25+ messages in thread
From: Wolfgang Denk @ 2013-01-08  6:25 UTC (permalink / raw)
  To: u-boot

Dear M?ns Rullg?rd,

In message <yw1xpq1g4na0.fsf@unicorn.mansr.com> you wrote:
> 
> >> +typedef _Bool bool;
> >
> > And what exactly would "_Bool" be?
> 
> _Bool is a C99 type (though I fail to imagine why).  If using this, one
> might as well use the C99 header stdbool.h providing macros for 'bool',
> 'true' and 'false' instead of this.

Agreed - if we should really stick with that, that that's the way to
go.

> > Can we rather try and get rid of all this "bool" stuff instead?  It's
> > just obfuscating the code...
> 
> Indeed.

Thanks.


Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Save energy:  Drive a smaller shell.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-07 22:39 ` [U-Boot] " Wolfgang Denk
  2013-01-07 22:50   ` Scott Wood
  2013-01-07 22:54   ` Måns Rullgård
@ 2013-01-08 16:51   ` Tabi Timur-B04825
  2013-01-08 17:49     ` Wolfgang Denk
  2 siblings, 1 reply; 25+ messages in thread
From: Tabi Timur-B04825 @ 2013-01-08 16:51 UTC (permalink / raw)
  To: u-boot

On Mon, Jan 7, 2013 at 4:39 PM, Wolfgang Denk <wd@denx.de> wrote:
>
> This seems broken to me.  Can we rather try8 and get rid of all this
> "bool" stuff instead?  It's just obfuscating the code...

Like Scott said, we sometimes copy code from Linux that uses 'bool',
so it's simpler if we just retain this commonly-used type.  If it's
part of the language, how is it obfuscating?  Maybe the Linux
developers should have used _Bool instead of bool, but they didn't,
and so here we are.

-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 16:51   ` Tabi Timur-B04825
@ 2013-01-08 17:49     ` Wolfgang Denk
  2013-01-08 17:53       ` Timur Tabi
  2013-01-08 18:34       ` Bernhard Walle
  0 siblings, 2 replies; 25+ messages in thread
From: Wolfgang Denk @ 2013-01-08 17:49 UTC (permalink / raw)
  To: u-boot

Dear Tabi Timur-B04825,

In message <6AE080B68D46FC4BA2D2769E68D765B70820541F@039-SN2MPN1-023.039d.mgd.msft.net> you wrote:
> >
> > This seems broken to me.  Can we rather try8 and get rid of all this
> > "bool" stuff instead?  It's just obfuscating the code...
> 
> Like Scott said, we sometimes copy code from Linux that uses 'bool',
> so it's simpler if we just retain this commonly-used type.  If it's
> part of the language, how is it obfuscating?  Maybe the Linux

_Bool has been introduced very late to any C standard, and you can
still see this from the ugly, unnatural name.

It is my personal firm conviction that the people pushed it were not
the ones who have been using C right from the beginning, say from the
times of Unix v6 or so.

IMHO it is much better to rely on '0' meaning "false" and anything
else meaning "true" instead of insisting on one specific value of
"true".  Yes, people claim the code is easier to read and understand,
but these are the same people who claim drop-down menues are easier to
work wit than a CLI.  And I've seen more than one case where bugs were
caused by using "proper bool types" like this:

	i = 0;
	j = 0;
	k = 2;

	if ((i | j | k) == true) ...


> developers should have used _Bool instead of bool, but they didn't,
> and so here we are.

Well, I raised my concerns, but I do not intend to formally NAK it.
In any case, I insist on using the standard header file.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Lispers are among  the  best  grads  of  the  Sweep-It-Under-Someone-
Else's-Carpet  School of Simulated Simplicity. [Was that sufficiently
incendiary? :-)]  - Larry Wall in <1992Jan10.201804.11926@netlabs.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 17:49     ` Wolfgang Denk
@ 2013-01-08 17:53       ` Timur Tabi
  2013-01-08 19:07         ` Wolfgang Denk
  2013-01-08 18:34       ` Bernhard Walle
  1 sibling, 1 reply; 25+ messages in thread
From: Timur Tabi @ 2013-01-08 17:53 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> Dear Tabi Timur-B04825,
> 
> In message <6AE080B68D46FC4BA2D2769E68D765B70820541F@039-SN2MPN1-023.039d.mgd.msft.net> you wrote:
>>>
>>> This seems broken to me.  Can we rather try8 and get rid of all this
>>> "bool" stuff instead?  It's just obfuscating the code...

>>
>> Like Scott said, we sometimes copy code from Linux that uses 'bool',
>> so it's simpler if we just retain this commonly-used type.  If it's
>> part of the language, how is it obfuscating?  Maybe the Linux
> 
> _Bool has been introduced very late to any C standard, and you can
> still see this from the ugly, unnatural name.

It was introduced in C99, which is over 12 years old.

> It is my personal firm conviction that the people pushed it were not
> the ones who have been using C right from the beginning, say from the
> times of Unix v6 or so.
> 
> IMHO it is much better to rely on '0' meaning "false" and anything
> else meaning "true" instead of insisting on one specific value of
> "true".  Yes, people claim the code is easier to read and understand,
> but these are the same people who claim drop-down menues are easier to
> work wit than a CLI.  And I've seen more than one case where bugs were
> caused by using "proper bool types" like this:
> 
> 	i = 0;
> 	j = 0;
> 	k = 2;
> 
> 	if ((i | j | k) == true) ...

Ok, but this is just wrong.  i, j, and k are not boolean types, so they
should not be compared with 'true' or 'false'.  I don't think you'll find
any disagreement with that.


-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 17:49     ` Wolfgang Denk
  2013-01-08 17:53       ` Timur Tabi
@ 2013-01-08 18:34       ` Bernhard Walle
  2013-01-08 19:08         ` Wolfgang Denk
  1 sibling, 1 reply; 25+ messages in thread
From: Bernhard Walle @ 2013-01-08 18:34 UTC (permalink / raw)
  To: u-boot

* Wolfgang Denk <wd@denx.de> [2013-01-08 18:49]:
> In message <6AE080B68D46FC4BA2D2769E68D765B70820541F@039-SN2MPN1-023.039d.mgd.msft.net> you wrote:
> > >
> > > This seems broken to me.  Can we rather try8 and get rid of all this
> > > "bool" stuff instead?  It's just obfuscating the code...
> > 
> > Like Scott said, we sometimes copy code from Linux that uses 'bool',
> > so it's simpler if we just retain this commonly-used type.  If it's
> > part of the language, how is it obfuscating?  Maybe the Linux
> 
> _Bool has been introduced very late to any C standard, and you can
> still see this from the ugly, unnatural name.

But C99 (well, that's 12 years now!) also includes <stdbool.h> that
defines 'bool', 'true' and 'false'.


Regards,
Bernhard

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 17:53       ` Timur Tabi
@ 2013-01-08 19:07         ` Wolfgang Denk
  2013-01-08 19:09           ` Timur Tabi
                             ` (2 more replies)
  0 siblings, 3 replies; 25+ messages in thread
From: Wolfgang Denk @ 2013-01-08 19:07 UTC (permalink / raw)
  To: u-boot

Dear Timur Tabi,

In message <50EC5D29.1070408@freescale.com> you wrote:
>
> > _Bool has been introduced very late to any C standard, and you can
> > still see this from the ugly, unnatural name.
> 
> It was introduced in C99, which is over 12 years old.

And how old is C?   I think the "official" announcment was 1972, so
that's more than twice as long without that addition.

> > work wit than a CLI.  And I've seen more than one case where bugs were
> > caused by using "proper bool types" like this:
> > 
> > 	i = 0;
> > 	j = 0;
> > 	k = 2;
> > 
> > 	if ((i | j | k) == true) ...
> 
> Ok, but this is just wrong.  i, j, and k are not boolean types, so they
> should not be compared with 'true' or 'false'.  I don't think you'll find
> any disagreement with that.

You are right.  And I wrote that it's a bug.  But this is what you can
easily get from using boolean types.  This is example has not been
invented by me.  I don't even claim that this was good programming
style - all I want to say is that from what I have seen the boolean
types are not a panacea; they cause new problems as well.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
A modem is a baudy house.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 18:34       ` Bernhard Walle
@ 2013-01-08 19:08         ` Wolfgang Denk
  0 siblings, 0 replies; 25+ messages in thread
From: Wolfgang Denk @ 2013-01-08 19:08 UTC (permalink / raw)
  To: u-boot

Dear Bernhard Walle,

In message <20130108183424.GA2761@regiomontanus.your-server.de> you wrote:
>
> > _Bool has been introduced very late to any C standard, and you can
> > still see this from the ugly, unnatural name.
> 
> But C99 (well, that's 12 years now!) also includes <stdbool.h> that
> defines 'bool', 'true' and 'false'.

That's strange - you make the same mistake as Timur.

For me 2013 - 1999 != 12 :-P

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
: ... and it's got weird formatting - Notepad, Write, Works  3  can't
: decipher it, and it's too big to go in DOS Edit. Help!
Install an operating system. :-)                  -- Tom Christiansen

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 19:07         ` Wolfgang Denk
@ 2013-01-08 19:09           ` Timur Tabi
  2013-01-08 19:56           ` York Sun
  2013-01-19  9:30           ` Albert ARIBAUD
  2 siblings, 0 replies; 25+ messages in thread
From: Timur Tabi @ 2013-01-08 19:09 UTC (permalink / raw)
  To: u-boot

Wolfgang Denk wrote:
> You are right.  And I wrote that it's a bug.  But this is what you can
> easily get from using boolean types.  This is example has not been
> invented by me.  I don't even claim that this was good programming
> style - all I want to say is that from what I have seen the boolean
> types are not a panacea; they cause new problems as well.

I don't disagree with any of that, but I don't see what your point is.
Every time you use a new feature, there are also new ways to use it
incorrectly.  By your logic, we should use no new features of the C
language that were invented in the past 20 years.


-- 
Timur Tabi
Linux kernel developer at Freescale

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 19:07         ` Wolfgang Denk
  2013-01-08 19:09           ` Timur Tabi
@ 2013-01-08 19:56           ` York Sun
  2013-01-08 21:39             ` Wolfgang Denk
  2013-01-19  9:30           ` Albert ARIBAUD
  2 siblings, 1 reply; 25+ messages in thread
From: York Sun @ 2013-01-08 19:56 UTC (permalink / raw)
  To: u-boot

On 01/08/2013 11:07 AM, Wolfgang Denk wrote:
> Dear Timur Tabi,
> 
> In message <50EC5D29.1070408@freescale.com> you wrote:
>>
>>> _Bool has been introduced very late to any C standard, and you can
>>> still see this from the ugly, unnatural name.
>>
>> It was introduced in C99, which is over 12 years old.
> 
> And how old is C?   I think the "official" announcment was 1972, so
> that's more than twice as long without that addition.
> 
>>> work wit than a CLI.  And I've seen more than one case where bugs were
>>> caused by using "proper bool types" like this:
>>>
>>> 	i = 0;
>>> 	j = 0;
>>> 	k = 2;
>>>
>>> 	if ((i | j | k) == true) ...
>>
>> Ok, but this is just wrong.  i, j, and k are not boolean types, so they
>> should not be compared with 'true' or 'false'.  I don't think you'll find
>> any disagreement with that.
> 
> You are right.  And I wrote that it's a bug.  But this is what you can
> easily get from using boolean types.  This is example has not been
> invented by me.  I don't even claim that this was good programming
> style - all I want to say is that from what I have seen the boolean
> types are not a panacea; they cause new problems as well.
> 

No disagree. How shall we close this? Will some change like below
acceptable?

diff --git a/include/linux/types.h b/include/linux/types.h
index 925ece7..f07ba41 100644
--- a/include/linux/types.h
+++ b/include/linux/types.h
@@ -7,6 +7,7 @@

 #include <linux/posix_types.h>
 #include <asm/types.h>
+#include <stdbool.h>

 #ifndef __KERNEL_STRICT_NAMES

@@ -113,10 +114,6 @@ typedef            __u64           u_int64_t;
 typedef                __s64           int64_t;
 #endif

-typedef _Bool bool;
-#define false 0
-#define true 1
-
 #endif /* __KERNEL_STRICT_NAMES */

 /*

York

^ permalink raw reply related	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 19:56           ` York Sun
@ 2013-01-08 21:39             ` Wolfgang Denk
  2013-01-08 21:43               ` York Sun
  0 siblings, 1 reply; 25+ messages in thread
From: Wolfgang Denk @ 2013-01-08 21:39 UTC (permalink / raw)
  To: u-boot

Dear York Sun,

In message <50EC79E1.1080802@freescale.com> you wrote:
>
> No disagree. How shall we close this? Will some change like below
> acceptable?
> 
> diff --git a/include/linux/types.h b/include/linux/types.h
> index 925ece7..f07ba41 100644
> --- a/include/linux/types.h
> +++ b/include/linux/types.h
> @@ -7,6 +7,7 @@
> 
>  #include <linux/posix_types.h>
>  #include <asm/types.h>
> +#include <stdbool.h>

Yes, but it needs to be tested that this is working for at least the
most popular tool chains, including when using USE_PRIVATE_LIBGCC.

>  #ifndef __KERNEL_STRICT_NAMES
> 
> @@ -113,10 +114,6 @@ typedef            __u64           u_int64_t;
>  typedef                __s64           int64_t;
>  #endif
> 
> -typedef _Bool bool;
> -#define false 0
> -#define true 1
> -

Yes, but similar removals are needed in a nomber of other header
files as well.

Best regards,

Wolfgang Denk

-- 
DENX Software Engineering GmbH,     MD: Wolfgang Denk & Detlev Zundel
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany
Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd at denx.de
Hindsight is an exact science.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 21:39             ` Wolfgang Denk
@ 2013-01-08 21:43               ` York Sun
  2013-01-08 21:46                 ` Scott Wood
  0 siblings, 1 reply; 25+ messages in thread
From: York Sun @ 2013-01-08 21:43 UTC (permalink / raw)
  To: u-boot

On 01/08/2013 01:39 PM, Wolfgang Denk wrote:
> Dear York Sun,
> 
> In message <50EC79E1.1080802@freescale.com> you wrote:
>>
>> No disagree. How shall we close this? Will some change like below
>> acceptable?
>>
>> diff --git a/include/linux/types.h b/include/linux/types.h
>> index 925ece7..f07ba41 100644
>> --- a/include/linux/types.h
>> +++ b/include/linux/types.h
>> @@ -7,6 +7,7 @@
>>
>>  #include <linux/posix_types.h>
>>  #include <asm/types.h>
>> +#include <stdbool.h>
> 
> Yes, but it needs to be tested that this is working for at least the
> most popular tool chains, including when using USE_PRIVATE_LIBGCC.

Who can help here? I don't have the setup for other than powerpc.

> 
>>  #ifndef __KERNEL_STRICT_NAMES
>>
>> @@ -113,10 +114,6 @@ typedef            __u64           u_int64_t;
>>  typedef                __s64           int64_t;
>>  #endif
>>
>> -typedef _Bool bool;
>> -#define false 0
>> -#define true 1
>> -
> 
> Yes, but similar removals are needed in a nomber of other header
> files as well.
> 
Yes. I have those in my local tree.

York

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 21:43               ` York Sun
@ 2013-01-08 21:46                 ` Scott Wood
  0 siblings, 0 replies; 25+ messages in thread
From: Scott Wood @ 2013-01-08 21:46 UTC (permalink / raw)
  To: u-boot

On 01/08/2013 03:43:34 PM, York Sun wrote:
> On 01/08/2013 01:39 PM, Wolfgang Denk wrote:
> > Dear York Sun,
> >
> > In message <50EC79E1.1080802@freescale.com> you wrote:
> >>
> >> No disagree. How shall we close this? Will some change like below
> >> acceptable?
> >>
> >> diff --git a/include/linux/types.h b/include/linux/types.h
> >> index 925ece7..f07ba41 100644
> >> --- a/include/linux/types.h
> >> +++ b/include/linux/types.h
> >> @@ -7,6 +7,7 @@
> >>
> >>  #include <linux/posix_types.h>
> >>  #include <asm/types.h>
> >> +#include <stdbool.h>
> >
> > Yes, but it needs to be tested that this is working for at least the
> > most popular tool chains, including when using USE_PRIVATE_LIBGCC.
> 
> Who can help here? I don't have the setup for other than powerpc.

http://marc.info/?l=u-boot&m=132164771013203&w=1

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-08 19:07         ` Wolfgang Denk
  2013-01-08 19:09           ` Timur Tabi
  2013-01-08 19:56           ` York Sun
@ 2013-01-19  9:30           ` Albert ARIBAUD
  2013-01-21 18:05             ` Scott Wood
  2 siblings, 1 reply; 25+ messages in thread
From: Albert ARIBAUD @ 2013-01-19  9:30 UTC (permalink / raw)
  To: u-boot

Hi Wolfgang,

My 2 EUR cents:

On Tue, 08 Jan 2013 20:07:15 +0100, Wolfgang Denk <wd@denx.de> wrote:

(sorry for the late chiming in)

> Dear Timur Tabi,
> 
> In message <50EC5D29.1070408@freescale.com> you wrote:
> >
> > > _Bool has been introduced very late to any C standard, and you can
> > > still see this from the ugly, unnatural name.
> > 
> > It was introduced in C99, which is over 12 years old.
> 
> And how old is C?   I think the "official" announcment was 1972, so
> that's more than twice as long without that addition.
> 
> > > work wit than a CLI.  And I've seen more than one case where bugs were
> > > caused by using "proper bool types" like this:
> > > 
> > > 	i = 0;
> > > 	j = 0;
> > > 	k = 2;
> > > 
> > > 	if ((i | j | k) == true) ...
> 
> > Ok, but this is just wrong.  i, j, and k are not boolean types, so they
> > should not be compared with 'true' or 'false'.  I don't think you'll find
> > any disagreement with that.
> 
> You are right.  And I wrote that it's a bug.  But this is what you can
> easily get from using boolean types.  This is example has not been
> invented by me.  I don't even claim that this was good programming
> style - all I want to say is that from what I have seen the boolean
> types are not a panacea; they cause new problems as well.

Ok, so there are three things in Wolfgang's example: a lax boolean (set
to 2), a mix-up between bitwise and boolean operators (which a compiler
may or may not detect or at least flag as suspicious), and finally a
comparison of the lax boolean (2) with a strict boolean (true, equal
to 1) which will fail.

I guess we're all aware of this type of problem. To avoid it, I
personally try to apply the Postel principle here: be conservative in
what you do, thus only produce strict boolean objects, and be liberal
in what you get, i.e. consider all boolean expressions to be lax.

This means that as far as coding practice is concerned, I tend to favor
the style set forth in the next three lines, where I always compute 
booleans with true and false and boolean operators, but test them
'zero/nonzero':

	/* what I favor */
	clk_is_enabled = ((reg_val >> 9) & 1) ? true: false;
	ip_is_enabled = clk_is_enabled && pwd_is_enabled;
	if (clk_is_enabled) { ...

rather than assigning them 'zero/nonzero', or using bitwise ops on
booleans, or testing against boolean constants (although I concede
that the first line below wins over its counterpart above as far as
concision is concerned).

	/* what I don't favor */
	clk_is_enabled = ((register >> 9) & 1);
	ip_is_enabled = clk_is_enabled & pwd_is_enabled;
	if (clk_is_enabled == true) { ...

This way I am sure to evaluate any nonzero value as 'true', so lax code
can safely pass me lax booleans, and I am sure that my booleans equal 1
when true, so I always pass strict booleans to strict code.

Oh, and I also try to wisely name boolean objects, so that they read out
loud as a boolean statement, e.g. "clock is enabled", but this is a bit
(more) beside the point.

> Best regards,
> 
> Wolfgang Denk

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-19  9:30           ` Albert ARIBAUD
@ 2013-01-21 18:05             ` Scott Wood
  2013-01-21 22:36               ` Måns Rullgård
  0 siblings, 1 reply; 25+ messages in thread
From: Scott Wood @ 2013-01-21 18:05 UTC (permalink / raw)
  To: u-boot

On 01/19/2013 03:30:30 AM, Albert ARIBAUD wrote:
> 	/* what I favor */
> 	clk_is_enabled = ((reg_val >> 9) & 1) ? true: false;
> 	ip_is_enabled = clk_is_enabled && pwd_is_enabled;
> 	if (clk_is_enabled) { ...
> 
> rather than assigning them 'zero/nonzero', or using bitwise ops on
> booleans, or testing against boolean constants (although I concede
> that the first line below wins over its counterpart above as far as
> concision is concerned).

Conciseness can be improved with "!!((reg_val >> 9) & 1)".

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-21 18:05             ` Scott Wood
@ 2013-01-21 22:36               ` Måns Rullgård
  2013-01-21 22:51                 ` Scott Wood
  0 siblings, 1 reply; 25+ messages in thread
From: Måns Rullgård @ 2013-01-21 22:36 UTC (permalink / raw)
  To: u-boot

Scott Wood <scottwood@freescale.com> writes:

> On 01/19/2013 03:30:30 AM, Albert ARIBAUD wrote:
>> 	/* what I favor */
>> 	clk_is_enabled = ((reg_val >> 9) & 1) ? true: false;
>> 	ip_is_enabled = clk_is_enabled && pwd_is_enabled;
>> 	if (clk_is_enabled) { ...
>> 
>> rather than assigning them 'zero/nonzero', or using bitwise ops on
>> booleans, or testing against boolean constants (although I concede
>> that the first line below wins over its counterpart above as far as
>> concision is concerned).
>
> Conciseness can be improved with "!!((reg_val >> 9) & 1)".

x & 1 is already either zero or one.  Any further operations are nothing
but obfuscation.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-21 22:36               ` Måns Rullgård
@ 2013-01-21 22:51                 ` Scott Wood
  2013-01-21 23:08                   ` Måns Rullgård
  0 siblings, 1 reply; 25+ messages in thread
From: Scott Wood @ 2013-01-21 22:51 UTC (permalink / raw)
  To: u-boot

On 01/21/2013 04:36:42 PM, M?ns Rullg?rd wrote:
> Scott Wood <scottwood@freescale.com> writes:
> 
> > On 01/19/2013 03:30:30 AM, Albert ARIBAUD wrote:
> >> 	/* what I favor */
> >> 	clk_is_enabled = ((reg_val >> 9) & 1) ? true: false;
> >> 	ip_is_enabled = clk_is_enabled && pwd_is_enabled;
> >> 	if (clk_is_enabled) { ...
> >>
> >> rather than assigning them 'zero/nonzero', or using bitwise ops on
> >> booleans, or testing against boolean constants (although I concede
> >> that the first line below wins over its counterpart above as far as
> >> concision is concerned).
> >
> > Conciseness can be improved with "!!((reg_val >> 9) & 1)".
> 
> x & 1 is already either zero or one.  Any further operations are  
> nothing
> but obfuscation.

The point is to avoid depending on the actual integer values of the  
boolean type, and make the code more robust against changes (e.g.  
someone later comes along and says "hmm, that 1 should be a 3 because  
we care about that other register bit as well" without noticing that  
it's being assigned to a boolean.

-Scott

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-21 22:51                 ` Scott Wood
@ 2013-01-21 23:08                   ` Måns Rullgård
  2013-01-22  7:41                     ` Albert ARIBAUD
  0 siblings, 1 reply; 25+ messages in thread
From: Måns Rullgård @ 2013-01-21 23:08 UTC (permalink / raw)
  To: u-boot

Scott Wood <scottwood@freescale.com> writes:

> On 01/21/2013 04:36:42 PM, M?ns Rullg?rd wrote:
>> Scott Wood <scottwood@freescale.com> writes:
>> 
>> > On 01/19/2013 03:30:30 AM, Albert ARIBAUD wrote:
>> >> 	/* what I favor */
>> >> 	clk_is_enabled = ((reg_val >> 9) & 1) ? true: false;
>> >> 	ip_is_enabled = clk_is_enabled && pwd_is_enabled;
>> >> 	if (clk_is_enabled) { ...
>> >>
>> >> rather than assigning them 'zero/nonzero', or using bitwise ops on
>> >> booleans, or testing against boolean constants (although I concede
>> >> that the first line below wins over its counterpart above as far as
>> >> concision is concerned).
>> >
>> > Conciseness can be improved with "!!((reg_val >> 9) & 1)".
>> 
>> x & 1 is already either zero or one.  Any further operations are  
>> nothing
>> but obfuscation.
>
> The point is to avoid depending on the actual integer values of the  
> boolean type,

Boolean expressions are defined to have a value of zero or one, and the
_Bool type (may it burn in hell) must be able to represent those values.

> and make the code more robust against changes (e.g. someone later
> comes along and says "hmm, that 1 should be a 3 because we care about
> that other register bit as well" without noticing that it's being
> assigned to a boolean.

If you stayed away from the silly _Bool type that wouldn't be a problem,
as long as any uses of the value treat all non-zero values equally, i.e. 
only use it in a boolean context and not compare against explicit values
or perform arithmetic or bitwise logic operations on it.

In other words, boolifying on use rather than on assignment is generally
safer and usually at least as efficient.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-21 23:08                   ` Måns Rullgård
@ 2013-01-22  7:41                     ` Albert ARIBAUD
  2013-01-22 12:59                       ` Måns Rullgård
  0 siblings, 1 reply; 25+ messages in thread
From: Albert ARIBAUD @ 2013-01-22  7:41 UTC (permalink / raw)
  To: u-boot

Hi M?ns,

> In other words, boolifying on use rather than on assignment is generally
> safer and usually at least as efficient.

Except when assigning a C = A & B where A and B happen to have
no common bit set. Which is why I think 'boolifying' as soon as
possible -- on assignment -- is way safer than on use.

Amicalement,
-- 
Albert.

^ permalink raw reply	[flat|nested] 25+ messages in thread

* [U-Boot] [PATCH] Introduce a global bool type
  2013-01-22  7:41                     ` Albert ARIBAUD
@ 2013-01-22 12:59                       ` Måns Rullgård
  0 siblings, 0 replies; 25+ messages in thread
From: Måns Rullgård @ 2013-01-22 12:59 UTC (permalink / raw)
  To: u-boot

Albert ARIBAUD <albert.u.boot@aribaud.net> writes:

> Hi M?ns,
>
>> In other words, boolifying on use rather than on assignment is generally
>> safer and usually at least as efficient.
>
> Except when assigning a C = A & B where A and B happen to have
> no common bit set. Which is why I think 'boolifying' as soon as
> possible -- on assignment -- is way safer than on use.

But that's not a boolean context.  You should use A && B.

The thing is, when using a value, you know if you need a boolean, but
not necessarily (easily) how the value was assigned.  Conversely, when
assigning a value, you do not know how it will be used.  By always
boolifying on use, you remove the need to keep track of which values are
"true" booleans and which ones are arbitrary values (or even pointers)
you happen to be using in a boolean fashion.

-- 
M?ns Rullg?rd
mans at mansr.com

^ permalink raw reply	[flat|nested] 25+ messages in thread

end of thread, other threads:[~2013-01-22 12:59 UTC | newest]

Thread overview: 25+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-01-07 22:10 [U-Boot] [PATCH] Introduce a global bool type York Sun
2013-01-07 22:29 ` Scott Wood
2013-01-07 22:32 ` [U-Boot] [u-boot-release] " Timur Tabi
2013-01-07 22:39 ` [U-Boot] " Wolfgang Denk
2013-01-07 22:50   ` Scott Wood
2013-01-07 22:54   ` Måns Rullgård
2013-01-08  6:25     ` Wolfgang Denk
2013-01-08 16:51   ` Tabi Timur-B04825
2013-01-08 17:49     ` Wolfgang Denk
2013-01-08 17:53       ` Timur Tabi
2013-01-08 19:07         ` Wolfgang Denk
2013-01-08 19:09           ` Timur Tabi
2013-01-08 19:56           ` York Sun
2013-01-08 21:39             ` Wolfgang Denk
2013-01-08 21:43               ` York Sun
2013-01-08 21:46                 ` Scott Wood
2013-01-19  9:30           ` Albert ARIBAUD
2013-01-21 18:05             ` Scott Wood
2013-01-21 22:36               ` Måns Rullgård
2013-01-21 22:51                 ` Scott Wood
2013-01-21 23:08                   ` Måns Rullgård
2013-01-22  7:41                     ` Albert ARIBAUD
2013-01-22 12:59                       ` Måns Rullgård
2013-01-08 18:34       ` Bernhard Walle
2013-01-08 19:08         ` Wolfgang Denk

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.