[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH] Remove typedef for bool from eepro100.c
From: |
Jamie Lokier |
Subject: |
Re: [Qemu-devel] [PATCH] Remove typedef for bool from eepro100.c |
Date: |
Thu, 27 Aug 2009 15:59:34 +0100 |
User-agent: |
Mutt/1.5.13 (2006-08-11) |
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<address@hidden> 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