[PATCH] sim: dv-cfi: include stdbool.h

[PATCH] sim: dv-cfi: include stdbool.h

[Mike Frysinger]

The dv-cfi code uses the bool type but doesn't include the stdbool.h
header.  I didn't notice for Blackfin targets as the core Blackfin
header will include stdbool.h, but most targets don't do this.

Signed-off-by: Mike Frysinger <vapier@gentoo.org>

2011-07-11  Mike Frysinger  <vapier@gentoo.org>

	* dv-cfi.c: Include stdbool.h.
---
 sim/common/dv-cfi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sim/common/dv-cfi.c b/sim/common/dv-cfi.c
index 42d868c..a1ecaf9 100644
--- a/sim/common/dv-cfi.c
+++ b/sim/common/dv-cfi.c
@@ -27,6 +27,7 @@
 #include <math.h>
 #include <errno.h>
 #include <fcntl.h>
+#include <stdbool.h>
 #include <unistd.h>
 #ifdef HAVE_SYS_MMAN_H
 #include <sys/mman.h>
-- 
1.7.6

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 277 bytes --]

On Monday, July 11, 2011 14:58:13 Mike Frysinger wrote:
> The dv-cfi code uses the bool type but doesn't include the stdbool.h
> header.  I didn't notice for Blackfin targets as the core Blackfin
> header will include stdbool.h, but most targets don't do this.

ping ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Mike Frysinger]

[-- Attachment #1: Type: Text/Plain, Size: 477 bytes --]

On Wednesday 28 September 2011 22:01:32 Mike Frysinger wrote:
> On Monday, July 11, 2011 14:58:13 Mike Frysinger wrote:
> > The dv-cfi code uses the bool type but doesn't include the stdbool.h
> > header.  I didn't notice for Blackfin targets as the core Blackfin
> > header will include stdbool.h, but most targets don't do this.
> 
> ping ...

i've committed this since it fixes known build errors for some targets.  
hopefully that doesn't annoy people ...
-mike

[-- Attachment #2: This is a digitally signed message part. --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Joel Brobecker]

> The dv-cfi code uses the bool type but doesn't include the stdbool.h
> header.  I didn't notice for Blackfin targets as the core Blackfin
> header will include stdbool.h, but most targets don't do this.
> 
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> 
> 2011-07-11  Mike Frysinger  <vapier@gentoo.org>
> 
> 	* dv-cfi.c: Include stdbool.h.

I don't think that this is right. stdbool.h is C99, whereas GDB should
build with a C90 compiler.

What we should do is get rid of uses of true and false, and use the
typical zero/nonzero values instead. I think that the blackfin header
and code should be cleaned up accordingly as well.

-- 
Joel

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Mike Frysinger]

On Monday 17 October 2011 12:50:24 Joel Brobecker wrote:
> > The dv-cfi code uses the bool type but doesn't include the stdbool.h
> > header.  I didn't notice for Blackfin targets as the core Blackfin
> > header will include stdbool.h, but most targets don't do this.
> > 
> > Signed-off-by: Mike Frysinger <vapier@gentoo.org>
> > 
> > 2011-07-11  Mike Frysinger  <vapier@gentoo.org>
> > 
> > 	* dv-cfi.c: Include stdbool.h.
> 
> I don't think that this is right. stdbool.h is C99, whereas GDB should
> build with a C90 compiler.

hmm, i was not aware of this.  i thought it dated back further.

> What we should do is get rid of uses of true and false, and use the
> typical zero/nonzero values instead. I think that the blackfin header
> and code should be cleaned up accordingly as well.

ah, this would make me sad.  would a  configure check for stdbool.h be 
acceptable ?
-mike

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Joel Brobecker]

> > What we should do is get rid of uses of true and false, and use the
> > typical zero/nonzero values instead. I think that the blackfin header
> > and code should be cleaned up accordingly as well.
> 
> ah, this would make me sad.  would a  configure check for stdbool.h be 
> acceptable ?

But what would you do if stdbool.h is not available? Wouldn't that
complicate the code in the end?

-- 
Joel

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Mike Frysinger]

On Monday 17 October 2011 13:26:03 Joel Brobecker wrote:
> > > What we should do is get rid of uses of true and false, and use the
> > > typical zero/nonzero values instead. I think that the blackfin header
> > > and code should be cleaned up accordingly as well.
> > 
> > ah, this would make me sad.  would a  configure check for stdbool.h be
> > acceptable ?
> 
> But what would you do if stdbool.h is not available? Wouldn't that
> complicate the code in the end?

#ifdef HAVE_STDBOOL_H
# include <stdbool.h>
#else
# define bool int
# define true 1
# define false 0
#endif

when reading the code, i find true/false to be more natural than 0/1.  
especially when 0/1 return values are not consistent across code bases.

if you really prefer to not have this, i understand.  but i find stdbool to be 
a useful addition ...
-mike

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Joel Brobecker]

> #ifdef HAVE_STDBOOL_H
> # include <stdbool.h>
> #else
> # define bool int
> # define true 1
> # define false 0
> #endif

