Re: [Qemu-devel] [PATCH] Remove typedef for bool from eepro100.c

[Jamie Lokier <jamie@shareable.org>]

Reimar Döffinger wrote:
> On Thu, Aug 27, 2009 at 04:00:17PM +0200, Laurent Desnogues wrote:
> > On Thu, Aug 27, 2009 at 2:45 PM, Reimar
> > Döffinger<Reimar.Doeffinger@gmx.de> wrote:
> > > 1) stdbool bool is probably 4 bytes, not just 1 like char
> > 
> > It's one byte on my gcc 4.4.0.
> > 
> > > I suggest to just get rid of bool in this file, it is only used in 5
> > > places, i.e. change
> > >>        bool bit_el = ((command & 0x8000) != 0);
> > > to
> > >>        int bit_el = command & 0x8000;
> > 
> > This is dangerous if you start using bit_el in integer expressions
> > by accident (for instance using & or |).
> 
> Programming errors are dangerous in general. I don't see much of a
> point of cluttering the code with not really effective ways to hide
> their effects (unless you wanted to suggest using "bool bit_el =
> command & 0x8000;", I see that bool is already used in some places
> in qemu and performance doesn't matter here so it indeed shouldn't
> be a problem).

I've seen the "int flag = (x & FLAG); if (flag & otherflag)" bug
enough times to consider it one of those subtle things that
programmers don't notice easily.

A variation is "long flag = (x & FLAG); ... function(flag)" where
"function" takes an int argument and FLAG doesn't fit.  Or just "int
flag = (x & FLAG)".

My approach is to use "!= 0" if I think the variable really is
boolean, and only keep the original masked bit pattern if it's clear
in context that the variable is supposed to contain a bit pattern.

-- Jamie