I don't know what the others feel about this, I'm always wary of
this, but maybe I'm too paranoid. I remember using these sorts of
defines, and getting into portability issues, but that was a very
long time ago, and I don't remember the details. Perhaps I didn't
do something right, or it's now OBE.

In the end, I am a GDB maintainer, not a sim maintainer, so I can
only express an opinion. If the above doesn't break any other
platforms, I suppose that's good enough.

I think that the best compromise, if you really want to "true" and/or
"false", is to probably start using gnulib in the sim code as well.
I just don't know offhand how easy it would be to integrate that
with GDB's use of gnulib. But it'd allow you to include stdbool.h
unconditionally, knowing that gnulib would provide it if the system
doesn't already.

Anyways, I'll let you decide on this particular topic, I don't think
it's a critical piece of the sim code.

> when reading the code, i find true/false to be more natural than 0/1.
> especially when 0/1 return values are not consistent across code
> bases.

Then just be carefule that "if COND == true" is no longer the same as
"if COND".  I just find it more robust to think that truth is equivalent
to nonzero.

-- 
Joel

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Mike Frysinger]

On Monday 17 October 2011 16:27:17 Joel Brobecker wrote:
> > #ifdef HAVE_STDBOOL_H
> > # include <stdbool.h>
> > #else
> > # define bool int
> > # define true 1
> > # define false 0
> > #endif
> 
> I don't know what the others feel about this, I'm always wary of
> this, but maybe I'm too paranoid. I remember using these sorts of
> defines, and getting into portability issues, but that was a very
> long time ago, and I don't remember the details. Perhaps I didn't
> do something right, or it's now OBE.
> 
> In the end, I am a GDB maintainer, not a sim maintainer, so I can
> only express an opinion. If the above doesn't break any other
> platforms, I suppose that's good enough.

do we have any "global" sim maintainers ?  either i'm reading sim/MAINTAINERS 
wrong, or we don't ...

i can migrate the dv-cfi code to use 0/1 to match the rest of the common/ tree, 
and because the "bool" usage is contained inside the one file.  np there.  but 
for bfin/, i'd like to keep stdbool.h usage for the arch port since the bool 
usage spreads across files.

this would also contain the breakage to anyone targeting Blackfin ports rather 
than anyone trying to use the sim.

> I think that the best compromise, if you really want to "true" and/or
> "false", is to probably start using gnulib in the sim code as well.
> I just don't know offhand how easy it would be to integrate that
> with GDB's use of gnulib. But it'd allow you to include stdbool.h
> unconditionally, knowing that gnulib would provide it if the system
> doesn't already.

i don't think we can integrate the two.  gdb needs sim to complete first 
because gdb links against the resulting libsim.a.  so we can't have sim depend 
on any of the build targets under the gdb subdir.

> Anyways, I'll let you decide on this particular topic, I don't think
> it's a critical piece of the sim code.
> 
> > when reading the code, i find true/false to be more natural than 0/1.
> > especially when 0/1 return values are not consistent across code
> > bases.
> 
> Then just be carefule that "if COND == true" is no longer the same as
> "if COND".  I just find it more robust to think that truth is equivalent
> to nonzero.

sure, if people are careful, 0/1 isn't an issue.  but if people were careful, 
we wouldn't need stdbool in the first place :).  i find it a comforting layer 
for sanity to be enforced in the language.
-mike

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Tom Tromey]

>>>>> "Joel" == Joel Brobecker <brobecker@adacore.com> writes:

>> #ifdef HAVE_STDBOOL_H
>> # include <stdbool.h>
>> #else
>> # define bool int
>> # define true 1
>> # define false 0
>> #endif

Joel> I don't know what the others feel about this, I'm always wary of
Joel> this, but maybe I'm too paranoid. I remember using these sorts of
Joel> defines, and getting into portability issues, but that was a very
Joel> long time ago, and I don't remember the details. Perhaps I didn't
Joel> do something right, or it's now OBE.

GCC has done this for years and years.
I think it has been a net positive for code readability.

Joel> I think that the best compromise, if you really want to "true" and/or
Joel> "false", is to probably start using gnulib in the sim code as well.
Joel> I just don't know offhand how easy it would be to integrate that
Joel> with GDB's use of gnulib. But it'd allow you to include stdbool.h
Joel> unconditionally, knowing that gnulib would provide it if the system
Joel> doesn't already.

It could perhaps be put in libiberty more easily.

Tom

Re: [PATCH] sim: dv-cfi: include stdbool.h

[Joel Brobecker]

> >> #ifdef HAVE_STDBOOL_H
> >> # include <stdbool.h>
> >> #else
> >> # define bool int
> >> # define true 1
> >> # define false 0
> >> #endif
> 
> Joel> I don't know what the others feel about this, I'm always wary of
> Joel> this, but maybe I'm too paranoid. I remember using these sorts of
> Joel> defines, and getting into portability issues, but that was a very
> Joel> long time ago, and I don't remember the details. Perhaps I didn't
> Joel> do something right, or it's now OBE.
> 
> GCC has done this for years and years.
> I think it has been a net positive for code readability.

OK, then it's fine for me as well.

-- 
Joel